public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
@ 2021-11-29 15:37 Simon Marchi
  2022-01-26 17:45 ` Simon Marchi
  2022-01-26 19:49 ` Bruno Larsen
  0 siblings, 2 replies; 12+ messages in thread
From: Simon Marchi @ 2021-11-29 15:37 UTC (permalink / raw)
  To: gdb-patches, binutils

From: Simon Marchi <simon.marchi@polymtl.ca>

g++ 11.1.0 has a bug where it will emit a negative
DW_AT_data_member_location in some cases:

    $ cat test.cpp
    #include <memory>

    int
    main()
    {
      std::unique_ptr<int> ptr;
    }
    $ g++ -g test.cpp
    $ llvm-dwarfdump -F a.out
    ...
    0x00000964:       DW_TAG_member
                        DW_AT_name [DW_FORM_strp]   ("_M_head_impl")
                        DW_AT_decl_file [DW_FORM_data1]     ("/usr/include/c++/11.1.0/tuple")
                        DW_AT_decl_line [DW_FORM_data1]     (125)
                        DW_AT_decl_column [DW_FORM_data1]   (0x27)
                        DW_AT_type [DW_FORM_ref4]   (0x0000067a "default_delete<int>")
                        DW_AT_data_member_location [DW_FORM_sdata]  (-1)
    ...

This leads to a GDB crash (when built with ASan, otherwise probably
garbage results), since it tries to read just before (to the left, in
ASan speak) of the value's buffer:

    ==888645==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000c52af at pc 0x7f711b239f4b bp 0x7fff356bd470 sp 0x7fff356bcc18
    READ of size 1 at 0x6020000c52af thread T0
        #0 0x7f711b239f4a in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
        #1 0x555c4977efa1 in value_contents_copy_raw /home/simark/src/binutils-gdb/gdb/value.c:1347
        #2 0x555c497909cd in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3126
        #3 0x555c478f2eaa in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:333
        #4 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #5 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #6 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #10 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #11 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #12 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #13 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
        #14 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
        #15 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
        #16 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #17 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #18 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #19 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #20 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #21 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
        #22 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
        #23 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
        #24 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #25 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #26 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #27 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
        #28 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
        #29 0x555c4760f04c in c_value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:587
        #30 0x555c483ff954 in language_defn::value_print(value*, ui_file*, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:614
        #31 0x555c49759f61 in value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1189
        #32 0x555c48950f70 in print_formatted /home/simark/src/binutils-gdb/gdb/printcmd.c:337
        #33 0x555c48958eda in print_value(value*, value_print_options const&) /home/simark/src/binutils-gdb/gdb/printcmd.c:1258
        #34 0x555c48959891 in print_command_1 /home/simark/src/binutils-gdb/gdb/printcmd.c:1367
        #35 0x555c4895a3df in print_command /home/simark/src/binutils-gdb/gdb/printcmd.c:1458
        #36 0x555c4767f974 in do_simple_func /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:97
        #37 0x555c47692e25 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2475
        #38 0x555c4936107e in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:670
        #39 0x555c485f1bff in catch_command_errors /home/simark/src/binutils-gdb/gdb/main.c:523
        #40 0x555c485f249c in execute_cmdargs /home/simark/src/binutils-gdb/gdb/main.c:618
        #41 0x555c485f6677 in captured_main_1 /home/simark/src/binutils-gdb/gdb/main.c:1317
        #42 0x555c485f6c83 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1338
        #43 0x555c485f6d65 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1363
        #44 0x555c46e41ba8 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
        #45 0x7f71198bcb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
        #46 0x555c46e4197d in _start (/home/simark/build/binutils-gdb-one-target/gdb/gdb+0x77f197d)

    0x6020000c52af is located 1 bytes to the left of 8-byte region [0x6020000c52b0,0x6020000c52b8)
    allocated by thread T0 here:
        #0 0x7f711b2b7459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x555c470acdc9 in xcalloc /home/simark/src/binutils-gdb/gdb/alloc.c:100
        #2 0x555c49b775cd in xzalloc(unsigned long) /home/simark/src/binutils-gdb/gdbsupport/common-utils.cc:29
        #3 0x555c4977bdeb in allocate_value_contents /home/simark/src/binutils-gdb/gdb/value.c:1029
        #4 0x555c4977be25 in allocate_value(type*) /home/simark/src/binutils-gdb/gdb/value.c:1040
        #5 0x555c4979030d in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3092
        #6 0x555c478f6280 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:501
        #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #10 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #11 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #12 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #13 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #14 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #15 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
        #16 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
        #17 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
        #18 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
        #19 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
        #20 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #21 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #22 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #23 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
        #24 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
        #25 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
        #26 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
        #27 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
        #28 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
        #29 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048

Since there are some binaries with this in the wild, I think it would be
useful for GDB to work around this.  I did the obvious simple thing, if
the DW_AT_data_member_location's value is -1, replace it with 0.  I
didn't add a producer check, because I don't think that a
DW_AT_data_member_location value of -1 is ever legitimate, but we could
certainly add one if needed.  The difficult part would be if GCC 11 ever
emits a legitimate DW_AT_data_member_location value of -1 in other
situations, then we would need to identify when the -1 is legitimate and
when it is not.

