aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom de Vries <tdevries@suse.de>2021-07-30 14:07:40 +0200
committerTom de Vries <tdevries@suse.de>2021-07-30 14:07:40 +0200
commitfb6262e8534e0148a4a424e9e5138159af19faf1 (patch)
treea710f96bbfe6f017bf4823ce6047e7a635d88fb4
parentf681e5867de63f1c8ca692023cf86e4c884fdae7 (diff)
downloadgdb-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.h75
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