From: Michael Jeanson Date: Mon, 13 Sep 2021 18:16:22 +0000 (-0400) Subject: fix: Revert "Makefile: Enable -Wimplicit-fallthrough for Clang" (v5.15) X-Git-Tag: v2.12.7~8 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=1d00109f7168cd30f3a2de6173d1e2ec36372ba9;p=lttng-modules.git fix: Revert "Makefile: Enable -Wimplicit-fallthrough for Clang" (v5.15) Starting with v5.15, "-Wimplicit-fallthrough=5" was added to the build flags which requires the use of "__attribute__((__fallthrough__))" to annotate fallthrough case statements. See upstream commit by the man himself: commit d936eb23874433caa3e3d841cfa16f5434b85dcf Author: Linus Torvalds Date: Thu Jul 15 18:05:31 2021 -0700 Revert "Makefile: Enable -Wimplicit-fallthrough for Clang" This reverts commit b7eb335e26a9c7f258c96b3962c283c379d3ede0. It turns out that the problem with the clang -Wimplicit-fallthrough warning is not about the kernel source code, but about clang itself, and that the warning is unusable until clang fixes its broken ways. In particular, when you enable this warning for clang, you not only get warnings about implicit fallthroughs. You also get this: warning: fallthrough annotation in unreachable code [-Wimplicit-fallthrough] which is completely broken becasue it (a) doesn't even tell you where the problem is (seriously: no line numbers, no filename, no nothing). (b) is fundamentally broken anyway, because there are perfectly valid reasons to have a fallthrough statement even if it turns out that it can perhaps not be reached. In the kernel, an example of that second case is code in the scheduler: switch (state) { case cpuset: if (IS_ENABLED(CONFIG_CPUSETS)) { cpuset_cpus_allowed_fallback(p); state = possible; break; } fallthrough; case possible: where if CONFIG_CPUSETS is enabled you actually never hit the fallthrough case at all. But that in no way makes the fallthrough wrong. So the warning is completely broken, and enabling it for clang is a very bad idea. In the meantime, we can keep the gcc option enabled, and make the gcc build use -Wimplicit-fallthrough=5 which means that we will at least continue to require a proper fallthrough statement, and that gcc won't silently accept the magic comment versions. Because gcc does this all correctly, and while the odd "=5" part is kind of obscure, it's documented in [1]: "-Wimplicit-fallthrough=5 doesn’t recognize any comments as fallthrough comments, only attributes disable the warning" so if clang ever fixes its bad behavior we can try enabling it there again. Change-Id: Iea69849592fb69ac04fb9bb28efcd6b8dce8ba88 Signed-off-by: Michael Jeanson Signed-off-by: Mathieu Desnoyers --- diff --git a/lttng-abi.c b/lttng-abi.c index 18f5123c..09b14d66 100644 --- a/lttng-abi.c +++ b/lttng-abi.c @@ -30,6 +30,7 @@ #include #include #include +#include #include /* for wrapper_vmalloc_sync_mappings() */ #include #include @@ -881,7 +882,8 @@ long lttng_metadata_ring_buffer_ioctl(struct file *filp, */ return -ENOSYS; } - case RING_BUFFER_FLUSH_EMPTY: /* Fall-through. */ + case RING_BUFFER_FLUSH_EMPTY: + lttng_fallthrough; case RING_BUFFER_FLUSH: { struct lttng_metadata_stream *stream = filp->private_data; @@ -990,7 +992,8 @@ long lttng_metadata_ring_buffer_compat_ioctl(struct file *filp, */ return -ENOSYS; } - case RING_BUFFER_FLUSH_EMPTY: /* Fall-through. */ + case RING_BUFFER_FLUSH_EMPTY: + lttng_fallthrough; case RING_BUFFER_FLUSH: { struct lttng_metadata_stream *stream = filp->private_data; @@ -1290,14 +1293,19 @@ int lttng_abi_validate_event_param(struct lttng_kernel_event *event_param) } break; - case LTTNG_KERNEL_TRACEPOINT: /* Fallthrough */ - case LTTNG_KERNEL_KPROBE: /* Fallthrough */ - case LTTNG_KERNEL_KRETPROBE: /* Fallthrough */ - case LTTNG_KERNEL_NOOP: /* Fallthrough */ + case LTTNG_KERNEL_TRACEPOINT: + lttng_fallthrough; + case LTTNG_KERNEL_KPROBE: + lttng_fallthrough; + case LTTNG_KERNEL_KRETPROBE: + lttng_fallthrough; + case LTTNG_KERNEL_NOOP: + lttng_fallthrough; case LTTNG_KERNEL_UPROBE: break; - case LTTNG_KERNEL_FUNCTION: /* Fallthrough */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: return -EINVAL; } diff --git a/lttng-events.c b/lttng-events.c index 5be4c9b8..60f5623b 100644 --- a/lttng-events.c +++ b/lttng-events.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -430,7 +431,8 @@ int lttng_event_enable(struct lttng_event *event) case LTTNG_KERNEL_KRETPROBE: ret = lttng_kretprobes_event_enable_state(event, 1); break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -466,7 +468,8 @@ int lttng_event_disable(struct lttng_event *event) case LTTNG_KERNEL_KRETPROBE: ret = lttng_kretprobes_event_enable_state(event, 0); break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -613,7 +616,8 @@ struct lttng_event *_lttng_event_create(struct lttng_channel *chan, case LTTNG_KERNEL_SYSCALL: event_name = event_param->name; break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through. */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -796,7 +800,8 @@ struct lttng_event *_lttng_event_create(struct lttng_channel *chan, ret = try_module_get(event->desc->owner); WARN_ON_ONCE(!ret); break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); ret = -EINVAL; @@ -863,7 +868,8 @@ void register_event(struct lttng_event *event) case LTTNG_KERNEL_NOOP: ret = 0; break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); } @@ -907,7 +913,8 @@ int _lttng_event_unregister(struct lttng_event *event) lttng_uprobes_unregister(event); ret = 0; break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); } @@ -943,7 +950,8 @@ void _lttng_event_destroy(struct lttng_event *event) module_put(event->desc->owner); lttng_uprobes_destroy_private(event); break; - case LTTNG_KERNEL_FUNCTION: /* Fall-through */ + case LTTNG_KERNEL_FUNCTION: + lttng_fallthrough; default: WARN_ON_ONCE(1); } @@ -2649,7 +2657,7 @@ int print_escaped_ctf_string(struct lttng_session *session, const char *string) if (ret) goto error; /* We still print the current char */ - /* Fallthrough */ + lttng_fallthrough; default: ret = lttng_metadata_printf(session, "%c", cur); break; diff --git a/lttng-filter-interpreter.c b/lttng-filter-interpreter.c index 6e5a5139..d9dc6195 100644 --- a/lttng-filter-interpreter.c +++ b/lttng-filter-interpreter.c @@ -7,6 +7,7 @@ * Copyright (C) 2010-2016 Mathieu Desnoyers */ +#include #include #include #include @@ -419,7 +420,8 @@ static int dynamic_get_index(struct lttng_probe_ctx *lttng_probe_ctx, } break; case LOAD_ROOT_CONTEXT: - case LOAD_ROOT_APP_CONTEXT: /* Fall-through */ + lttng_fallthrough; + case LOAD_ROOT_APP_CONTEXT: { ret = context_get_index(lttng_probe_ctx, &stack_top->u.ptr, diff --git a/lttng-filter-specialize.c b/lttng-filter-specialize.c index 4f9a0cd1..e9a85c78 100644 --- a/lttng-filter-specialize.c +++ b/lttng-filter-specialize.c @@ -11,6 +11,8 @@ #include #include "lib/align.h" +#include + static ssize_t bytecode_reserve_data(struct bytecode_runtime *runtime, size_t align, size_t len) { @@ -246,7 +248,8 @@ static int specialize_get_index(struct bytecode_runtime *runtime, } case OBJECT_TYPE_STRUCT: /* Only generated by the specialize phase. */ - case OBJECT_TYPE_VARIANT: /* Fall-through */ + case OBJECT_TYPE_VARIANT: + lttng_fallthrough; default: printk(KERN_WARNING "Unexpected get index type %d", (int) stack_top->load.object_type); diff --git a/lttng-string-utils.c b/lttng-string-utils.c index 416896cc..e8075706 100644 --- a/lttng-string-utils.c +++ b/lttng-string-utils.c @@ -4,6 +4,7 @@ */ #include +#include #include @@ -302,7 +303,7 @@ retry: p = pattern_get_char_at_cb(p_at, pattern_get_char_at_cb_data); - /* Fall-through. */ + lttng_fallthrough; default: /* * Default case which will compare the escaped diff --git a/wrapper/compiler_attributes.h b/wrapper/compiler_attributes.h new file mode 100644 index 00000000..ae449033 --- /dev/null +++ b/wrapper/compiler_attributes.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: (GPL-2.0-only or LGPL-2.1-only) + * + * wrapper/compiler_attributes.h + * + * Copyright (C) 2021 Michael Jeanson + */ + +#ifndef _LTTNG_WRAPPER_COMPILER_ATTRIBUTES_H +#define _LTTNG_WRAPPER_COMPILER_ATTRIBUTES_H + +#include + +#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(4,20,0)) +#include +#endif + +#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,4,0)) + +/* + * Use the kernel provided fallthrough attribute macro. + */ +#define lttng_fallthrough fallthrough + +#else /* LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,4,0) */ + +/* + * Fallback to the comment for kernels pre 5.15 that don't build with + * '-Wimplicit-fallthrough=5'. + */ +#define lttng_fallthrough do {} while (0) /* fallthrough */ + +#endif /* LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,4,0) */ + +#endif /* _LTTNG_WRAPPER_COMPILER_ATTRIBUTES_H */