public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb, solib-svr4: support namespaces in DSO iteration
@ 2022-10-18 13:25 Markus Metzger
  0 siblings, 0 replies; only message in thread
From: Markus Metzger @ 2022-10-18 13:25 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=aebb370bae3f511df8afb68a01a79c54e2066650

commit aebb370bae3f511df8afb68a01a79c54e2066650
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Mon Mar 28 11:20:10 2022 +0200

    gdb, solib-svr4: support namespaces in DSO iteration
    
    When looking up names, GDB needs to stay within one linker namespace to
    find the correct instance in case the same name is provided in more than
    one namespace.
    
    Modify svr4_iterate_over_objfiles_in_search_order() to stay within the
    namespace of the current_objfile argument.  If no current_objfile is
    provided (i.e. it is nullptr), iterate over objfiles in the initial
    namespace.
    
    For objfiles that do not have a corresponding so_list to provide the
    namespace, assume that the objfile was loaded into the initial namespace.
    This would cover the main executable objfile (which is indeed loaded into
    the initial namespace) as well as manually added symbol files.
    
    Expected fails:
    
      - gdb.base/non-lazy-array-index.exp: the expression parser may lookup
        global symbols, which may result in xfers to read auxv for determining
        the debug base as part of svr4_iterate_over_objfiles_in_search_order().
    
      - gdb.server/non-lazy-array-index.exp: symbol lookup may access the
        target to read AUXV in order to determine the debug base for SVR4
        linker namespaces.
    
    Known issues:
    
      - get_symbol_address() and get_msymbol_address() search objfiles for a
        'better' match.  This was introduced by
    
            4b610737f02 Handle copy relocations
    
        to handle copy relocations but it now causes a wrong address to be
        read after symbol lookup actually cound the correct symbol.  This can
        be seen, for example, with gdb.base/dlmopen.exp when compiled with
        clang.
    
      - gnu ifuncs are only looked up in the initial namespace.
    
      - lookup_minimal_symbol() and lookup_minimal_symbol_text() directly
        iterate over objfiles and are not aware of linker namespaces.

Diff:
---
 gdb/solib-svr4.c                                 | 78 +++++++++++++++++++++++-
 gdb/testsuite/gdb.base/dlmopen-lib-dep.c         | 21 +++++++
 gdb/testsuite/gdb.base/dlmopen-lib.c             |  5 +-
 gdb/testsuite/gdb.base/dlmopen.exp               | 40 +++++++++---
 gdb/testsuite/gdb.base/non-lazy-array-index.exp  | 18 +++++-
 gdb/testsuite/gdb.server/bkpt-other-inferior.exp | 13 ++--
 6 files changed, 159 insertions(+), 16 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 295e9b8b69d..6acaf87960b 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -3313,9 +3313,60 @@ svr4_lp64_fetch_link_map_offsets (void)
 }
 \f
 
+/* Return the DSO matching OBJFILE or nullptr if none can be found.  */
+
+static so_list *
+find_solib_for_objfile (struct objfile *objfile)
+{
+  if (objfile == nullptr)
+    return nullptr;
+
+  /* If OBJFILE is a separate debug object file, look for the original
+     object file.  */
+  if (objfile->separate_debug_objfile_backlink != nullptr)
+    objfile = objfile->separate_debug_objfile_backlink;
+
+  for (so_list *so : current_program_space->solibs ())
+    if (so->objfile == objfile)
+      return so;
+
+  return nullptr;
+}
+
+/* Return the address of the r_debug object for the namespace containing
+   SOLIB or zero if it cannot be found.  This may happen when symbol files
+   are added manually, for example, or with the main executable.
+
+   Current callers treat zero as initial namespace so they are doing the
+   right thing for the main executable.  */
+
+static CORE_ADDR
+find_debug_base_for_solib (so_list *solib)
+{
+  if (solib == nullptr)
+    return 0;
+
+  svr4_info *info = get_svr4_info (current_program_space);
+  gdb_assert (info != nullptr);
+  for (const std::pair<CORE_ADDR, so_list *> tuple
+	 : info->solib_lists)
+    {
+      CORE_ADDR debug_base = tuple.first;
+      so_list *solist = tuple.second;
+
+      for (; solist != nullptr; solist = solist->next)
+	if (svr4_same (solib, solist))
+	  return debug_base;
+    }
+
+  return 0;
+}
+
 /* Search order for ELF DSOs linked with -Bsymbolic.  Those DSOs have a
-   different rule for symbol lookup.  The lookup begins here in the DSO, not in
-   the main executable.  */
+   different rule for symbol lookup.  The lookup begins here in the DSO,
+   not in the main executable.  When starting from CURRENT_OBJFILE, we
+   stay in the same namespace as that file.  Otherwise, we only consider
+   the initial namespace.  */
 
 static void
 svr4_iterate_over_objfiles_in_search_order