Change-Id: Ieef3459b0b9bbce8bdad838ba83b4b64e7269d42
---
 gdb/dwarf2/read.c                             | 10 +++
 .../negative-data-member-location.c           | 29 +++++++
 .../negative-data-member-location.exp         | 76 +++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 737d8a4c81b..0c66a6daf97 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
       if (attr->form_is_constant ())
 	{
 	  LONGEST offset = attr->constant_value (0);
+
+	  /* Work around this GCC 11 bug, where it would erroneously use -1
+	     data member locations, instead of 0:
+
+	       Negative DW_AT_data_member_location
+	       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
+	     */
+	  if (offset == -1)
+	    offset = 0;
+
 	  field->set_loc_bitpos (offset * bits_per_byte);
 	}
       else if (attr->form_is_section_offset ())
diff --git a/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
new file mode 100644
index 00000000000..4871b2f1203
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
@@ -0,0 +1,29 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+struct the_struct
+{
+  char x[4];
+};
+
+struct the_struct s = { { 0x11, 0x22, 0x22, 0x11 } };
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
new file mode 100644
index 00000000000..664c4e47acc
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
@@ -0,0 +1,76 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for
+# DW_AT_data_member_location:
+#
+#   Negative DW_AT_data_member_location
+#   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if ![dwarf2_support] {
+    return 0
+}
+
+standard_testfile .c -dw.S
+
+set asm_file [standard_output_file ${srcfile2}]
+
+Dwarf::assemble ${asm_file} {
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C99}
+	    {name ${::srcfile}}
+        } {
+	    declare_labels int_label struct_label
+
+	    int_label: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_udata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name "int"}
+	    }
+
+	    struct_label: DW_TAG_structure_type {
+		{DW_AT_name "the_struct"}
+		{DW_AT_byte_size 4 DW_FORM_udata}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "field"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location -1 DW_FORM_sdata}
+		}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "s"}
+		{DW_AT_type :$struct_label}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "s"]} SPECIAL_expr}
+	    }
+	}
+    }
+}
+
+if [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $asm_file] {nodebug}] {
+    return
+}
+
+if ![runto_main] then {
+    return
+}
+
+gdb_test "print /x s" " = {field = 0x11222211}"
+gdb_test "print /x s.field" " = 0x11222211"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2021-11-29 15:37 [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug Simon Marchi
@ 2022-01-26 17:45 ` Simon Marchi
  2022-01-26 18:17   ` Keith Seitz
  2022-01-26 19:49 ` Bruno Larsen
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2022-01-26 17:45 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

Ping.

On 2021-11-29 10:37, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> g++ 11.1.0 has a bug where it will emit a negative
> DW_AT_data_member_location in some cases:
> 
>     $ cat test.cpp
>     #include <memory>
> 
>     int
>     main()
>     {
>       std::unique_ptr<int> ptr;
>     }
>     $ g++ -g test.cpp
>     $ llvm-dwarfdump -F a.out
>     ...
>     0x00000964:       DW_TAG_member
>                         DW_AT_name [DW_FORM_strp]   ("_M_head_impl")
>                         DW_AT_decl_file [DW_FORM_data1]     ("/usr/include/c++/11.1.0/tuple")
>                         DW_AT_decl_line [DW_FORM_data1]     (125)
>                         DW_AT_decl_column [DW_FORM_data1]   (0x27)
>                         DW_AT_type [DW_FORM_ref4]   (0x0000067a "default_delete<int>")
>                         DW_AT_data_member_location [DW_FORM_sdata]  (-1)
>     ...
> 
> This leads to a GDB crash (when built with ASan, otherwise probably
> garbage results), since it tries to read just before (to the left, in
> ASan speak) of the value's buffer:
> 
>     ==888645==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000c52af at pc 0x7f711b239f4b bp 0x7fff356bd470 sp 0x7fff356bcc18
>     READ of size 1 at 0x6020000c52af thread T0
>         #0 0x7f711b239f4a in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
>         #1 0x555c4977efa1 in value_contents_copy_raw /home/simark/src/binutils-gdb/gdb/value.c:1347
>         #2 0x555c497909cd in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3126
>         #3 0x555c478f2eaa in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:333
>         #4 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #5 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #6 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #10 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #11 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #12 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #13 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>         #14 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>         #15 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>         #16 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #17 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #18 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #19 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #20 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #21 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>         #22 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>         #23 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>         #24 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #25 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #26 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #27 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>         #28 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>         #29 0x555c4760f04c in c_value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:587
>         #30 0x555c483ff954 in language_defn::value_print(value*, ui_file*, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:614
>         #31 0x555c49759f61 in value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1189
>         #32 0x555c48950f70 in print_formatted /home/simark/src/binutils-gdb/gdb/printcmd.c:337
>         #33 0x555c48958eda in print_value(value*, value_print_options const&) /home/simark/src/binutils-gdb/gdb/printcmd.c:1258
>         #34 0x555c48959891 in print_command_1 /home/simark/src/binutils-gdb/gdb/printcmd.c:1367
>         #35 0x555c4895a3df in print_command /home/simark/src/binutils-gdb/gdb/printcmd.c:1458
>         #36 0x555c4767f974 in do_simple_func /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:97
>         #37 0x555c47692e25 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2475
>         #38 0x555c4936107e in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:670
>         #39 0x555c485f1bff in catch_command_errors /home/simark/src/binutils-gdb/gdb/main.c:523
>         #40 0x555c485f249c in execute_cmdargs /home/simark/src/binutils-gdb/gdb/main.c:618
>         #41 0x555c485f6677 in captured_main_1 /home/simark/src/binutils-gdb/gdb/main.c:1317
>         #42 0x555c485f6c83 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1338
>         #43 0x555c485f6d65 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1363
>         #44 0x555c46e41ba8 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
>         #45 0x7f71198bcb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
>         #46 0x555c46e4197d in _start (/home/simark/build/binutils-gdb-one-target/gdb/gdb+0x77f197d)
> 
>     0x6020000c52af is located 1 bytes to the left of 8-byte region [0x6020000c52b0,0x6020000c52b8)
>     allocated by thread T0 here:
>         #0 0x7f711b2b7459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
>         #1 0x555c470acdc9 in xcalloc /home/simark/src/binutils-gdb/gdb/alloc.c:100
>         #2 0x555c49b775cd in xzalloc(unsigned long) /home/simark/src/binutils-gdb/gdbsupport/common-utils.cc:29
>         #3 0x555c4977bdeb in allocate_value_contents /home/simark/src/binutils-gdb/gdb/value.c:1029
>         #4 0x555c4977be25 in allocate_value(type*) /home/simark/src/binutils-gdb/gdb/value.c:1040
>         #5 0x555c4979030d in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3092
>         #6 0x555c478f6280 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:501
>         #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #10 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #11 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #12 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #13 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #14 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #15 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>         #16 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>         #17 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>         #18 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>         #19 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>         #20 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #21 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #22 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #23 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>         #24 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>         #25 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>         #26 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>         #27 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>         #28 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>         #29 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
> 
> Since there are some binaries with this in the wild, I think it would be
> useful for GDB to work around this.  I did the obvious simple thing, if
> the DW_AT_data_member_location's value is -1, replace it with 0.  I
> didn't add a producer check, because I don't think that a
> DW_AT_data_member_location value of -1 is ever legitimate, but we could
> certainly add one if needed.  The difficult part would be if GCC 11 ever
> emits a legitimate DW_AT_data_member_location value of -1 in other
> situations, then we would need to identify when the -1 is legitimate and
> when it is not.
> 
> Change-Id: Ieef3459b0b9bbce8bdad838ba83b4b64e7269d42
> ---
>  gdb/dwarf2/read.c                             | 10 +++
>  .../negative-data-member-location.c           | 29 +++++++
>  .../negative-data-member-location.exp         | 76 +++++++++++++++++++
>  3 files changed, 115 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 737d8a4c81b..0c66a6daf97 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
>        if (attr->form_is_constant ())
>  	{
>  	  LONGEST offset = attr->constant_value (0);
> +
> +	  /* Work around this GCC 11 bug, where it would erroneously use -1
> +	     data member locations, instead of 0:
> +
> +	       Negative DW_AT_data_member_location
> +	       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
> +	     */
> +	  if (offset == -1)
> +	    offset = 0;
> +
>  	  field->set_loc_bitpos (offset * bits_per_byte);
>  	}
>        else if (attr->form_is_section_offset ())
> diff --git a/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
> new file mode 100644
> index 00000000000..4871b2f1203
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
> @@ -0,0 +1,29 @@
> +/* Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +struct the_struct
> +{
> +  char x[4];
> +};
> +
> +struct the_struct s = { { 0x11, 0x22, 0x22, 0x11 } };
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> new file mode 100644
> index 00000000000..664c4e47acc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> @@ -0,0 +1,76 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for
> +# DW_AT_data_member_location:
> +#
> +#   Negative DW_AT_data_member_location
> +#   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if ![dwarf2_support] {
> +    return 0
> +}
> +
> +standard_testfile .c -dw.S
> +
> +set asm_file [standard_output_file ${srcfile2}]
> +
> +Dwarf::assemble ${asm_file} {
> +    cu {} {
> +	DW_TAG_compile_unit {
> +	    {DW_AT_language @DW_LANG_C99}
> +	    {name ${::srcfile}}
> +        } {
> +	    declare_labels int_label struct_label
> +
> +	    int_label: DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_udata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name "int"}
> +	    }
> +
> +	    struct_label: DW_TAG_structure_type {
> +		{DW_AT_name "the_struct"}
> +		{DW_AT_byte_size 4 DW_FORM_udata}
> +	    } {
> +		DW_TAG_member {
> +		    {DW_AT_name "field"}
> +		    {DW_AT_type :$int_label}
> +		    {DW_AT_data_member_location -1 DW_FORM_sdata}
> +		}
> +	    }
> +
> +	    DW_TAG_variable {
> +		{DW_AT_name "s"}
> +		{DW_AT_type :$struct_label}
> +		{DW_AT_location {DW_OP_addr [gdb_target_symbol "s"]} SPECIAL_expr}
> +	    }
> +	}
> +    }
> +}
> +
> +if [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $asm_file] {nodebug}] {
> +    return
> +}
> +
> +if ![runto_main] then {
> +    return
> +}
> +
> +gdb_test "print /x s" " = {field = 0x11222211}"
> +gdb_test "print /x s.field" " = 0x11222211"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 17:45 ` Simon Marchi
@ 2022-01-26 18:17   ` Keith Seitz
  2022-01-26 18:30     ` Keith Seitz
  2022-01-26 19:18     ` Simon Marchi
  0 siblings, 2 replies; 12+ messages in thread
From: Keith Seitz @ 2022-01-26 18:17 UTC (permalink / raw)
  To: Simon Marchi, Simon Marchi, gdb-patches

On 1/26/22 09:45, Simon Marchi via Gdb-patches wrote:
> Ping.
> 
> On 2021-11-29 10:37, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> Since there are some binaries with this in the wild, I think it would be
>> useful for GDB to work around this.

Re: "in the wild": I checked on several Fedora releases whether this was
a problem, and sure enough, there binaries in Fedora 35 and Rawhide that
do exhibit this bug.

So, thank you for catching this and submitting a fix!

>> I did the obvious simple thing, if the DW_AT_data_member_location's
>> value is -1, replace it with 0.  I didn't add a producer check,
>> because I don't think that a DW_AT_data_member_location value of -1
>> is ever legitimate, but we could certainly add one if needed.

That seems like a reasonable workaround to me.

Aside: One thing I've always wanted was some sort of quasi-automatic
deprecation of these types of bug workarounds... Something even
as simple as a standardized marker in the source that some
nightly/weekly/monthly/quarterly script could search for and
send out a notification to remove (or do it itself). That's a
conversation for another year, though.

>> The difficult part would be if GCC 11 ever emits a legitimate
>> DW_AT_data_member_location value of -1 in other situations, then we
>> would need to identify when the -1 is legitimate and when it is
>> not.
We'll cross that bridge when/if we get to it? ;-)

I just have two tiny nits...

>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 737d8a4c81b..0c66a6daf97 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
>>         if (attr->form_is_constant ())
>>   	{
>>   	  LONGEST offset = attr->constant_value (0);
>> +
>> +	  /* Work around this GCC 11 bug, where it would erroneously use -1
>> +	     data member locations, instead of 0:
>> +
>> +	       Negative DW_AT_data_member_location
>> +	       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>> +	     */
>> +	  if (offset == -1)
>> +	    offset = 0;
>> +

Kevin and I discussed this briefly, and he convinced me that a complaint here might
be useful. What do you think?

member-location.exp
>> new file mode 100644
>> index 00000000000..664c4e47acc
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
>> @@ -0,0 +1,76 @@
>> +# Copyright 2021 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for
>> +# DW_AT_data_member_location:
>> +#
>> +#   Negative DW_AT_data_member_location
>> +#   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>> +
>> +load_lib dwarf.exp
>> +
>> +# This test can only be run on targets which support DWARF-2 and use gas.
>> +if ![dwarf2_support] {
>> +    return 0
>> +}
>> +

Please pardon my Tclish fetish, but since this is a new file (and likely to
be used as a model for future contributions), may I ask that we be more
pedantic about the formatting? I.e., all conditional expressions enclosed in
curly brackets, no use of "then" in "if" statements? Those are so
last-century. [:-)]

I think this is go to go regardless. I recommend you approve your patch.

Keith

>> +standard_testfile .c -dw.S
>> +
>> +set asm_file [standard_output_file ${srcfile2}]
>> +
>> +Dwarf::assemble ${asm_file} {
>> +    cu {} {
>> +	DW_TAG_compile_unit {
>> +	    {DW_AT_language @DW_LANG_C99}
>> +	    {name ${::srcfile}}
>> +        } {
>> +	    declare_labels int_label struct_label
>> +
>> +	    int_label: DW_TAG_base_type {
>> +		{DW_AT_byte_size 4 DW_FORM_udata}
>> +		{DW_AT_encoding @DW_ATE_signed}
>> +		{DW_AT_name "int"}
>> +	    }
>> +
>> +	    struct_label: DW_TAG_structure_type {
>> +		{DW_AT_name "the_struct"}
>> +		{DW_AT_byte_size 4 DW_FORM_udata}
>> +	    } {
>> +		DW_TAG_member {
>> +		    {DW_AT_name "field"}
>> +		    {DW_AT_type :$int_label}
>> +		    {DW_AT_data_member_location -1 DW_FORM_sdata}
>> +		}
>> +	    }
>> +
>> +	    DW_TAG_variable {
>> +		{DW_AT_name "s"}
>> +		{DW_AT_type :$struct_label}
>> +		{DW_AT_location {DW_OP_addr [gdb_target_symbol "s"]} SPECIAL_expr}
>> +	    }
>> +	}
>> +    }
>> +}
>> +
>> +if [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $asm_file] {nodebug}] {
>> +    return
>> +}
>> +
>> +if ![runto_main] then {
>> +    return
>> +}
>> +
>> +gdb_test "print /x s" " = {field = 0x11222211}"
>> +gdb_test "print /x s.field" " = 0x11222211"
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 18:17   ` Keith Seitz
@ 2022-01-26 18:30     ` Keith Seitz
  2022-01-26 19:34       ` Simon Marchi
  2022-01-26 19:18     ` Simon Marchi
  1 sibling, 1 reply; 12+ messages in thread
From: Keith Seitz @ 2022-01-26 18:30 UTC (permalink / raw)
  To: Simon Marchi, Simon Marchi, gdb-patches

On 1/26/22 10:17, Keith Seitz via Gdb-patches wrote:
> On 1/26/22 09:45, Simon Marchi via Gdb-patches wrote:
>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>> index 737d8a4c81b..0c66a6daf97 100644
>>> --- a/gdb/dwarf2/read.c
>>> +++ b/gdb/dwarf2/read.c
>>> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
>>>         if (attr->form_is_constant ())
>>>       {
>>>         LONGEST offset = attr->constant_value (0);
>>> +
>>> +      /* Work around this GCC 11 bug, where it would erroneously use -1
>>> +         data member locations, instead of 0:
>>> +
>>> +           Negative DW_AT_data_member_location
>>> +           https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>>> +         */
>>> +      if (offset == -1)
>>> +        offset = 0;
>>> +

I apologize, I forgot to ask: would the more general " < 0" be
appropriate to catch other related bugs, such as fuzzing?

Keith


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 18:17   ` Keith Seitz
  2022-01-26 18:30     ` Keith Seitz
@ 2022-01-26 19:18     ` Simon Marchi
  2022-01-26 20:23       ` Keith Seitz
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2022-01-26 19:18 UTC (permalink / raw)
  To: Keith Seitz, Simon Marchi, gdb-patches

On 2022-01-26 13:17, Keith Seitz wrote:
>>> The difficult part would be if GCC 11 ever emits a legitimate
>>> DW_AT_data_member_location value of -1 in other situations, then we
>>> would need to identify when the -1 is legitimate and when it is
>>> not.
> We'll cross that bridge when/if we get to it? ;-)
> 
> I just have two tiny nits...
> 
>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>> index 737d8a4c81b..0c66a6daf97 100644
>>> --- a/gdb/dwarf2/read.c
>>> +++ b/gdb/dwarf2/read.c
>>> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
>>>         if (attr->form_is_constant ())
>>>       {
>>>         LONGEST offset = attr->constant_value (0);
>>> +
>>> +      /* Work around this GCC 11 bug, where it would erroneously use -1
>>> +         data member locations, instead of 0:
>>> +
>>> +           Negative DW_AT_data_member_location
>>> +           https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>>> +         */
>>> +      if (offset == -1)
>>> +        offset = 0;
>>> +
> 
> Kevin and I discussed this briefly, and he convinced me that a complaint here might
> be useful. What do you think?

Would that work for you?

	  if (offset == -1)
	    {
	      complaint (_("DW_AT_data_member_location value of -1, assuming 0"));
	      offset = 0;
	    }

> member-location.exp
>>> new file mode 100644
>>> index 00000000000..664c4e47acc
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
>>> @@ -0,0 +1,76 @@
>>> +# Copyright 2021 Free Software Foundation, Inc.
>>> +
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for
>>> +# DW_AT_data_member_location:
>>> +#
>>> +#   Negative DW_AT_data_member_location
>>> +#   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>>> +
>>> +load_lib dwarf.exp
>>> +
>>> +# This test can only be run on targets which support DWARF-2 and use gas.
>>> +if ![dwarf2_support] {
>>> +    return 0
>>> +}
>>> +
> 
> Please pardon my Tclish fetish, but since this is a new file (and likely to
> be used as a model for future contributions), may I ask that we be more
> pedantic about the formatting? I.e., all conditional expressions enclosed in
> curly brackets, no use of "then" in "if" statements? Those are so
> last-century. [:-)]

No problem, I'm all for consistency.

Would you like to list those rules here?

  https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards

It's always good to have something to refer to.

> I think this is go to go regardless. I recommend you approve your patch.

I'll go read your other email.

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 18:30     ` Keith Seitz
@ 2022-01-26 19:34       ` Simon Marchi
  2022-01-26 19:38         ` Kevin Buettner
  2022-01-26 20:18         ` Keith Seitz
  0 siblings, 2 replies; 12+ messages in thread
From: Simon Marchi @ 2022-01-26 19:34 UTC (permalink / raw)
  To: Keith Seitz, Simon Marchi, gdb-patches



On 2022-01-26 13:30, Keith Seitz wrote:
> On 1/26/22 10:17, Keith Seitz via Gdb-patches wrote:
>> On 1/26/22 09:45, Simon Marchi via Gdb-patches wrote:
>>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>>> index 737d8a4c81b..0c66a6daf97 100644
>>>> --- a/gdb/dwarf2/read.c
>>>> +++ b/gdb/dwarf2/read.c
>>>> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
>>>>         if (attr->form_is_constant ())
>>>>       {
>>>>         LONGEST offset = attr->constant_value (0);
>>>> +
>>>> +      /* Work around this GCC 11 bug, where it would erroneously use -1
>>>> +         data member locations, instead of 0:
>>>> +
>>>> +           Negative DW_AT_data_member_location
>>>> +           https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
>>>> +         */
>>>> +      if (offset == -1)
>>>> +        offset = 0;
>>>> +
> 
> I apologize, I forgot to ask: would the more general " < 0" be
> appropriate to catch other related bugs, such as fuzzing?

In this case, we want to handle a very specific known bug by a specific
compiler.  We know that GCC meant 0 and not -1.  But I wouldn't turn any
negative value into 0, because that might not be the value the compiler
intended.  For example, another compiler could put -4 when it meant 4.
If we turned that into 0, we would just add to the confusion.

In fact, I am tempted to add a producer check and only apply the fixup
if the producer is gcc 11.

Since GDB doesn't know how to handle negative data member offsets (if
that's even possible), I think that if we encounter a negative offset
(other than the case above), we should just emit a complaint and leave
the field's location as unknown.

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 19:34       ` Simon Marchi
@ 2022-01-26 19:38         ` Kevin Buettner
  2022-01-26 19:40           ` Simon Marchi
  2022-01-26 20:18         ` Keith Seitz
  1 sibling, 1 reply; 12+ messages in thread
From: Kevin Buettner @ 2022-01-26 19:38 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, Keith Seitz, Simon Marchi

On Wed, 26 Jan 2022 14:34:11 -0500
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:

> In fact, I am tempted to add a producer check and only apply the fixup
> if the producer is gcc 11.
> 
> Since GDB doesn't know how to handle negative data member offsets (if
> that's even possible), I think that if we encounter a negative offset
> (other than the case above), we should just emit a complaint and leave
> the field's location as unknown.

I like both of these ideas.

Kevin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 19:38         ` Kevin Buettner
@ 2022-01-26 19:40           ` Simon Marchi
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2022-01-26 19:40 UTC (permalink / raw)
  To: Kevin Buettner, Simon Marchi via Gdb-patches; +Cc: Keith Seitz, Simon Marchi



On 2022-01-26 14:38, Kevin Buettner wrote:
> On Wed, 26 Jan 2022 14:34:11 -0500
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
>> In fact, I am tempted to add a producer check and only apply the fixup
>> if the producer is gcc 11.
>>
>> Since GDB doesn't know how to handle negative data member offsets (if
>> that's even possible), I think that if we encounter a negative offset
>> (other than the case above), we should just emit a complaint and leave
>> the field's location as unknown.
> 
> I like both of these ideas.

Ok.  I forgot to mention that I don't plan to do that last part as part
of this patch... future work :).

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2021-11-29 15:37 [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug Simon Marchi
  2022-01-26 17:45 ` Simon Marchi
@ 2022-01-26 19:49 ` Bruno Larsen
  2022-01-26 19:58   ` Simon Marchi
  1 sibling, 1 reply; 12+ messages in thread
From: Bruno Larsen @ 2022-01-26 19:49 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches, binutils

On 11/29/21 12:37, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> g++ 11.1.0 has a bug where it will emit a negative
> DW_AT_data_member_location in some cases:
> 
>      $ cat test.cpp
>      #include <memory>
> 
>      int
>      main()
>      {
>        std::unique_ptr<int> ptr;
>      }
>      $ g++ -g test.cpp
>      $ llvm-dwarfdump -F a.out
>      ...
>      0x00000964:       DW_TAG_member
>                          DW_AT_name [DW_FORM_strp]   ("_M_head_impl")
>                          DW_AT_decl_file [DW_FORM_data1]     ("/usr/include/c++/11.1.0/tuple")
>                          DW_AT_decl_line [DW_FORM_data1]     (125)
>                          DW_AT_decl_column [DW_FORM_data1]   (0x27)
>                          DW_AT_type [DW_FORM_ref4]   (0x0000067a "default_delete<int>")
>                          DW_AT_data_member_location [DW_FORM_sdata]  (-1)
>      ...
> 
> This leads to a GDB crash (when built with ASan, otherwise probably
> garbage results), since it tries to read just before (to the left, in
> ASan speak) of the value's buffer:
> 
>      ==888645==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000c52af at pc 0x7f711b239f4b bp 0x7fff356bd470 sp 0x7fff356bcc18
>      READ of size 1 at 0x6020000c52af thread T0
>          #0 0x7f711b239f4a in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
>          #1 0x555c4977efa1 in value_contents_copy_raw /home/simark/src/binutils-gdb/gdb/value.c:1347
>          #2 0x555c497909cd in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3126
>          #3 0x555c478f2eaa in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:333
>          #4 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #5 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #6 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #10 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #11 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #12 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #13 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>          #14 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>          #15 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>          #16 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #17 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #18 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #19 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #20 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #21 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>          #22 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>          #23 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>          #24 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #25 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #26 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #27 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>          #28 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>          #29 0x555c4760f04c in c_value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:587
>          #30 0x555c483ff954 in language_defn::value_print(value*, ui_file*, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:614
>          #31 0x555c49759f61 in value_print(value*, ui_file*, value_print_options const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1189
>          #32 0x555c48950f70 in print_formatted /home/simark/src/binutils-gdb/gdb/printcmd.c:337
>          #33 0x555c48958eda in print_value(value*, value_print_options const&) /home/simark/src/binutils-gdb/gdb/printcmd.c:1258
>          #34 0x555c48959891 in print_command_1 /home/simark/src/binutils-gdb/gdb/printcmd.c:1367
>          #35 0x555c4895a3df in print_command /home/simark/src/binutils-gdb/gdb/printcmd.c:1458
>          #36 0x555c4767f974 in do_simple_func /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:97
>          #37 0x555c47692e25 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2475
>          #38 0x555c4936107e in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:670
>          #39 0x555c485f1bff in catch_command_errors /home/simark/src/binutils-gdb/gdb/main.c:523
>          #40 0x555c485f249c in execute_cmdargs /home/simark/src/binutils-gdb/gdb/main.c:618
>          #41 0x555c485f6677 in captured_main_1 /home/simark/src/binutils-gdb/gdb/main.c:1317
>          #42 0x555c485f6c83 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1338
>          #43 0x555c485f6d65 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1363
>          #44 0x555c46e41ba8 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
>          #45 0x7f71198bcb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
>          #46 0x555c46e4197d in _start (/home/simark/build/binutils-gdb-one-target/gdb/gdb+0x77f197d)
> 
>      0x6020000c52af is located 1 bytes to the left of 8-byte region [0x6020000c52b0,0x6020000c52b8)
>      allocated by thread T0 here:
>          #0 0x7f711b2b7459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
>          #1 0x555c470acdc9 in xcalloc /home/simark/src/binutils-gdb/gdb/alloc.c:100
>          #2 0x555c49b775cd in xzalloc(unsigned long) /home/simark/src/binutils-gdb/gdbsupport/common-utils.cc:29
>          #3 0x555c4977bdeb in allocate_value_contents /home/simark/src/binutils-gdb/gdb/value.c:1029
>          #4 0x555c4977be25 in allocate_value(type*) /home/simark/src/binutils-gdb/gdb/value.c:1040
>          #5 0x555c4979030d in value_primitive_field(value*, long, int, type*) /home/simark/src/binutils-gdb/gdb/value.c:3092
>          #6 0x555c478f6280 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:501
>          #7 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #8 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #9 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #10 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #11 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #12 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #13 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #14 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #15 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>          #16 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>          #17 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>          #18 0x555c478f63b2 in cp_print_value /home/simark/src/binutils-gdb/gdb/cp-valprint.c:513
>          #19 0x555c478f02ca in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:161
>          #20 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #21 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #22 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #23 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
>          #24 0x555c49759b17 in common_val_print(value*, ui_file*, int, value_print_options const*, language_defn const*) /home/simark/src/binutils-gdb/gdb/valprint.c:1151
>          #25 0x555c478f2fcb in cp_print_value_fields(value*, ui_file*, int, value_print_options const*, type**, int) /home/simark/src/binutils-gdb/gdb/cp-valprint.c:335
>          #26 0x555c4760d45f in c_value_print_struct /home/simark/src/binutils-gdb/gdb/c-valprint.c:383
>          #27 0x555c4760df4c in c_value_print_inner(value*, ui_file*, int, value_print_options const*) /home/simark/src/binutils-gdb/gdb/c-valprint.c:438
>          #28 0x555c483ff9a7 in language_defn::value_print_inner(value*, ui_file*, int, value_print_options const*) const /home/simark/src/binutils-gdb/gdb/language.c:632
>          #29 0x555c49758b68 in do_val_print /home/simark/src/binutils-gdb/gdb/valprint.c:1048
> 
> Since there are some binaries with this in the wild, I think it would be
> useful for GDB to work around this.  I did the obvious simple thing, if
> the DW_AT_data_member_location's value is -1, replace it with 0.  I
> didn't add a producer check, because I don't think that a
> DW_AT_data_member_location value of -1 is ever legitimate, but we could
> certainly add one if needed.  The difficult part would be if GCC 11 ever
> emits a legitimate DW_AT_data_member_location value of -1 in other
> situations, then we would need to identify when the -1 is legitimate and
> when it is not.

Fortran allows for arrays with negative stride. I don't know Fortran well enough to know if this would be a problem, it could use a a completely different mechanism. I think maybe a producer check would be a good idea, either to confirm that we don't break fortran, or to avoid the confusion mentioned in the other thread.

> 
> Change-Id: Ieef3459b0b9bbce8bdad838ba83b4b64e7269d42
> ---
>   gdb/dwarf2/read.c                             | 10 +++
>   .../negative-data-member-location.c           | 29 +++++++
>   .../negative-data-member-location.exp         | 76 +++++++++++++++++++
>   3 files changed, 115 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.c
>   create mode 100644 gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> <snip>
> diff --git a/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> new file mode 100644
> index 00000000000..664c4e47acc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp
> @@ -0,0 +1,76 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for
> +# DW_AT_data_member_location:
> +#
> +#   Negative DW_AT_data_member_location
> +#   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if ![dwarf2_support] {
> +    return 0
> +}

