[v2,1/1] gdb: Fix assertion in 'value_primitive_field'

Message ID 20240305113653.2546426-2-stephan.rohr@intel.com
State New
Headers
Series Fix assertion in 'value_primitive_field' |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Rohr, Stephan March 5, 2024, 11:36 a.m. UTC
  From: "Rohr, Stephan" <stephan.rohr@intel.com>

GDB asserts that the data location of a value's field with a dynamic
data location is resolved when fetching the field's value in
'value_primitive_field'.  This assertion was hit because of bogus DWARF
generated by the Intel Fortran compiler.  While that compiler should fix
the DWARF, we prefer GDB to error out here instead, e.g. to allow the
user to continue debugging other parts of the program.
---
 .../locexpr-data-member-dynamic-location.c    |  31 ++++
 .../locexpr-data-member-dynamic-location.exp  | 136 ++++++++++++++++++
 gdb/value.c                                   |   4 +-
 3 files changed, 170 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
  

Comments

Andrew Burgess March 6, 2024, 2:48 p.m. UTC | #1
Stephan Rohr <stephan.rohr@intel.com> writes:

> From: "Rohr, Stephan" <stephan.rohr@intel.com>
>
> GDB asserts that the data location of a value's field with a dynamic
> data location is resolved when fetching the field's value in
> 'value_primitive_field'.  This assertion was hit because of bogus DWARF
> generated by the Intel Fortran compiler.  While that compiler should fix
> the DWARF, we prefer GDB to error out here instead, e.g. to allow the
> user to continue debugging other parts of the program.
> ---
>  .../locexpr-data-member-dynamic-location.c    |  31 ++++
>  .../locexpr-data-member-dynamic-location.exp  | 136 ++++++++++++++++++
>  gdb/value.c                                   |   4 +-
>  3 files changed, 170 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
>
> diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
> new file mode 100644
> index 00000000000..3c3f6910298
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
> @@ -0,0 +1,31 @@
> +/* Copyright 2024 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 container
> +{
> +  int *data;
> +  int size;
> +};
> +
> +int var_a_data[] = {5, 8, 13, 21, 34};
> +struct container var_a = {var_a_data, 5};
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
> new file mode 100644
> index 00000000000..8d13ac04651
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
> @@ -0,0 +1,136 @@
> +# Copyright 2024 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/>.
> +#
> +# Tests that GDB prints an error when fetching a value's (dynamic) field
> +# with unresolved data location.  For a reproducer, we setup the following
> +# struct type:
> +#
> +#   struct container
> +#     {
> +#       int *data;
> +#       int size;
> +#     };
> +#
> +# We use 'DW_AT_upper_bound' to create a non-static field 'data'.  The error
> +# reproduces with invalid DWARF, i.e. when pushing the memory location of an
> +# instance 'var_a' of struct 'container' on the DWARF stack:
> +#
> +#   DW_AT_location {
> +#     DW_OP_addr [gdb_target_symbol var_a]
> +#     DW_OP_deref
> +#     DW_OP_stack_value
> +#   } SPECIAL_expr}
> +
> +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
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +# Make some DWARF for the test.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    cu {} {
> +	DW_TAG_compile_unit {} {
> +	    declare_labels integer_label array_label struct_label var_a_label
> +	    set int_size [get_sizeof "int" 4]
> +	    set addr_size [get_sizeof "void *" 8]
> +
> +	    integer_label: DW_TAG_base_type {
> +		{DW_AT_byte_size $int_size DW_FORM_sdata}
> +		{DW_AT_encoding  @DW_ATE_signed}
> +	    }
> +
> +	    array_label: DW_TAG_array_type {
> +		{DW_AT_type :$integer_label}
> +		{DW_AT_data_location {
> +		    DW_OP_push_object_address
> +		    DW_OP_deref
> +		} SPECIAL_expr}
> +	    } {
> +		DW_TAG_subrange_type {
> +		    {DW_AT_type :$integer_label}
> +		    {DW_AT_upper_bound {
> +			DW_OP_push_object_address
> +			DW_OP_plus_uconst $addr_size
> +			DW_OP_deref_size $int_size
> +			DW_OP_lit1
> +			DW_OP_minus
> +		    } SPECIAL_expr}
> +		}
> +	    }
> +
> +	    struct_label: DW_TAG_structure_type {
> +		{DW_AT_byte_size 12 DW_FORM_udata}
> +	    } {
> +		DW_TAG_member {
> +		    {DW_AT_name "data"}
> +		    {DW_AT_type :${array_label}}
> +		    {DW_AT_data_member_location 0 DW_FORM_udata}
> +		}
> +
> +		DW_TAG_member {
> +		    {DW_AT_name "size"}
> +		    {DW_AT_type :${integer_label}}
> +		    {DW_AT_data_member_location $addr_size DW_FORM_udata}
> +		}
> +	    }
> +
> +	    DW_TAG_variable {
> +		{DW_AT_name var_a_valid_location}
> +		{DW_AT_type :$struct_label}
> +		{DW_AT_location {
> +		    DW_OP_addr [gdb_target_symbol var_a]
> +		} SPECIAL_expr}
> +	    }
> +
> +	    DW_TAG_variable {
> +		{DW_AT_name var_a_invalid_location}
> +		{DW_AT_type :$struct_label}
> +		{DW_AT_location {
> +		    DW_OP_addr [gdb_target_symbol var_a]
> +		    DW_OP_deref
> +		    DW_OP_stack_value
> +		} SPECIAL_expr}
> +	    }
> +	}
> +    }
> +}
> +
> +# Now that we've generated the DWARF debugging info, rebuild our
> +# program using our debug info instead of the info generated by
> +# the compiler.
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_test "print var_a_valid_location" \
> +	 " = \\{data = \\{5, 8, 13, 21, 34\\}, size = 5\\}"
> +
> +gdb_test "print var_a_invalid_location" \
> +	 " = \\{data = <error reading variable: cannot read data.*"

When I tried this test I see this output:

  (gdb) print var_a_valid_location
  $1 = {data = {5, 8, 13, 21, 34}, size = 5}
  (gdb) PASS: gdb.dwarf2/locexpr-data-member-dynamic-location.exp: print var_a_valid_location
  print var_a_invalid_location
  ../../src/gdb/../gdbsupport/array-view.h:191: internal-error: slice: Assertion `start + size <= m_size' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  FAIL: gdb.dwarf2/locexpr-data-member-dynamic-location.exp: print var_a_invalid_location (GDB internal error)
  Resyncing due to internal error.
  0x60958a gdb_internal_backtrace_1
          ../../src/gdb/bt-utils.c:122
  0x60962d _Z22gdb_internal_backtracev
          ../../src/gdb/bt-utils.c:168
  0xf2e67d internal_vproblem
          ../../src/gdb/utils.c:421
  0xf2ea4c _Z15internal_verrorPKciS0_P13__va_list_tag
          ../../src/gdb/utils.c:501
  0x1722a94 _Z18internal_error_locPKciS0_z
          ../../src/gdbsupport/errors.cc:58
  0x438931 _ZNK3gdb10array_viewIKhE5sliceEmm
          ../../src/gdb/../gdbsupport/array-view.h:191
  0x78479a _ZN18dwarf_expr_context12fetch_resultEP4typeS1_lb
          ../../src/gdb/dwarf2/expr.c:1038
  0x784954 _ZN18dwarf_expr_context8evaluateEPKhmbP18dwarf2_per_cu_dataRK14frame_info_ptrPK18property_addr_infoP4typeSB_l
          ../../src/gdb/dwarf2/expr.c:1089
  0x7d61be dwarf2_evaluate_loc_desc_full
          ../../src/gdb/dwarf2/loc.c:1525
  0x7d6396 _Z24dwarf2_evaluate_loc_descP4typeRK14frame_info_ptrPKhmP18dwarf2_per_cu_dataP18dwarf2_per_objfileb
          ../../src/gdb/dwarf2/loc.c:1569
  0x7d932c locexpr_read_variable
          ../../src/gdb/dwarf2/loc.c:3061
  0x8c6e91 _ZNK13language_defn14read_var_valueEP6symbolPK5blockRK14frame_info_ptr
          ../../src/gdb/findvar.c:505
  0x8c78cf _Z14read_var_valueP6symbolPK5blockRK14frame_info_ptr
          ../../src/gdb/findvar.c:715
  0xf4aab8 _Z17value_of_variableP6symbolPK5block
          ../../src/gdb/valops.c:1379
  0x8878ef _Z18evaluate_var_value6nosidePK5blockP6symbol
          ../../src/gdb/eval.c:535
  0x8879e2 _ZN4expr19var_value_operation8evaluateEP4typeP10expression6noside
          ../../src/gdb/eval.c:561
  0x886bd2 _ZN10expression8evaluateEP4type6noside
          ../../src/gdb/eval.c:111
  0xb6aeb4 process_print_command_args
          ../../src/gdb/printcmd.c:1325
  0xb6af58 print_command_1
          ../../src/gdb/printcmd.c:1338
  0xb6b279 print_command
          ../../src/gdb/printcmd.c:1405
  0x67ec70 do_simple_func
          ../../src/gdb/cli/cli-decode.c:95
  0x683c79 _Z8cmd_funcP16cmd_list_elementPKci
          ../../src/gdb/cli/cli-decode.c:2742
  0xe66aa0 _Z15execute_commandPKci
          ../../src/gdb/top.c:571
  0x89152e _Z15command_handlerPKc
          ../../src/gdb/event-top.c:567
  0x891a37 _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
          ../../src/gdb/event-top.c:803
  0xeaac2d tui_command_line_handler
          ../../src/gdb/tui/tui-interp.c:104
  0x890e3e gdb_rl_callback_handler
          ../../src/gdb/event-top.c:259
  0xfdcaf5 rl_callback_read_char
          ../../../src/readline/readline/callback.c:290
  0x890ca6 gdb_rl_callback_read_char_wrapper_noexcept
          ../../src/gdb/event-top.c:195
  0x890d42 gdb_rl_callback_read_char_wrapper
          ../../src/gdb/event-top.c:234
  0xee7387 stdin_event_handler
          ../../src/gdb/ui.c:155
  0x1723a4c handle_file_event
          ../../src/gdbsupport/event-loop.cc:573
  0x1723fe2 gdb_wait_for_event
          ../../src/gdbsupport/event-loop.cc:694
  0x1722ecc _Z16gdb_do_one_eventi
          ../../src/gdbsupport/event-loop.cc:264
  0xa5cae0 start_event_loop
          ../../src/gdb/main.c:401
  0xa5cc41 captured_command_loop
          ../../src/gdb/main.c:465
  0xa5e61f captured_main
          ../../src/gdb/main.c:1339
  0xa5e6b9 _Z8gdb_mainP18captured_main_args
          ../../src/gdb/main.c:1358
  0x4188c1 main
          ../../src/gdb/gdb.c:39

Looking at the assert that triggered you likely need to compile with
'-D_GLIBCXX_DEBUG=1', but it's also worth adding
'-D_GLIBCXX_DEBUG_PEDANTIC=1' too as this will enable even more error
checking within libstdc++.

Thanks,
Andrew




> diff --git a/gdb/value.c b/gdb/value.c
> index a2b2721d183..145e70074a9 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3060,7 +3060,9 @@ value::primitive_field (LONGEST offset, int fieldno, struct type *arg_type)
>  
>        gdb_assert (0 == offset);
>        /* We expect an already resolved data location.  */
> -      gdb_assert (TYPE_DATA_LOCATION (type)->is_constant ());
> +      if (!TYPE_DATA_LOCATION (type)->is_constant ())
> +	error (_("cannot read %s, expected an already resolved data "
> +		 "location."), arg_type->field (fieldno).name ());
>        /* For dynamic data types defer memory allocation
>  	 until we actual access the value.  */
>        v = value::allocate_lazy (type);
> -- 
> 2.34.1
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
new file mode 100644
index 00000000000..3c3f6910298
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.c
@@ -0,0 +1,31 @@ 
+/* Copyright 2024 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 container
+{
+  int *data;
+  int size;
+};
+
+int var_a_data[] = {5, 8, 13, 21, 34};
+struct container var_a = {var_a_data, 5};
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
new file mode 100644
index 00000000000..8d13ac04651
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-data-member-dynamic-location.exp
@@ -0,0 +1,136 @@ 
+# Copyright 2024 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/>.
+#
+# Tests that GDB prints an error when fetching a value's (dynamic) field
+# with unresolved data location.  For a reproducer, we setup the following
+# struct type:
+#
+#   struct container
+#     {
+#       int *data;
+#       int size;
+#     };
+#
+# We use 'DW_AT_upper_bound' to create a non-static field 'data'.  The error
+# reproduces with invalid DWARF, i.e. when pushing the memory location of an
+# instance 'var_a' of struct 'container' on the DWARF stack:
+#
+#   DW_AT_location {
+#     DW_OP_addr [gdb_target_symbol var_a]
+#     DW_OP_deref
+#     DW_OP_stack_value
+#   } SPECIAL_expr}
+
+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
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+	DW_TAG_compile_unit {} {
+	    declare_labels integer_label array_label struct_label var_a_label
+	    set int_size [get_sizeof "int" 4]
+	    set addr_size [get_sizeof "void *" 8]
+
+	    integer_label: DW_TAG_base_type {
+		{DW_AT_byte_size $int_size DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+	    }
+
+	    array_label: DW_TAG_array_type {
+		{DW_AT_type :$integer_label}
+		{DW_AT_data_location {
+		    DW_OP_push_object_address
+		    DW_OP_deref
+		} SPECIAL_expr}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type :$integer_label}
+		    {DW_AT_upper_bound {
+			DW_OP_push_object_address
+			DW_OP_plus_uconst $addr_size
+			DW_OP_deref_size $int_size
+			DW_OP_lit1
+			DW_OP_minus
+		    } SPECIAL_expr}
+		}
+	    }
+
+	    struct_label: DW_TAG_structure_type {
+		{DW_AT_byte_size 12 DW_FORM_udata}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "data"}
+		    {DW_AT_type :${array_label}}
+		    {DW_AT_data_member_location 0 DW_FORM_udata}
+		}
+
+		DW_TAG_member {
+		    {DW_AT_name "size"}
+		    {DW_AT_type :${integer_label}}
+		    {DW_AT_data_member_location $addr_size DW_FORM_udata}
+		}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name var_a_valid_location}
+		{DW_AT_type :$struct_label}
+		{DW_AT_location {
+		    DW_OP_addr [gdb_target_symbol var_a]
+		} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name var_a_invalid_location}
+		{DW_AT_type :$struct_label}
+		{DW_AT_location {
+		    DW_OP_addr [gdb_target_symbol var_a]
+		    DW_OP_deref
+		    DW_OP_stack_value
+		} SPECIAL_expr}
+	    }
+	}
+    }
+}
+
+# Now that we've generated the DWARF debugging info, rebuild our
+# program using our debug info instead of the info generated by
+# the compiler.
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "print var_a_valid_location" \
+	 " = \\{data = \\{5, 8, 13, 21, 34\\}, size = 5\\}"
+
+gdb_test "print var_a_invalid_location" \
+	 " = \\{data = <error reading variable: cannot read data.*"
diff --git a/gdb/value.c b/gdb/value.c
index a2b2721d183..145e70074a9 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3060,7 +3060,9 @@  value::primitive_field (LONGEST offset, int fieldno, struct type *arg_type)
 
       gdb_assert (0 == offset);
       /* We expect an already resolved data location.  */
-      gdb_assert (TYPE_DATA_LOCATION (type)->is_constant ());
+      if (!TYPE_DATA_LOCATION (type)->is_constant ())
+	error (_("cannot read %s, expected an already resolved data "
+		 "location."), arg_type->field (fieldno).name ());
       /* For dynamic data types defer memory allocation
 	 until we actual access the value.  */
       v = value::allocate_lazy (type);