@@ -3344,10 +3395,33 @@ svr4_iterate_over_objfiles_in_search_order
 	}
     }
 
+  /* The linker namespace to iterate identified by the address of its
+     r_debug object, defaulting to the initial namespace.  */
+  CORE_ADDR initial = elf_locate_base ();
+  so_list *curr_solib = find_solib_for_objfile (current_objfile);
+  CORE_ADDR debug_base = find_debug_base_for_solib (curr_solib);
+  if (debug_base == 0)
+    debug_base = initial;
+
   for (objfile *objfile : current_program_space->objfiles ())
     {
       if (checked_current_objfile && objfile == current_objfile)
 	continue;
+
+      /* Try to determine the namespace into which objfile was loaded.
+
+	 If we fail, e.g. for manually added symbol files or for the main
+	 executable, we assume that they were added to the initial
+	 namespace.  */
+      so_list *solib = find_solib_for_objfile (objfile);
+      CORE_ADDR solib_base = find_debug_base_for_solib (solib);
+      if (solib_base == 0)
+	solib_base = initial;
+
+      /* Ignore objfiles that were added to a different namespace.  */
+      if (solib_base != debug_base)
+	continue;
+
       if (cb (objfile))
 	return;
     }
diff --git a/gdb/testsuite/gdb.base/dlmopen-lib-dep.c b/gdb/testsuite/gdb.base/dlmopen-lib-dep.c
new file mode 100644
index 00000000000..c996d76dcb6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dlmopen-lib-dep.c
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021-2022 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/>.
+
+*/
+
+__attribute__((visibility ("default")))
+int gdb_dlmopen_glob = 1;
diff --git a/gdb/testsuite/gdb.base/dlmopen-lib.c b/gdb/testsuite/gdb.base/dlmopen-lib.c
index 616bf97b4f5..4645bfde2a6 100644
--- a/gdb/testsuite/gdb.base/dlmopen-lib.c
+++ b/gdb/testsuite/gdb.base/dlmopen-lib.c
@@ -17,9 +17,12 @@
 
 */
 
+extern int gdb_dlmopen_glob;
+
 __attribute__((visibility ("default")))
 int
 inc (int n)
 {
-  return n + 1;  /* bp.inc.  */
+  int amount = gdb_dlmopen_glob;
+  return n + amount;  /* bp.inc.  */
 }
diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp
index 8e86d5d5ccc..a80db75f9ac 100644
--- a/gdb/testsuite/gdb.base/dlmopen.exp
+++ b/gdb/testsuite/gdb.base/dlmopen.exp
@@ -32,13 +32,22 @@ set basename_lib dlmopen-lib
 set srcfile_lib $srcdir/$subdir/$basename_lib.c
 set binfile_lib1 [standard_output_file $basename_lib.1.so]
 set binfile_lib2 [standard_output_file $basename_lib.2.so]
+set srcfile_lib_dep $srcdir/$subdir/$basename_lib-dep.c
+set binfile_lib_dep [standard_output_file $basename_lib-dep.so]
 
