Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/external/gpl3/gdb/dist Cherry-picking two upstream commits[1...



details:   https://anonhg.NetBSD.org/src/rev/f2c5423867f7
branches:  trunk
changeset: 368143:f2c5423867f7
user:      hgutch <hgutch%NetBSD.org@localhost>
date:      Sat Jun 25 17:46:05 2022 +0000

description:
Cherry-picking two upstream commits[1,2] to fix tools build under gcc 12
if MKCROSSGDB is set.  Without these, gcc 12 correctly points out that
certain checkr for pointers being NULL are always false and errors out
due to -Werror=nonnull-compare/-Werror=address (implicitly set by -Wall).

Build failure reported by Piyush Sachdeva.

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fb6262e8534e0148a4a424e9e5138159af19faf1;hp=f681e5867de63f1c8ca692023cf86e4c884fdae7
[2] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=da7ee7f9ce2fc8c278a46e0b360d44319a5a1e7a;hp=cb2e519a5e41052a4dd55be4f1c4d818d2e8af9d

diffstat:

 external/gpl3/gdb/dist/gdb/location.c           |  11 +--
 external/gpl3/gdb/dist/gdbsupport/common-defs.h |  75 +++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 6 deletions(-)

diffs (106 lines):

diff -r 7c594a369674 -r f2c5423867f7 external/gpl3/gdb/dist/gdb/location.c
--- a/external/gpl3/gdb/dist/gdb/location.c     Sat Jun 25 16:09:28 2022 +0000
+++ b/external/gpl3/gdb/dist/gdb/location.c     Sat Jun 25 17:46:05 2022 +0000
@@ -960,12 +960,11 @@
       return 0;
 
     case EXPLICIT_LOCATION:
-      return (EL_EXPLICIT (location) == NULL
-             || (EL_EXPLICIT (location)->source_filename == NULL
-                 && EL_EXPLICIT (location)->function_name == NULL
-                 && EL_EXPLICIT (location)->label_name == NULL
-                 && (EL_EXPLICIT (location)->line_offset.sign
-                     == LINE_OFFSET_UNKNOWN)));
+      return (EL_EXPLICIT (location)->source_filename == NULL
+             && EL_EXPLICIT (location)->function_name == NULL
+             && EL_EXPLICIT (location)->label_name == NULL
+             && (EL_EXPLICIT (location)->line_offset.sign
+                 == LINE_OFFSET_UNKNOWN));
 
     case PROBE_LOCATION:
       return EL_PROBE (location) == NULL;
diff -r 7c594a369674 -r f2c5423867f7 external/gpl3/gdb/dist/gdbsupport/common-defs.h
--- a/external/gpl3/gdb/dist/gdbsupport/common-defs.h   Sat Jun 25 16:09:28 2022 +0000
+++ b/external/gpl3/gdb/dist/gdbsupport/common-defs.h   Sat Jun 25 17:46:05 2022 +0000
@@ -110,6 +110,81 @@
 #undef ATTRIBUTE_PRINTF
 #define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF
 
+/* 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



Home | Main Index | Thread Index | Old Index