Module Name:    src
Committed By:   hgutch
Date:           Sat Jun 25 17:46:05 UTC 2022

Modified Files:
        src/external/gpl3/gdb/dist/gdb: location.c
        src/external/gpl3/gdb/dist/gdbsupport: common-defs.h

Log Message:
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


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.4 -r1.2 src/external/gpl3/gdb/dist/gdb/location.c
cvs rdiff -u -r1.1.1.1 -r1.2 \
    src/external/gpl3/gdb/dist/gdbsupport/common-defs.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/gpl3/gdb/dist/gdb/location.c
diff -u src/external/gpl3/gdb/dist/gdb/location.c:1.1.1.4 src/external/gpl3/gdb/dist/gdb/location.c:1.2
--- src/external/gpl3/gdb/dist/gdb/location.c:1.1.1.4	Tue Sep 15 01:43:31 2020
+++ src/external/gpl3/gdb/dist/gdb/location.c	Sat Jun 25 17:46:05 2022
@@ -960,12 +960,11 @@ event_location_empty_p (const struct eve
       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;

Index: src/external/gpl3/gdb/dist/gdbsupport/common-defs.h
diff -u src/external/gpl3/gdb/dist/gdbsupport/common-defs.h:1.1.1.1 src/external/gpl3/gdb/dist/gdbsupport/common-defs.h:1.2
--- src/external/gpl3/gdb/dist/gdbsupport/common-defs.h:1.1.1.1	Tue Sep 15 01:43:49 2020
+++ src/external/gpl3/gdb/dist/gdbsupport/common-defs.h	Sat Jun 25 17:46:05 2022
@@ -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

Reply via email to