Correct me if I'm wrong, but I think Dwarf::assemble requires the compiler to be GCC.

> +
> +standard_testfile .c -dw.S
> +
> +set asm_file [standard_output_file ${srcfile2}]
> +
> +Dwarf::assemble ${asm_file} {
> +    cu {} {
> +	DW_TAG_compile_unit {
> +	    {DW_AT_language @DW_LANG_C99}
> +	    {name ${::srcfile}}
> +        } {
> +	    declare_labels int_label struct_label
> +
> +	    int_label: DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_udata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name "int"}
> +	    }
> +
> +	    struct_label: DW_TAG_structure_type {
> +		{DW_AT_name "the_struct"}
> +		{DW_AT_byte_size 4 DW_FORM_udata}
> +	    } {
> +		DW_TAG_member {
> +		    {DW_AT_name "field"}
> +		    {DW_AT_type :$int_label}
> +		    {DW_AT_data_member_location -1 DW_FORM_sdata}
> +		}
> +	    }
> +
> +	    DW_TAG_variable {
> +		{DW_AT_name "s"}
> +		{DW_AT_type :$struct_label}
> +		{DW_AT_location {DW_OP_addr [gdb_target_symbol "s"]} SPECIAL_expr}
> +	    }
> +	}
> +    }
> +}
> +
> +if [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $asm_file] {nodebug}] {
> +    return
> +}
> +
> +if ![runto_main] then {
> +    return
> +}
> +
> +gdb_test "print /x s" " = {field = 0x11222211}"
> +gdb_test "print /x s.field" " = 0x11222211"


