Toolchain/binutils-gdb
1
0
Fork 0
forked from Imagelibrary/binutils-gdb
Code Pull Requests Activity

[gdb/build] Disable attribute nonnull

Browse Source 
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.
This commit is contained in:
Tom de Vries
2021-07-30 14:07:40 +02:00
parent f681e5867d
commit fb6262e853
1 changed files with 75 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

75
gdbsupport/common-defs.h
View File

@@ -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