[PATCH 1/2] gdbtypes.c: Add the case for FIELD_LOC_KIND_DWARF_BLOCK

[PATCH 1/2] gdbtypes.c: Add the case for FIELD_LOC_KIND_DWARF_BLOCK

[Alexandra Hájková]

From: Alexandra Hájková <ahajkova@redhat.com>

The case for FIELD_LOC_KIND_DWARF_BLOCK was missing for
switch TYPE_FIELD_LOC_KIND. Thas caused an internal-error
under some circumstances.

Fixes bug 28030.
---
 gdb/gdbtypes.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 74ad5d6f7fe..8fbc5d3a80b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5579,6 +5579,10 @@ copy_type_recursive (struct objfile *objfile,
 				  xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type,
 								       i)));
 	      break;
+            case FIELD_LOC_KIND_DWARF_BLOCK:
+              SET_FIELD_DWARF_BLOCK (new_type->field (i),
+                                     TYPE_FIELD_DWARF_BLOCK (type, i));
+              break;
 	    default:
 	      internal_error (__FILE__, __LINE__,
 			      _("Unexpected type field location kind: %d"),
-- 
2.26.3

[PATCH 2/2] Test case reproducing PR28030 bug

[Alexandra Hájková]

From: Kevin Buettner <kevinb@redhat.com>

The original reproducer for PR28030 required use of a specific
compiler version - gcc-c++-11.1.1-3.fc34 is mentioned in the PR,
though it seems probable that other gcc versions might also be able to
reproduce the bug as well.  This commit introduces a test case which,
using the DWARF assembler, provides a reproducer which is independent
of the compiler version.  (Well, it'll work with whatever compilers
the DWARF assembler works with.)

To the best of my knowledge, it's also the first test case which uses
the DWARF assembler to provide debug info for a shared object.  That
being the case, I provided more than the usual commentary which should
allow this case to be used as a template when a combo shared
library / DWARF assembler test case is required in the future.

I provide some details regarding the bug in a comment near the
beginning of locexpr-dml.exp.

This problem was difficult to reproduce; I found myself constantly
referring to the backtrace while trying to figure out what (else) I
might be missing while trying to create a reproducer.  Below is a
partial backtrace which I include for posterity.

 #0  internal_error (
    file=0xc50110 "/ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/gdbtypes.c", line=5575,
    fmt=0xc520c0 "Unexpected type field location kind: %d")
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdbsupport/errors.cc:51
 #1  0x00000000006ef0c5 in copy_type_recursive (objfile=0x1635930,
    type=0x274c260, copied_types=0x30bb290)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/gdbtypes.c:5575
 #2  0x00000000006ef382 in copy_type_recursive (objfile=0x1635930,
    type=0x274ca10, copied_types=0x30bb290)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/gdbtypes.c:5602
 #3  0x0000000000a7409a in preserve_one_value (value=0x24269f0,
    objfile=0x1635930, copied_types=0x30bb290)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/value.c:2529
 #4  0x000000000072012a in gdbscm_preserve_values (
    extlang=0xc55720 <extension_language_guile>, objfile=0x1635930,
    copied_types=0x30bb290)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/guile/scm-value.c:94
 #5  0x00000000006a3f82 in preserve_ext_lang_values (objfile=0x1635930,
    copied_types=0x30bb290)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/extension.c:568
 #6  0x0000000000a7428d in preserve_values (objfile=0x1635930)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/value.c:2579
 #7  0x000000000082d514 in objfile::~objfile (this=0x1635930,
    __in_chrg=<optimized out>)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/objfiles.c:549
 #8  0x0000000000831cc8 in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x1654580)
    at /usr/include/c++/11/bits/shared_ptr_base.h:348
 #9  0x00000000004e6617 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x1654580) at /usr/include/c++/11/bits/shared_ptr_base.h:168
 #10 0x00000000004e1d2f in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x190bb88, __in_chrg=<optimized out>)
    at /usr/include/c++/11/bits/shared_ptr_base.h:705
 #11 0x000000000082feee in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x190bb80, __in_chrg=<optimized out>)
    at /usr/include/c++/11/bits/shared_ptr_base.h:1154
 #12 0x000000000082ff0a in std::shared_ptr<objfile>::~shared_ptr (
    this=0x190bb80, __in_chrg=<optimized out>)
    at /usr/include/c++/11/bits/shared_ptr.h:122
 #13 0x000000000085ed7e in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x114bc00,
    __p=0x190bb80) at /usr/include/c++/11/ext/new_allocator.h:168
 #14 0x000000000085e88d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=...,
    __p=0x190bb80) at /usr/include/c++/11/bits/alloc_traits.h:531
 #15 0x000000000085e50c in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x114bc00, __position=
  std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x1635930})
    at /usr/include/c++/11/bits/stl_list.h:1925
 #16 0x000000000085df0e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x114bc00, __position=
  std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x1635930})
    at /usr/include/c++/11/bits/list.tcc:158
 #17 0x000000000085c748 in program_space::remove_objfile (this=0x114bbc0,
    objfile=0x1635930)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/progspace.c:210
 #18 0x000000000082d3ae in objfile::unlink (this=0x1635930)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/objfiles.c:487
 #19 0x000000000082e68c in objfile_purge_solibs ()
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/objfiles.c:875
 #20 0x000000000092dd37 in no_shared_libraries (ignored=0x0, from_tty=1)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/solib.c:1236
 #21 0x00000000009a37fe in target_pre_inferior (from_tty=1)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/target.c:2496
 #22 0x00000000007454d6 in run_command_1 (args=0x0, from_tty=1,
    run_how=RUN_NORMAL)
    at /ironwood1/sourceware-git/f34-pr28030/bld/../../worktree-pr28030/gdb/infcmd.c:437

I'll note a few points regarding this backtrace:

Frame #1 is where the internal error occurs.  It's caused by an
unhandled case for FIELD_LOC_KIND_DWARF_BLOCK.  The fix for this bug
adds support for this case.

