On Mon, Jul 30, 2018 at 1:01 PM Nathan Chancellor natechancellor@gmail.com wrote:
On Mon, Jul 30, 2018 at 12:48:06PM -0700, Nick Desaulniers wrote:
On Mon, Jul 30, 2018 at 11:39 AM Nathan Chancellor natechancellor@gmail.com wrote:
On Mon, Jul 30, 2018 at 10:06:20AM -0700, Nick Desaulniers wrote:
Starting with Clang-7.0, _THIS_IP_ generates -Wreturn-stack-address warnings for almost every translation unit. In general, I'd prefer to leave this on (returning the address of a stack allocated variable is in general a bad idea) and disable it only at whitelisted call sites.
We can't do something like: #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wreturn-stack-address"
<code> #pragma clang diagnostic pop
in a GNU Statement Expression or macro, hence we use _Pragma, which is its raison d'être: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Signed-off-by: Nick Desaulniers ndesaulniers@google.com
include/linux/kernel.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 941dc0a5a877..5906f5727f90 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -168,7 +168,15 @@
#define _RET_IP_ (unsigned long)__builtin_return_address(0) -#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; }) +#define _THIS_IP_ ( \ +{ \
_Pragma("clang diagnostic push") \_Pragma("clang diagnostic ignored \"-Wreturn-stack-address\"") \__label__ __here; \+__here: (unsigned long)&&__here; \
_Pragma("clang diagnostic pop") \+} \ +)
#ifdef CONFIG_LBDAF
# include <asm/div64.h>
2.18.0.233.g985f88cf7e-goog
This generates a ton of warnings with GCC:
In file included from ./include/linux/spinlock.h:58, from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:6, from ./include/linux/slab.h:15, from ./include/linux/crypto.h:24, from arch/x86/kernel/asm-offsets.c:9: ./include/linux/bottom_half.h: In function ‘local_bh_disable’: ./include/linux/bottom_half.h:19: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] __local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
./include/linux/bottom_half.h:19: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] ./include/linux/bottom_half.h:19: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] ./include/linux/bottom_half.h: In function ‘local_bh_enable’: ./include/linux/bottom_half.h:32: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] __local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
./include/linux/bottom_half.h:32: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] ./include/linux/bottom_half.h:32: error: ignoring #pragma clang diagnostic [-Werror=unknown-pragmas] cc1: all warnings being treated as errors make[1]: *** [Kbuild:56: arch/x86/kernel/asm-offsets.s] Error 1 make: *** [Makefile:1081: prepare0] Error 2
Ah, good catch. I thought I had tested locally with gcc-8, but it seems that it was likely only godbolt. I do see the errors locally when building with gcc-8.
A proper solution is probably going to involve what was done for the -Wattribute-alias warnings from GCC 8 in commits 8793bb7f4a9d ("kbuild: add macro for controlling warnings to linux/compiler.h") and bee20031772a ("disable -Wattribute-alias warning for SYSCALL_DEFINEx()")
I'll take a look at it in a bit unless someone beats me to it.
I'll fix this up, triple check with gcc-8, and attribute you in the Suggested-by tag. Thank you for verifying. -- Thanks, ~Nick Desaulniers
I forgot to mention there is a similar macro that will need this same fix in arch/arm64/include/asm/processor.h (current_text_addr), if you want to tackle it in v2. Do CC me on the next series so that I can test, thank you for doing this!
!!!
$ grep -R current_text_addr
shows many definitions of current_text_addr per arch/ that are all essentially the same, and very very few uses of it. I prefer to consolidate current_text_addr() in a follow up series, since a common definition with per-arch overrides will have to get a lot of sign offs.
Though, for arm64, a simple: -#define current_text_addr() ({ __label__ _l; _l: &&_l;}) +#define current_text_addr() _THIS_IP_
does the trick, it would be better to have a single shared definition.