[PATCH v5 15/15] gdb, solib-svr4: support namespaces in DSO iteration

[Markus Metzger <markus.t.metzger@intel.com>]

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.
---
 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.base/non-lazy-array-index.exp         | 18 ++++-
 .../gdb.server/bkpt-other-inferior.exp        | 13 +++-
 6 files changed, 159 insertions(+), 16 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/dlmopen-lib-dep.c

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index ca396acf24f..8d23905235e 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -3316,9 +3316,60 @@ svr4_lp64_fetch_link_map_offsets (void)
 
 struct target_so_ops svr4_so_ops;
 
+/* 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
@@ -3347,10 +3398,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"
 	    }
 	}
 
-- 
2.35.3

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