Frame #22 - it's a partial backtrace - shows that GDB is attempting to
(re)run the program.  You can see the exact command sequence that was
used for reproducing this problem in the PR (at
https://sourceware.org/bugzilla/show_bug.cgi?id=28030), but in a
nutshell, after starting the program and advancing to the appropriate
source line, GDB was asked to step into libstdc++; a "finish" command
was issued, returning a value.  The fact that a value was returned is
very important.  GDB was then used to step back into libstdc++.  A
breakpoint was set on a source line in the library after which a "run"
command was issued.

Frame #19 shows a call to objfile_purge_solibs.  It's aptly named.

Frame #7 is a call to the destructor for one of the objfile solibs; it
turned out to be the one for libstdc++.

Frames #6 thru #3 show various value preservation frames.  If you look
at preserve_values() in gdb/value.c, the value history is preserved
first, followed by internal variables, followed by values for the
extension languages (python and guile).  I found it strange that the
bug did not manifest while preserving the value history nor while
preserving python's values, but only presented itself while (finally)
preserving guile's values.  I didn't pursue this puzzlement far
enough to find out why this was the case.
---
 gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c  |  48 +++
 gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c |  27 ++
 gdb/testsuite/gdb.dwarf2/locexpr-dml.exp    | 360 ++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/locexpr-dml.h      |  30 ++
 4 files changed, 465 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-dml.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/locexpr-dml.h

diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c b/gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c
new file mode 100644
index 00000000000..697676ef262
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c
@@ -0,0 +1,48 @@
+/* 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/>.  */
+
+#include "locexpr-dml.h"
+
+struct A g_A = {3, 4};
+struct B g_B = { {8, 9}, 10, 11 };
+
+B *
+foo ()
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  return &g_B;					/* foo return */
+}						/* foo end */
+
+B *
+bar (B *v)
+{						/* bar prologue */
+  asm ("bar_label: .globl bar_label");
+  return v;					/* bar return */
+}						/* bar end */
+
+/* Some of the DWARF assembler procs (e.g. function_range) compile
+   this file, expecting it to be a complete program with a main()
+   function.  When IS_SHAREDLIB is NOT defined, we have main() as
+   defined below.  */
+
+#ifndef IS_SHAREDLIB
+int
+main ()
+{
+  B *b = foo ();
+}
+#endif
diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c b/gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c
new file mode 100644
index 00000000000..59118e3bda1
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c
@@ -0,0 +1,27 @@
+/* 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/>.  */
+
+#include "locexpr-dml.h"
+
+int
+main (void)
+{
+  B *v1;
+  v1 = bar (foo ());
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-dml.exp b/gdb/testsuite/gdb.dwarf2/locexpr-dml.exp
new file mode 100644
index 00000000000..2a58799aab2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-dml.exp
@@ -0,0 +1,360 @@
+# 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/>.
+
+# This test case uses the DWARF assembler to reproduce the problem
+# described by PR28030.  The bug turned out to be that
+# FIELD_LOC_KIND_DWARF_BLOCK was not handled when recursively copying
+# a value's type when preserving the value history during the freeing
+# up of objfiles associated with a shared object.  (Yes, figuring out
+# how to make this happen in a concise test case turned out to be
+# challenging.)
+#
+# The following elements proved to be necessary for reproducing the
+# problem:
+#
+# 1) A location expression needed to be used with
+#    DW_AT_data_member_location rather than a simple offset.
+#    Moreover, this location expression needed to use opcodes
+#    which GDB's DWARF reader could not convert to a simple
+#    offset.  (Note, however, that GDB could probably be improved
+#    to handle the opcodes chosen for this test.)
+#
+# 2) The debug info containing the above DWARF info needed
+#    to be associated with a shared object since the problem
+#    occurred while GDB was preserving values during the
+#    purging of shared objects.
+#
+# 3) After performing some simple gdb commands, the program is
+#    run again.  In the course of running the objfile destructor
+#    associated with the shared object, values are preserved
+#    along with their types.  As noted earlier, it was during
+#    the recursive type copy that the bug was observed.
+#
+# Therefore, due to #2 above, this test case creates debug info
+# which is then used by a shared object.
+
+# This test can't be run on targets lacking shared library support.
+if [skip_shlib_tests] {
+    return 0
+}
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if ![dwarf2_support] {
+    return 0
+}
+
+# gdb_test_file_name is the name of this file without the .exp
+# extension.  Use it to form basenames for the main program
+# and shared object.
+global gdb_test_file_name
+set main_basename ${gdb_test_file_name}-main
+set lib_basename ${gdb_test_file_name}-lib
+
+# We're generating DWARF assembly for the shared object; therefore,
+# the source file for the library / shared object must be listed first
+# (in the standard_testfile invocation) since ${srcfile} is used by
+# get_func_info (for determining the start, end, and length of a
+# function).
+#
+# The output of Dwarf::assemble will be placed in $lib_basename.S
+# which will be ${srcfile3} after the execution of standard_testfile.
+
+standard_testfile $lib_basename.c $main_basename.c $lib_basename.S
+
+set libsrc "${srcdir}/${subdir}/${srcfile}"
+set lib_so [standard_output_file ${lib_basename}.so]
+set asm_file [standard_output_file ${srcfile3}]
+
+# We need to know the size of some types in order to write some of the
+# debugging info that we're about to generate.  For that, we ask GDB
+# by debugging the program / shared object associated with this test
+# case.
+
+# Compile the shared library: -DIS_SHAREDLIB prevents main() from
+# being defined.  Note that debugging symbols will be present for
+# this compilation.
+if {[gdb_compile_shlib $libsrc $lib_so \
+                       {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
+    untested "failed to compile shared library"
+    return
+}
+
+# Compile and start GDB on the main program, again with debugging
+# symbols.  Note the use of "shlib=" which indicates that our shared
+# library should be used.
+if [prepare_for_testing "failed to prepare" ${testfile} \
+                        ${srcfile2} [list debug shlib=$lib_so]] {
+    return -1
+}
+
+# Do whatever is necessary to make sure that the shared library is
+# loaded for remote targets.
+gdb_load_shlib ${lib_so}
+
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return
+}
+
+# Using our running GDB session, determine sizes of several types.
+set long_size [get_sizeof "long" -1]
+set addr_size [get_sizeof "void *" -1]
+set struct_A_size [get_sizeof "g_A" -1]
+set struct_B_size [get_sizeof "g_B" -1]
+
+if { $long_size == -1 || $addr_size == -1 \
+     || $struct_A_size == -1 || $struct_B_size == -1} {
+    # No point going on if we can't determine type sizes.
+    return
+}
+
+# Retrieve struct offset of MBR in struct TP
+proc get_offsetof { tp mbr } {
+    return [get_integer_valueof "&((${tp} *) 0)->${mbr}" -1]
+}
+
+# Use running GDB session to get struct offsets
+set A_a [get_offsetof A a]
+set A_x [get_offsetof A x]
+set B_a [get_offsetof B a]
+set B_b [get_offsetof B b]
+set B_x2 [get_offsetof B x2]
+
+# Create the DWARF.  
+Dwarf::assemble ${asm_file} {
+    global srcdir subdir srcfile srcfile2
+    global long_size addr_size struct_A_size struct_B_size
+    global A_a A_x B_a B_b B_x2
+    declare_labels L
+
+    # Find start, end, and length of functions foo and bar. 
+    # These calls to get_func_info will create and set variables
+    # foo_start, bar_start, foo_end, bar_end, foo_len, and
+    # bar_len.
+    #
+    # In order to get the right answers, get_func_info (and,
+    # underneath, function_range) should use the same compiler flags
+    # as those used to make a shared object.  For any targets that get
+    # this far, -fpic is probably correct.
+    set flags {additional_flags=-fpic debug}
+    get_func_info foo $flags
+    get_func_info bar $flags
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {name ${srcfile}}
+	    {stmt_list $L DW_FORM_sec_offset}
+        } {
+	    declare_labels int_label class_A_label class_B_label \
+	                   B_ptr_label
+     
+	    int_label: DW_TAG_base_type {
+		{DW_AT_byte_size ${long_size} DW_FORM_udata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name "int"}
+	    }
+
+	    class_A_label: DW_TAG_class_type {
+		{DW_AT_name "A"}
+		{DW_AT_byte_size $struct_A_size DW_FORM_sdata}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "a"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location $A_a DW_FORM_udata}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "x"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location $A_x DW_FORM_udata}
+		}
+	    }
+
+	    class_B_label: DW_TAG_class_type {
+		{DW_AT_name "B"}
+		{DW_AT_byte_size $struct_B_size DW_FORM_sdata}
+	    } {
+		# While there are easier / better ways to specify an
+		# offset used by DW_AT_data_member_location than that
+		# used below, we need a location expression here in
+		# order to reproduce the bug.  Moreover, this location
+		# expression needs to use opcodes that aren't handled
+		# by decode_locdesc() in dwarf2/read.c; if we use
+		# opcodes that _are_ handled by that function, the
+		# location expression will be converted into a simple
+		# offset - which will then (again) not reproduce the
+		# bug.  At the time that this test was written,
+		# neither DW_OP_pick nor DW_OP_drop were being handled
+		# by decode_locdesc(); this is why those opcodes were
+		# chosen.
+		DW_TAG_inheritance {
+		    {DW_AT_type :$class_A_label}
+		    {DW_AT_data_member_location {
+			DW_OP_constu $B_a
+			DW_OP_plus
+			DW_OP_pick 0
+			DW_OP_drop} SPECIAL_expr}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "b"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location $B_b DW_FORM_udata}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "x2"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location $B_x2 DW_FORM_udata}
+		}
+	    }
+
+	    B_ptr_label: DW_TAG_pointer_type {
+		{DW_AT_type :$class_B_label}
+		{DW_AT_byte_size $addr_size DW_FORM_sdata}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "g_A"}
+		{DW_AT_type :$class_A_label}
+		{DW_AT_external 1 flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "g_A"]} \
+		                 SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "g_B"}
+		{DW_AT_type :$class_B_label}
+		{DW_AT_external 1 flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "g_B"]} \
+		                 SPECIAL_expr}
+	    }
+
+	    # We can't use MACRO_AT for the definitions of foo and bar
+	    # because it doesn't provide a way to pass the appropriate
+	    # flags.  Therefore, we list the name, low_pc, and high_pc
+	    # explicitly.
+	    DW_TAG_subprogram {
+		{DW_AT_name foo}
+		{DW_AT_low_pc $foo_start DW_FORM_addr}
+		{DW_AT_high_pc $foo_end DW_FORM_addr}
+		{DW_AT_type :${B_ptr_label}}
+		{DW_AT_external 1 flag}
+	    }
+
+	    DW_TAG_subprogram {
+		{DW_AT_name bar}
+		{DW_AT_low_pc $bar_start DW_FORM_addr}
+		{DW_AT_high_pc $bar_end DW_FORM_addr}
+		{DW_AT_type :${B_ptr_label}}
+		{DW_AT_external 1 flag}
+	    } {
+		DW_TAG_formal_parameter {
+		    {DW_AT_name v}
+		    {DW_AT_type :${B_ptr_label}}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} L {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	# Generate a line table program.
+	program {
+	    {DW_LNE_set_address $foo_start}
+	    {line [gdb_get_line_number "foo prologue"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_label}
+	    {line [gdb_get_line_number "foo return"]}
+	    {DW_LNS_copy}
+	    {line [gdb_get_line_number "foo end"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $foo_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $bar_start}
+	    {line [gdb_get_line_number "bar prologue"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address bar_label}
+	    {line [gdb_get_line_number "bar return"]}
+	    {DW_LNS_copy}
+	    {line [gdb_get_line_number "bar end"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $bar_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+# Compile the shared object again, but this time include / use the
+# DWARF info that we've created above.  Note that (again)
+# -DIS_SHAREDLIB is used to prevent inclusion of main() in the shared
+# object.  Also note the use of the "nodebug" option.  Any debugging
+# information that we need will be provided by the DWARF info created
+# above.
+if {[gdb_compile_shlib [list $libsrc $asm_file] $lib_so \
+                       {additional_flags=-DIS_SHAREDLIB nodebug}] != ""} {
+    untested "failed to compile shared library"
+    return
+}
+
+# Compile the main program for use with the shared object.
+if [prepare_for_testing "failed to prepare" ${testfile} \
+                        ${srcfile2} [list debug shlib=$lib_so]] {
+    return -1
+}
+
+# Do whatever is necessary to make sure that the shared library is
+# loaded for remote targets.
+gdb_load_shlib ${lib_so}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return
+}
+
+# Step into foo so that we can finish out of it.
+gdb_test "step" "foo .. at .* foo end.*" "step into foo"
+
+# Finishing out of foo will create a value that will later need to
+# be preserved when restarting the program.
+global hex
+gdb_test "finish" "= \\(class B \\*\\) $hex .*" "finish out of foo"
+
+# Dereferencing and printing the return value isn't necessary
+# for reproducing the bug, but we should make sure that the
+# return value is what we expect it to be.
+gdb_test "p *$" { = {<A> = {a = 8, x = 9}, b = 10, x2 = 11}} \
+         "dereference return value"
+
+# The original PR28030 reproducer stepped back into the shared object,
+# so we'll do the same here:
+gdb_test "step" "bar \\(.*" "step into bar"
+
+# We don't want a clean restart here since that will be too clean. 
+# The original reproducer for PR28030 set a breakpoint in the shared
+# library and then restarted via "run".  The command below does roughly
+# the same thing.  It's at this step that an internal error would
+# occur for PR28030.
+runto "bar"
diff --git a/gdb/testsuite/gdb.dwarf2/locexpr-dml.h b/gdb/testsuite/gdb.dwarf2/locexpr-dml.h
new file mode 100644
index 00000000000..81cd2b38e85
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/locexpr-dml.h
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+typedef struct A {
+    long a;
+    long x;
+} A;
+
+typedef struct B {
+    A a;
+    long b;
+    long x2;
+} B;
+
+extern B *foo ();
+extern B *bar (B *v);
-- 
2.26.3

Re: [PATCH 1/2] gdbtypes.c: Add the case for FIELD_LOC_KIND_DWARF_BLOCK

[Simon Marchi]

On 2021-08-31 4:34 p.m., Alexandra Hájková via Gdb-patches wrote:
> From: Alexandra Hájková <ahajkova@redhat.com>
> 
> The case for FIELD_LOC_KIND_DWARF_BLOCK was missing for
> switch TYPE_FIELD_LOC_KIND. Thas caused an internal-error
> under some circumstances.
> 
> Fixes bug 28030.

Hi,

Please add the git trailer:

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28030

LGTM with that added.

Simon

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Simon Marchi]

Thanks for the test case.  The comments are very nice, it would be very
hard to understand without them.  They also communicate the intentions
well, that will be useful if someone needs to debug it in the future.

>  gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c  |  48 +++
>  gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c |  27 ++
>  gdb/testsuite/gdb.dwarf2/locexpr-dml.exp    | 360 ++++++++++++++++++++
>  gdb/testsuite/gdb.dwarf2/locexpr-dml.h      |  30 ++

What does "dml" stand for?

> +# 1) A location expression needed to be used with
> +#    DW_AT_data_member_location rather than a simple offset.
> +#    Moreover, this location expression needed to use opcodes
> +#    which GDB's DWARF reader could not convert to a simple
> +#    offset.  (Note, however, that GDB could probably be improved
> +#    to handle the opcodes chosen for this test.)

Just to understand what is in parenthesis here: you are saying that GDB
could be improved to know how to convert the opcodes chosen for this
test to a simple offset, which I understand would make the test not
exercise what we want?

> +#
> +# 2) The debug info containing the above DWARF info needed
> +#    to be associated with a shared object since the problem
> +#    occurred while GDB was preserving values during the
> +#    purging of shared objects.
> +#
> +# 3) After performing some simple gdb commands, the program is
> +#    run again.  In the course of running the objfile destructor
> +#    associated with the shared object, values are preserved
> +#    along with their types.  As noted earlier, it was during
> +#    the recursive type copy that the bug was observed.
> +#
> +# Therefore, due to #2 above, this test case creates debug info
> +# which is then used by a shared object.
> +
> +# This test can't be run on targets lacking shared library support.
> +if [skip_shlib_tests] {
> +    return 0
> +}
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if ![dwarf2_support] {
> +    return 0
> +}
> +
> +# gdb_test_file_name is the name of this file without the .exp
> +# extension.  Use it to form basenames for the main program
> +# and shared object.
> +global gdb_test_file_name
> +set main_basename ${gdb_test_file_name}-main
> +set lib_basename ${gdb_test_file_name}-lib

You don't need to use "global gdb_test_file_name" here, you are already
in the global namespace.

> +# We're generating DWARF assembly for the shared object; therefore,
> +# the source file for the library / shared object must be listed first
> +# (in the standard_testfile invocation) since ${srcfile} is used by
> +# get_func_info (for determining the start, end, and length of a
> +# function).
> +#
> +# The output of Dwarf::assemble will be placed in $lib_basename.S
> +# which will be ${srcfile3} after the execution of standard_testfile.
> +
> +standard_testfile $lib_basename.c $main_basename.c $lib_basename.S
> +
> +set libsrc "${srcdir}/${subdir}/${srcfile}"
> +set lib_so [standard_output_file ${lib_basename}.so]
> +set asm_file [standard_output_file ${srcfile3}]
> +
> +# We need to know the size of some types in order to write some of the
> +# debugging info that we're about to generate.  For that, we ask GDB
> +# by debugging the program / shared object associated with this test
> +# case.
> +
> +# Compile the shared library: -DIS_SHAREDLIB prevents main() from
> +# being defined.  Note that debugging symbols will be present for
> +# this compilation.
> +if {[gdb_compile_shlib $libsrc $lib_so \
> +                       {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
> +    untested "failed to compile shared library"
> +    return
> +}

I don't understand when $libsrc is compiled without -DIS_SHAREDLIB.

> +
> +# Compile and start GDB on the main program, again with debugging
> +# symbols.  Note the use of "shlib=" which indicates that our shared
> +# library should be used.
> +if [prepare_for_testing "failed to prepare" ${testfile} \
> +                        ${srcfile2} [list debug shlib=$lib_so]] {
> +    return -1
> +}
> +
> +# Do whatever is necessary to make sure that the shared library is
> +# loaded for remote targets.
> +gdb_load_shlib ${lib_so}

You could save a few lines / cycles by simply loading the .so in GDB to
get the sizes.  You don't need a running program:


    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.dwarf2/locexpr-dml/locexpr-dml-lib.so -ex "print sizeof(A)"
    Reading symbols from testsuite/outputs/gdb.dwarf2/locexpr-dml/locexpr-dml-lib.so...
    $1 = 16

I tried, and this works:

    if {[gdb_compile_shlib $libsrc $lib_so \
                           {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
        untested "failed to compile shared library"
        return
    }

    clean_restart $lib_so

    # Using our running GDB session, determine sizes of several types.
    set long_size [get_sizeof "long" -1]
    set addr_size [get_sizeof "void *" -1]
    set struct_A_size [get_sizeof "g_A" -1]
    set struct_B_size [get_sizeof "g_B" -1]

> +
> +
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return
> +}
> +
> +# Using our running GDB session, determine sizes of several types.
> +set long_size [get_sizeof "long" -1]
> +set addr_size [get_sizeof "void *" -1]
> +set struct_A_size [get_sizeof "g_A" -1]
> +set struct_B_size [get_sizeof "g_B" -1]
> +
> +if { $long_size == -1 || $addr_size == -1 \
> +     || $struct_A_size == -1 || $struct_B_size == -1} {
> +    # No point going on if we can't determine type sizes.
> +    return

Will we get a FAIL or some other indication that something went wrong if
that happens?

> +}
> +
> +# Retrieve struct offset of MBR in struct TP
> +proc get_offsetof { tp mbr } {
> +    return [get_integer_valueof "&((${tp} *) 0)->${mbr}" -1]
> +}
> +
> +# Use running GDB session to get struct offsets
> +set A_a [get_offsetof A a]
> +set A_x [get_offsetof A x]
> +set B_a [get_offsetof B a]
> +set B_b [get_offsetof B b]
> +set B_x2 [get_offsetof B x2]

I checked, this works too with just loading the .so in GDB and not
running the program.

> +
> +# Create the DWARF.  
> +Dwarf::assemble ${asm_file} {
> +    global srcdir subdir srcfile srcfile2
> +    global long_size addr_size struct_A_size struct_B_size
> +    global A_a A_x B_a B_b B_x2

I now adopted the ${::foo} notation to refer to globals.  It spares some
lines of (possibly unused) global declarations, and I think it makes the
code more readable, since the :: tells you it's a global at the point
the variable is used.  I would encourage you to use it as well.

> +# We don't want a clean restart here since that will be too clean. 
> +# The original reproducer for PR28030 set a breakpoint in the shared
> +# library and then restarted via "run".  The command below does roughly
> +# the same thing.  It's at this step that an internal error would
> +# occur for PR28030.
> +runto "bar"

Since this runto is what tells us if things worked or not, perhaps pass
"message" as an option, so that it produces a PASS:

  runto "bar" message

Simon

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Simon Marchi]

> Frames #6 thru #3 show various value preservation frames.  If you look
> at preserve_values() in gdb/value.c, the value history is preserved
> first, followed by internal variables, followed by values for the
> extension languages (python and guile).  I found it strange that the
> bug did not manifest while preserving the value history nor while
> preserving python's values, but only presented itself while (finally)
> preserving guile's values.  I didn't pursue this puzzlement far
> enough to find out why this was the case.

FWIW, the stack trace I see when running your test does not show Guile
frames.  It's directly:

    #11 0x00005563c222d514 in copy_type_recursive (objfile=0x61300000b2c0, type=0x621001fc4fe0, copied_types=0x621003c9c510) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:5610
    #12 0x00005563c30a04e6 in preserve_one_value (value=0x6110000e6180, objfile=0x61300000b2c0, copied_types=0x621003c9c510) at /home/simark/src/binutils-gdb/gdb/value.c:2531
    #13 0x00005563c30a0990 in preserve_values (objfile=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/value.c:2576
    #14 0x00005563c2763354 in objfile::~objfile (this=0x61300000b2c0, __in_chrg=<optimized out>) at /home/simark/src/binutils-gdb/gdb/objfiles.c:549
    #15 0x00005563c2775609 in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x603000134d40) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:348
    #16 0x00005563c1b50e50 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x603000134d40) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:168
    #17 0x00005563c1b42b62 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x603000134d88, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:702
    #18 0x00005563c276bd32 in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x603000134d80, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:1149
    #19 0x00005563c276bd4e in std::shared_ptr<objfile>::~shared_ptr (this=0x603000134d80, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr.h:122
    #20 0x00005563c285e0b4 in __gnu_cxx::new_allocator<std::__cxx1998::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x61200001d918, __p=0x603000134d80) at /usr/include/c++/11.1.0/ext/new_allocator.h:162
    #21 0x00005563c285ae05 in std::allocator_traits<std::allocator<std::__cxx1998::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x603000134d80) at /usr/include/c++/11.1.0/bits/alloc_traits.h:531
    #22 0x00005563c28605bf in std::__cxx1998::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x61200001d918, __position=<error reading variable: Cannot access memory at address 0x603000134d40>) at /usr/include/c++/11.1.0/bits/stl_list.h:1925
    #23 0x00005563c285ca17 in std::__cxx1998::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x61200001d918, __position=<error reading variable: Cannot access memory at address 0x7ffc409817d0>) at /usr/include/c++/11.1.0/bits/list.tcc:158
    #24 0x00005563c2858cc4 in std::__debug::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x61200001d900, __position=<error reading variable: Cannot access memory at address 0x7ffc40981890>) at /usr/include/c++/11.1.0/debug/list:513
    #25 0x00005563c28566e9 in std::__debug::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x61200001d900, __position=<error reading variable: Cannot access memory at address 0x7ffc40981918>) at /usr/include/c++/11.1.0/debug/list:525
    #26 0x00005563c2850757 in program_space::remove_objfile (this=0x61200001d8c0, objfile=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/progspace.c:210
    #27 0x00005563c2762fba in objfile::unlink (this=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/objfiles.c:487
    #28 0x00005563c2766fd5 in objfile_purge_solibs () at /home/simark/src/binutils-gdb/gdb/objfiles.c:875
    #29 0x00005563c2bff11e in no_shared_libraries (ignored=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib.c:1238

Simon

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Kevin Buettner]

On Wed, 1 Sep 2021 09:35:09 -0400
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:

> Thanks for the test case.  The comments are very nice, it would be very
> hard to understand without them.  They also communicate the intentions
> well, that will be useful if someone needs to debug it in the future.
> 
> >  gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c  |  48 +++
> >  gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c |  27 ++
> >  gdb/testsuite/gdb.dwarf2/locexpr-dml.exp    | 360 ++++++++++++++++++++
> >  gdb/testsuite/gdb.dwarf2/locexpr-dml.h      |  30 ++  
> 
> What does "dml" stand for?

data member location, since the use of DW_AT_data_member_location was
one (of several) key elements needed to reproduce the bug.

I'll make a note of the meaning of dml early in the file. If someone
can think of a better name for this test case, I'll use that instead.

> > +# 1) A location expression needed to be used with
> > +#    DW_AT_data_member_location rather than a simple offset.
> > +#    Moreover, this location expression needed to use opcodes
> > +#    which GDB's DWARF reader could not convert to a simple
> > +#    offset.  (Note, however, that GDB could probably be improved
> > +#    to handle the opcodes chosen for this test.)  
> 
> Just to understand what is in parenthesis here: you are saying that GDB
> could be improved to know how to convert the opcodes chosen for this
> test to a simple offset, which I understand would make the test not
> exercise what we want?

That's correct.  The comment in the test case explains it - see
the part about DW_OP_pick and DW_OP_drop.

	# While there are easier / better ways to specify an
	# offset used by DW_AT_data_member_location than that
	# used below, we need a location expression here in
	# order to reproduce the bug.  Moreover, this location
	# expression needs to use opcodes that aren't handled
	# by decode_locdesc() in dwarf2/read.c; if we use
	# opcodes that _are_ handled by that function, the
	# location expression will be converted into a simple
	# offset - which will then (again) not reproduce the
	# bug.  At the time that this test was written,
	# neither DW_OP_pick nor DW_OP_drop were being handled
	# by decode_locdesc(); this is why those opcodes were
	# chosen.

[...]

> > +# Compile the shared library: -DIS_SHAREDLIB prevents main() from
> > +# being defined.  Note that debugging symbols will be present for
> > +# this compilation.
> > +if {[gdb_compile_shlib $libsrc $lib_so \
> > +                       {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
> > +    untested "failed to compile shared library"
> > +    return
> > +}  
> 
> I don't understand when $libsrc is compiled without -DIS_SHAREDLIB.

I call get_func_info in this test case.  It calls function_range. 
which in turn calls gdb_compile.  It's compiled without -DIS_SHAREDLIB
whenever get_func_info is called.

I'll find some place to mention this fact in one of the comments.

> > +
> > +# Compile and start GDB on the main program, again with debugging
> > +# symbols.  Note the use of "shlib=" which indicates that our shared
> > +# library should be used.
> > +if [prepare_for_testing "failed to prepare" ${testfile} \
> > +                        ${srcfile2} [list debug shlib=$lib_so]] {
> > +    return -1
> > +}
> > +
> > +# Do whatever is necessary to make sure that the shared library is
> > +# loaded for remote targets.
> > +gdb_load_shlib ${lib_so}  
> 
> You could save a few lines / cycles by simply loading the .so in GDB to
> get the sizes.  You don't need a running program:
> 
> 
>     $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.dwarf2/locexpr-dml/locexpr-dml-lib.so -ex "print sizeof(A)"
>     Reading symbols from testsuite/outputs/gdb.dwarf2/locexpr-dml/locexpr-dml-lib.so...
>     $1 = 16
> 
> I tried, and this works:
> 
>     if {[gdb_compile_shlib $libsrc $lib_so \
>                            {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
>         untested "failed to compile shared library"
>         return
>     }
> 
>     clean_restart $lib_so
> 
>     # Using our running GDB session, determine sizes of several types.
>     set long_size [get_sizeof "long" -1]
>     set addr_size [get_sizeof "void *" -1]
>     set struct_A_size [get_sizeof "g_A" -1]
>     set struct_B_size [get_sizeof "g_B" -1]

I'll make those changes.

> > +
> > +
> > +if ![runto_main] then {
> > +    fail "can't run to main"
> > +    return
> > +}
> > +
> > +# Using our running GDB session, determine sizes of several types.
> > +set long_size [get_sizeof "long" -1]
> > +set addr_size [get_sizeof "void *" -1]
> > +set struct_A_size [get_sizeof "g_A" -1]
> > +set struct_B_size [get_sizeof "g_B" -1]
> > +
> > +if { $long_size == -1 || $addr_size == -1 \
> > +     || $struct_A_size == -1 || $struct_B_size == -1} {
> > +    # No point going on if we can't determine type sizes.
> > +    return  
> 
> Will we get a FAIL or some other indication that something went wrong if
> that happens?

I'll add an error message.

> > +}
> > +
> > +# Retrieve struct offset of MBR in struct TP
> > +proc get_offsetof { tp mbr } {
> > +    return [get_integer_valueof "&((${tp} *) 0)->${mbr}" -1]
> > +}
> > +
> > +# Use running GDB session to get struct offsets
> > +set A_a [get_offsetof A a]
> > +set A_x [get_offsetof A x]
> > +set B_a [get_offsetof B a]
> > +set B_b [get_offsetof B b]
> > +set B_x2 [get_offsetof B x2]  
> 
> I checked, this works too with just loading the .so in GDB and not
> running the program.

Okay.

> > +
> > +# Create the DWARF.  
> > +Dwarf::assemble ${asm_file} {
> > +    global srcdir subdir srcfile srcfile2
> > +    global long_size addr_size struct_A_size struct_B_size
> > +    global A_a A_x B_a B_b B_x2  
> 
> I now adopted the ${::foo} notation to refer to globals.  It spares some
> lines of (possibly unused) global declarations, and I think it makes the
> code more readable, since the :: tells you it's a global at the point
> the variable is used.  I would encourage you to use it as well.

Cool.  I'll do that.

> > +# We don't want a clean restart here since that will be too clean. 
> > +# The original reproducer for PR28030 set a breakpoint in the shared
> > +# library and then restarted via "run".  The command below does roughly
> > +# the same thing.  It's at this step that an internal error would
> > +# occur for PR28030.
> > +runto "bar"  
> 
> Since this runto is what tells us if things worked or not, perhaps pass
> "message" as an option, so that it produces a PASS:
> 
>   runto "bar" message

Sounds good.  I'll make that change too.

Thanks,

Kevin

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Kevin Buettner]

On Wed, 1 Sep 2021 09:37:50 -0400
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:

> > Frames #6 thru #3 show various value preservation frames.  If you look
> > at preserve_values() in gdb/value.c, the value history is preserved
> > first, followed by internal variables, followed by values for the
> > extension languages (python and guile).  I found it strange that the
> > bug did not manifest while preserving the value history nor while
> > preserving python's values, but only presented itself while (finally)
> > preserving guile's values.  I didn't pursue this puzzlement far
> > enough to find out why this was the case.  
> 
> FWIW, the stack trace I see when running your test does not show Guile
> frames.  It's directly:
> 
>     #11 0x00005563c222d514 in copy_type_recursive (objfile=0x61300000b2c0, type=0x621001fc4fe0, copied_types=0x621003c9c510) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:5610
>     #12 0x00005563c30a04e6 in preserve_one_value (value=0x6110000e6180, objfile=0x61300000b2c0, copied_types=0x621003c9c510) at /home/simark/src/binutils-gdb/gdb/value.c:2531
>     #13 0x00005563c30a0990 in preserve_values (objfile=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/value.c:2576
>     #14 0x00005563c2763354 in objfile::~objfile (this=0x61300000b2c0, __in_chrg=<optimized out>) at /home/simark/src/binutils-gdb/gdb/objfiles.c:549
>     #15 0x00005563c2775609 in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x603000134d40) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:348
>     #16 0x00005563c1b50e50 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x603000134d40) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:168
>     #17 0x00005563c1b42b62 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x603000134d88, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:702
>     #18 0x00005563c276bd32 in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x603000134d80, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr_base.h:1149
>     #19 0x00005563c276bd4e in std::shared_ptr<objfile>::~shared_ptr (this=0x603000134d80, __in_chrg=<optimized out>) at /usr/include/c++/11.1.0/bits/shared_ptr.h:122
>     #20 0x00005563c285e0b4 in __gnu_cxx::new_allocator<std::__cxx1998::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x61200001d918, __p=0x603000134d80) at /usr/include/c++/11.1.0/ext/new_allocator.h:162
>     #21 0x00005563c285ae05 in std::allocator_traits<std::allocator<std::__cxx1998::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x603000134d80) at /usr/include/c++/11.1.0/bits/alloc_traits.h:531
>     #22 0x00005563c28605bf in std::__cxx1998::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x61200001d918, __position=<error reading variable: Cannot access memory at address 0x603000134d40>) at /usr/include/c++/11.1.0/bits/stl_list.h:1925
>     #23 0x00005563c285ca17 in std::__cxx1998::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x61200001d918, __position=<error reading variable: Cannot access memory at address 0x7ffc409817d0>) at /usr/include/c++/11.1.0/bits/list.tcc:158
>     #24 0x00005563c2858cc4 in std::__debug::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x61200001d900, __position=<error reading variable: Cannot access memory at address 0x7ffc40981890>) at /usr/include/c++/11.1.0/debug/list:513
>     #25 0x00005563c28566e9 in std::__debug::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x61200001d900, __position=<error reading variable: Cannot access memory at address 0x7ffc40981918>) at /usr/include/c++/11.1.0/debug/list:525
>     #26 0x00005563c2850757 in program_space::remove_objfile (this=0x61200001d8c0, objfile=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/progspace.c:210
>     #27 0x00005563c2762fba in objfile::unlink (this=0x61300000b2c0) at /home/simark/src/binutils-gdb/gdb/objfiles.c:487
>     #28 0x00005563c2766fd5 in objfile_purge_solibs () at /home/simark/src/binutils-gdb/gdb/objfiles.c:875
>     #29 0x00005563c2bff11e in no_shared_libraries (ignored=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib.c:1238

Interesting.  I'll remove the guile puzzlement part of it from my commit
remarks.

Thanks,

Kevin

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Simon Marchi]

On 2021-09-01 12:20 p.m., Kevin Buettner wrote:
> On Wed, 1 Sep 2021 09:35:09 -0400
> Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
>> Thanks for the test case.  The comments are very nice, it would be very
>> hard to understand without them.  They also communicate the intentions
>> well, that will be useful if someone needs to debug it in the future.
>>
>>>  gdb/testsuite/gdb.dwarf2/locexpr-dml-lib.c  |  48 +++
>>>  gdb/testsuite/gdb.dwarf2/locexpr-dml-main.c |  27 ++
>>>  gdb/testsuite/gdb.dwarf2/locexpr-dml.exp    | 360 ++++++++++++++++++++
>>>  gdb/testsuite/gdb.dwarf2/locexpr-dml.h      |  30 ++  
>>
>> What does "dml" stand for?
> 
> data member location, since the use of DW_AT_data_member_location was
> one (of several) key elements needed to reproduce the bug.
> 
> I'll make a note of the meaning of dml early in the file. If someone
> can think of a better name for this test case, I'll use that instead.

I would just spell it out (locexpr-data-member-location.exp).

>>> +# Compile the shared library: -DIS_SHAREDLIB prevents main() from
>>> +# being defined.  Note that debugging symbols will be present for
>>> +# this compilation.
>>> +if {[gdb_compile_shlib $libsrc $lib_so \
>>> +                       {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
>>> +    untested "failed to compile shared library"
>>> +    return
>>> +}  
>>
>> I don't understand when $libsrc is compiled without -DIS_SHAREDLIB.
> 
> I call get_func_info in this test case.  It calls function_range. 
> which in turn calls gdb_compile.  It's compiled without -DIS_SHAREDLIB
> whenever get_func_info is called.
> 
> I'll find some place to mention this fact in one of the comments.

Ah, I see, thanks.  Hopefully compiling with and without the main
doesn't change the addresses of the functions you are getting info
for.

It would take a bit of cleanup before hand, but ideally the
function_range proc would be able to compile shared libraries, or work
on an already built binary.

Simon

Re: [PATCH 2/2] Test case reproducing PR28030 bug

[Kevin Buettner]

On Wed, 1 Sep 2021 12:32:23 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> >> What does "dml" stand for?  
> > 
> > data member location, since the use of DW_AT_data_member_location was
> > one (of several) key elements needed to reproduce the bug.
> > 
> > I'll make a note of the meaning of dml early in the file. If someone
> > can think of a better name for this test case, I'll use that instead.  
> 
> I would just spell it out (locexpr-data-member-location.exp).

Done.  (The name has been changed in the v2 patch that I sent
out a few minutes ago.)

> >>> +# Compile the shared library: -DIS_SHAREDLIB prevents main() from
> >>> +# being defined.  Note that debugging symbols will be present for
> >>> +# this compilation.
> >>> +if {[gdb_compile_shlib $libsrc $lib_so \
> >>> +                       {additional_flags=-DIS_SHAREDLIB debug}] != ""} {
> >>> +    untested "failed to compile shared library"
> >>> +    return
> >>> +}    
> >>
> >> I don't understand when $libsrc is compiled without -DIS_SHAREDLIB.  
> > 
> > I call get_func_info in this test case.  It calls function_range. 
> > which in turn calls gdb_compile.  It's compiled without -DIS_SHAREDLIB
> > whenever get_func_info is called.
> > 
> > I'll find some place to mention this fact in one of the comments.  
> 
> Ah, I see, thanks.  Hopefully compiling with and without the main
> doesn't change the addresses of the functions you are getting info
> for.

I don't think it matters since the shared object ends up being
relocated.

What does matter is that the relative offsets and lengths of the functions
stay the same.  This definitely changes depending upon whether -fpic is
used or not.  (I found this out the hard way.)

From readelf -w:

 <1><a1>: Abbrev Number: 14 (DW_TAG_subprogram)
    <a2>   DW_AT_name        : foo
    <a6>   DW_AT_low_pc      : 0x10f9
    <ae>   DW_AT_high_pc     : 0x1106
    <b6>   DW_AT_type        : <0x73>
    <ba>   DW_AT_external    : 1
 <1><bb>: Abbrev Number: 15 (DW_TAG_subprogram)
    <bc>   DW_AT_name        : bar
    <c0>   DW_AT_low_pc      : 0x1106
    <c8>   DW_AT_high_pc     : 0x1114
    <d0>   DW_AT_type        : <0x73>
    <d4>   DW_AT_external    : 1

When running gdb:

(gdb) disassemble foo
Dump of assembler code for function foo():
   0x00007ffff7fbf0f9 <+0>:	push   %rbp
   0x00007ffff7fbf0fa <+1>:	mov    %rsp,%rbp
   0x00007ffff7fbf0fd <+4>:	mov    0x2edc(%rip),%rax        # 0x7ffff7fc1fe0
   0x00007ffff7fbf104 <+11>:	pop    %rbp
   0x00007ffff7fbf105 <+12>:	ret    
End of assembler dump.
(gdb) disassemble bar
Dump of assembler code for function bar(B*):
   0x00007ffff7fbf106 <+0>:	push   %rbp
   0x00007ffff7fbf107 <+1>:	mov    %rsp,%rbp
   0x00007ffff7fbf10a <+4>:	mov    %rdi,-0x8(%rbp)
   0x00007ffff7fbf10e <+8>:	mov    -0x8(%rbp),%rax
   0x00007ffff7fbf112 <+12>:	pop    %rbp
   0x00007ffff7fbf113 <+13>:	ret    
End of assembler dump.

This shows that the addresses were relocated, but that the offsets / lengths
are still correct.  (Offsets and lengths were determined with main() in
place.  I'm debugging the shared object which has no main().)

Kevin