-if { [gdb_compile_shlib $srcfile_lib $binfile_lib1 {debug}] != "" } {
+if { [gdb_compile_shlib $srcfile_lib_dep $binfile_lib_dep {debug}] != "" } {
     untested "failed to prepare shlib"
     return -1
 }
 
-if { [gdb_compile_shlib $srcfile_lib $binfile_lib2 {debug}] != "" } {
+if { [gdb_compile_shlib $srcfile_lib $binfile_lib1 \
+	  [list debug shlib_load libs=$binfile_lib_dep]] != "" } {
+    untested "failed to prepare shlib"
+    return -1
+}
+
+if { [gdb_compile_shlib $srcfile_lib $binfile_lib2 \
+	  [list debug shlib_load libs=$binfile_lib_dep]] != "" } {
     untested "failed to prepare shlib"
     return -1
 }
@@ -46,7 +55,7 @@ if { [gdb_compile_shlib $srcfile_lib $binfile_lib2 {debug}] != "" } {
 if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
 	  [list additional_flags=-DDSO1_NAME=\"$binfile_lib1\" \
 	       additional_flags=-DDSO2_NAME=\"$binfile_lib2\" \
-	       libs=-ldl debug]] } {
+	       shlib_load debug]] } {
     return -1
 }
 
@@ -73,7 +82,7 @@ proc check_dso_count { dso num } {
 }
 
 # The DSO part of the test.  We run it once per DSO call.
-proc test_dlmopen_one { ndso1 ndso2 } {
+proc test_dlmopen_one { ndso1 ndso2 exp_glob } {
     global srcfile_lib srcfile_lib basename_lib bp_inc
 
     # Try to reach the breakpoint in the dynamically loaded library.
@@ -87,16 +96,31 @@ proc test_dlmopen_one { ndso1 ndso2 } {
     # This might help debugging.
     gdb_test "info breakpoints" ".*"
     gdb_test "print \$pc" ".*"
+
+    # We expect different instances of GDB_DLMOPEN_GLOB per DSO.
+    gdb_test "print amount" "= $exp_glob"
+    gdb_test "print gdb_dlmopen_glob" "= $exp_glob"
+
+    # Modify that DSO's instance, which should leave the others intact.
+    gdb_test "print &gdb_dlmopen_glob" "= .*"
+    gdb_test "print gdb_dlmopen_glob = -1" "= -1"
 }
 
 # The actual test.  We run it twice.
 proc test_dlmopen {} {
     global srcfile basename_lib bp_main
 
-    with_test_prefix "dlmopen 1" { test_dlmopen_one 3 1 }
-    with_test_prefix "dlmopen 2" { test_dlmopen_one 2 1 }
-    with_test_prefix "dlmopen 3" { test_dlmopen_one 1 1 }
-    with_test_prefix "dlmopen 4" { test_dlmopen_one 0 1 }
+    # Note that when loading dlmopen-lib.1.so and dlmopen-lib.2.so into
+    # the same namespace, dlmopen-lib-dep.so is loaded only once, so in
+    # this case, the changes to gdb_dlmopen_glob inside test_dlmopen_one
+    # will actually be visible.
+    #
+    # Hence, we supply the expected value of this variable as argument to
+    # test_dlmopen_one.
+    with_test_prefix "dlmopen 1" { test_dlmopen_one 3 1 1 }
+    with_test_prefix "dlmopen 2" { test_dlmopen_one 2 1 1 }
+    with_test_prefix "dlmopen 3" { test_dlmopen_one 1 1 1 }
+    with_test_prefix "dlmopen 4" { test_dlmopen_one 0 1 -1 }
 
     with_test_prefix "main" {
 	# Try to reach the breakpoint in the dynamically loaded library.
diff --git a/gdb/testsuite/gdb.base/non-lazy-array-index.exp b/gdb/testsuite/gdb.base/non-lazy-array-index.exp
index 6b596eb042a..66686cfdd63 100644
--- a/gdb/testsuite/gdb.base/non-lazy-array-index.exp
+++ b/gdb/testsuite/gdb.base/non-lazy-array-index.exp
@@ -40,6 +40,7 @@ gdb_test_no_output "set debug target 1"
 # To check this we 'set debug target 1' (above), and then look for any
 # xfer_partial calls; there shouldn't be any.
 set saw_memory_access false
+set saw_auxv_parse false
 gdb_test_multiple "p \$.array\[1\]" "" {
     -re "^p \\\$\\.array\\\[1\\\]\r\n" {
 	exp_continue
@@ -48,6 +49,10 @@ gdb_test_multiple "p \$.array\[1\]" "" {
 	set saw_memory_access true
 	exp_continue
     }
+    -re "^->\[^\r\n\]+auxv_parse\[^\r\n\]+\r\n" {
+	set saw_auxv_parse true
+	exp_continue
+    }
     -re "^->\[^\r\n\]+\r\n" {
 	exp_continue
     }
@@ -58,7 +63,18 @@ gdb_test_multiple "p \$.array\[1\]" "" {
 	exp_continue
     }
     -re "^\\\$${decimal} = 2\r\n$gdb_prompt " {
-	gdb_assert { ! $saw_memory_access }
+	if { $saw_memory_access } {
+	    if { $saw_auxv_parse } {
+		# The expression parser may look up global symbols, which
+		# may require reading AUXV in order to determine the debug
+		# base for SVR4 linker namespaces.
+		xfail "$gdb_test_name"
+	    } else {
+		fail "$gdb_test_name"
+	    }
+	} else {
+	    pass "$gdb_test_name"
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
index d01b911d1ed..452c7c93db3 100644
--- a/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
+++ b/gdb/testsuite/gdb.server/bkpt-other-inferior.exp
@@ -78,13 +78,18 @@ foreach inf_sel {1 2} {
 
 	gdb_test_no_output "set debug remote 1"
 
-	set test "set breakpoint"
-	gdb_test_multiple "break -q main" $test {
+	gdb_test_multiple "break -q main" "set breakpoint" {
+	    -re "Sending packet: \\\$qXfer:auxv:read.*$gdb_prompt $" {
+		# Symbol lookup may access the target to read AUXV in
+		# order to determine the debug base for SVR4 linker
+		# namespaces.
+		xfail "$gdb_test_name"
+	    }
 	    -re "Sending packet.*$gdb_prompt $" {
-		fail $test
+		fail "$gdb_test_name"
 	    }
 	    -re "^break -q main\r\nBreakpoint .* at .*$gdb_prompt $" {
-		pass $test
+		pass "$gdb_test_name"
 	    }
 	}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-10-18 13:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-18 13:25 [binutils-gdb] gdb, solib-svr4: support namespaces in DSO iteration Markus Metzger

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).