-- 
Cheers!
Bruno Larsen


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 19:49 ` Bruno Larsen
@ 2022-01-26 19:58   ` Simon Marchi
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2022-01-26 19:58 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches, binutils

>> +# This test can only be run on targets which support DWARF-2 and use gas.
>> +if ![dwarf2_support] {
>> +    return 0
>> +}
> 
> Correct me if I'm wrong, but I think Dwarf::assemble requires the compiler to be GCC.

I don't know.  To be honest, I just copied this commetn and check
because we have it in all other tests that use the DWARF assembler.

Simon


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 19:34       ` Simon Marchi
  2022-01-26 19:38         ` Kevin Buettner
@ 2022-01-26 20:18         ` Keith Seitz
  1 sibling, 0 replies; 12+ messages in thread
From: Keith Seitz @ 2022-01-26 20:18 UTC (permalink / raw)
  To: Simon Marchi, Simon Marchi, gdb-patches

On 1/26/22 11:34, Simon Marchi wrote:
> 
> In this case, we want to handle a very specific known bug by a specific
> compiler.  We know that GCC meant 0 and not -1.  But I wouldn't turn any
> negative value into 0, because that might not be the value the compiler
> intended.  For example, another compiler could put -4 when it meant 4.
> If we turned that into 0, we would just add to the confusion.
> 
> In fact, I am tempted to add a producer check and only apply the fixup
> if the producer is gcc 11.
> 
> Since GDB doesn't know how to handle negative data member offsets (if
> that's even possible), I think that if we encounter a negative offset
> (other than the case above), we should just emit a complaint and leave
> the field's location as unknown.

All fair points; thank you for the explanation.

Keith


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug
  2022-01-26 19:18     ` Simon Marchi
