diff options
author | Tom de Vries <tdevries@suse.de> | 2021-07-30 14:07:40 +0200 |
---|---|---|
committer | Tom de Vries <tdevries@suse.de> | 2021-07-30 14:07:40 +0200 |
commit | fb6262e8534e0148a4a424e9e5138159af19faf1 (patch) | |
tree | a710f96bbfe6f017bf4823ce6047e7a635d88fb4 | |
parent | f681e5867de63f1c8ca692023cf86e4c884fdae7 (diff) | |
download | gdb-fb6262e8534e0148a4a424e9e5138159af19faf1.zip gdb-fb6262e8534e0148a4a424e9e5138159af19faf1.tar.gz gdb-fb6262e8534e0148a4a424e9e5138159af19faf1.tar.bz2 |
[gdb/build] Disable attribute nonnull
With trunk gcc (12.0) we're running into a -Werror=nonnull-compare build
breaker in gdb, which caused a broader review of the usage of the nonnull
attribute.
The current conclusion is that it's best to disable this. This is explained
at length in the gdbsupport/common-defs.h comment.
Tested by building with trunk gcc.
gdb/ChangeLog:
2021-07-29 Tom de Vries <tdevries@suse.de>
* gdbsupport/common-defs.h (ATTRIBUTE_NONNULL): Disable.
-rw-r--r-- | gdbsupport/common-defs.h | 75 |
1 files changed, 75 insertions, 0 deletions
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h index 5b64401..5678076 100644 --- a/gdbsupport/common-defs.h +++ b/gdbsupport/common-defs.h @@ -110,6 +110,81 @@ #undef ATTRIBUTE_PRINTF #define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD +/* This is defined by ansidecl.h, but we disable the attribute. + + Say a developer starts out with: + ... + extern void foo (void *ptr) __atttribute__((nonnull (1))); + void foo (void *ptr) {} + ... + with the idea in mind to catch: + ... + foo (nullptr); + ... + at compile time with -Werror=nonnull, and then adds: + ... + void foo (void *ptr) { + + gdb_assert (ptr != nullptr); + } + ... + to catch: + ... + foo (variable_with_nullptr_value); + ... + at runtime as well. + + Said developer then verifies that the assert works (using -O0), and commits + the code. + + Some other developer then checks out the code and accidentally writes some + variant of: + ... + foo (variable_with_nullptr_value); + ... + and builds with -O2, and ... the assert doesn't trigger, because it's + optimized away by gcc. + + There's no suppported recipe to prevent the assertion from being optimized + away (other than: build with -O0, or remove the nonnull attribute). Note + that -fno-delete-null-pointer-checks does not help. A patch was submitted + to improve gcc documentation to point this out more clearly ( + https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576218.html ). The + patch also mentions a possible workaround that obfuscates the pointer + using: + ... + void foo (void *ptr) { + + asm ("" : "+r"(ptr)); + gdb_assert (ptr != nullptr); + } + ... + but that still requires the developer to manually add this in all cases + where that's necessary. + + A warning was added to detect the situation: -Wnonnull-compare, which does + help in detecting those cases, but each new gcc release may indicate a new + batch of locations that needs fixing, which means we've added a maintenance + burden. + + We could try to deal with the problem more proactively by introducing a + gdb_assert variant like: + ... + void gdb_assert_non_null (void *ptr) { + asm ("" : "+r"(ptr)); + gdb_assert (ptr != nullptr); + } + void foo (void *ptr) { + gdb_assert_nonnull (ptr); + } + ... + and make it a coding style to use it everywhere, but again, maintenance + burden. + + With all these things considered, for now we go with the solution with the + least maintenance burden: disable the attribute, such that we reliably deal + with it everywhere. */ +#undef ATTRIBUTE_NONNULL +#define ATTRIBUTE_NONNULL(m) + #if GCC_VERSION >= 3004 #define ATTRIBUTE_UNUSED_RESULT __attribute__ ((__warn_unused_result__)) #else |