@ 2022-01-26 20:23       ` Keith Seitz
  0 siblings, 0 replies; 12+ messages in thread
From: Keith Seitz @ 2022-01-26 20:23 UTC (permalink / raw)
  To: Simon Marchi, Simon Marchi, gdb-patches

On 1/26/22 11:18, Simon Marchi wrote:
> 
> 	  if (offset == -1)
> 	    {
> 	      complaint (_("DW_AT_data_member_location value of -1, assuming 0"));
> 	      offset = 0;
> 	    }

Perfect!

> No problem, I'm all for consistency.
> 
> Would you like to list those rules here?
> 
>    https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards
> 
> It's always good to have something to refer to.

I'll add that to my TODO list. Thanks for the suggestion. There's also the
"Cookbook" article. I'll make sure to update that. [Likely with a link
to the above coding standard doc, which is unmentioned AFAICT]

The template.exp looks fine, so we're set there, I think.

Thanks!

Keith


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-01-26 20:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-29 15:37 [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug Simon Marchi
2022-01-26 17:45 ` Simon Marchi
2022-01-26 18:17   ` Keith Seitz
2022-01-26 18:30     ` Keith Seitz
2022-01-26 19:34       ` Simon Marchi
2022-01-26 19:38         ` Kevin Buettner
2022-01-26 19:40           ` Simon Marchi
2022-01-26 20:18         ` Keith Seitz
2022-01-26 19:18     ` Simon Marchi
2022-01-26 20:23       ` Keith Seitz
2022-01-26 19:49 ` Bruno Larsen
2022-01-26 19:58   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).