[PATCH v2 1/2] gdb: Require psymtab before calling quick_functions in objfile

[Lancelot SIX <lancelot.six@amd.com>]

The recent DWARF indexer rewrite introduced a regression when debugging
a forking program.

Here is a way to reproduce the issue (there might be other ways, but one
is enough and this one mimics the situation we encountered).  Consider a
program which forks, and the child loads a shared library and calls a
function in this shared library:

    if (fork () == 0)
      {
        void *solib = dlopen (some_solib, RTLD_NOW);
        void (*foo) () = dlsym (some_solib, "foo");
        foo ();
      }

Suppose that this program is compiled without debug info, but the shared
library it loads has debug info enabled.

When debugging such program with the following options:

  - set detach-on-fork off
  - set follow-fork-mode child

we see something like:

    (gdb) b foo
    Function "foo" not defined.
    Make breakpoint pending on future shared library load? (y or [n]) y
    Breakpoint 1 (foo) pending.
    (gdb) run
    Starting program: a.out
    [Attaching after process 19720 fork to child process 19723]
    [New inferior 2 (process 19723)]
    [Switching to process 19723]

    Thread 2.1 "a.out" hit Breakpoint 1, 0x00007ffff7fc3101 in foo () from .../libfoo.so
    (gdb) list

    Fatal signal: Segmentation fault
    ----- Backtrace -----
    0x55a278f77d76 gdb_internal_backtrace_1
            ../../gdb/bt-utils.c:122
    0x55a278f77f83 _Z22gdb_internal_backtracev
            ../../gdb/bt-utils.c:168
    0x55a27940b83b handle_fatal_signal
            ../../gdb/event-top.c:914
    0x55a27940bbb1 handle_sigsegv
            ../../gdb/event-top.c:987
    0x7effec0343bf ???
            /build/glibc-sMfBJT/glibc-2.31/nptl/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
    0x55a27924c9d3 _ZNKSt15__uniq_ptr_implI18dwarf2_per_cu_data26dwarf2_per_cu_data_deleterE6_M_ptrEv
            /usr/include/c++/9/bits/unique_ptr.h:154
    0x55a279248bc9 _ZNKSt10unique_ptrI18dwarf2_per_cu_data26dwarf2_per_cu_data_deleterE3getEv
            /usr/include/c++/9/bits/unique_ptr.h:361
    0x55a2792ae718 _ZN27dwarf2_base_index_functions23find_last_source_symtabEP7objfile
            ../../gdb/dwarf2/read.c:3164
    0x55a279afb93e _ZN7objfile23find_last_source_symtabEv
            ../../gdb/symfile-debug.c:139
    0x55a279aa3040 _Z20select_source_symtabP6symtab
            ../../gdb/source.c:365
    0x55a279aa22a1 _Z34set_default_source_symtab_and_linev
            ../../gdb/source.c:268
    0x55a27903c44c list_command
            ../../gdb/cli/cli-cmds.c:1185
    0x55a279051233 do_simple_func
            ../../gdb/cli/cli-decode.c:95
    0x55a27905f221 _Z8cmd_funcP16cmd_list_elementPKci
            ../../gdb/cli/cli-decode.c:2514
    0x55a279c3b0ba _Z15execute_commandPKci
            ../../gdb/top.c:660
    0x55a27940a6c3 _Z15command_handlerPKc
            ../../gdb/event-top.c:598
    0x55a27940b032 _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
            ../../gdb/event-top.c:797
    0x55a279caf401 tui_command_line_handler
            ../../gdb/tui/tui-interp.c:278
    0x55a279409098 gdb_rl_callback_handler
            ../../gdb/event-top.c:230
    0x55a279ed5df2 rl_callback_read_char
            ../../../readline/readline/callback.c:281
    0x55a279408bd8 gdb_rl_callback_read_char_wrapper_noexcept
            ../../gdb/event-top.c:188
    0x55a279408de7 gdb_rl_callback_read_char_wrapper
            ../../gdb/event-top.c:205
    0x55a27940a061 _Z19stdin_event_handleriPv
            ../../gdb/event-top.c:525
    0x55a27a23771e handle_file_event
            ../../gdbsupport/event-loop.cc:574
    0x55a27a237f5f gdb_wait_for_event
            ../../gdbsupport/event-loop.cc:700
    0x55a27a235d81 _Z16gdb_do_one_eventv
            ../../gdbsupport/event-loop.cc:237
    0x55a2796c2ef0 start_event_loop
            ../../gdb/main.c:418
    0x55a2796c3217 captured_command_loop
            ../../gdb/main.c:478
    0x55a2796c717b captured_main
            ../../gdb/main.c:1340
    0x55a2796c7217 _Z8gdb_mainP18captured_main_args
            ../../gdb/main.c:1355
    0x55a278d0b381 main
            ../../gdb/gdb.c:32
    ---------------------
    A fatal error internal to GDB has been detected, further
    debugging is not possible.  GDB will now terminate.

    This is a bug, please report it.  For instructions, see:
    <https://www.gnu.org/software/gdb/bugs/>.

The first issue observed is in the message printed when hitting the
breakpoint.  It says that there was a break in the .so file as if there
was no debug info associated with it, but there is.  Later, if we try to
display the source where the execution stopped, we have a segfault.

Note that not having the debug info on the main binary is not strictly
required to encounter some issues, it only is to encounter the segfault.
If the main binary has debug information, GDB shows some source form the
main binary, unrelated to where we stopped.

The core of the issue is that GDB never loads the psymtab for the
library.  It is not loaded when we first see the .so because in case of
detach-on-fork off, follow-fork-mode child, infrun.c sets
child_inf->symfile_flags = SYMFILE_NO_READ to delay the psymtab loading
as much as possible.  If we compare to what was done to handle this
before the new indexer was activated, the psymatb construction for the
shared library was done under
psymbol_functions::expand_symtabs_matching:

    bool
    psymbol_functions::expand_symtabs_matching (...)
    {
        for (partial_symtab *ps : require_partial_symbols (objfile))
        ...
    }

The new indexer's expand_symtabs_matching callback does not have a call
to the objfile's require_partial_symbols, so if the partial symbol table
is not loaded at this point, there is no mechanism to fix this.

Instead of requiring each implementation of the quick_functions to check
that partial symbols have been read, I think it is safer to enforce this
when calling the quick functions.  The general pattern for calling the
quick functions is:

    for (auto *iter : qf)
      iter->the_actual_method_call (...)

This patch proposes to wrap the access of the `qf` field with an accessor
which ensures that partial symbols have been read before iterating:
qf_require_partial_symbols.  All calls to quick functions are updated
except:

- quick_functions::dump
- quick_functions::read_partial_symbols (from
  objfile::require_partial_symbols)
- quick_functions::can_lazily_read_symbols and quick_functions::has_symbols
  (from objfile::has_partial_symbols)

Regression tested on x86_64-gnu-linux.

Change-Id: I39a13a937fdbaae613a5cf68864b021000554546
---
 gdb/objfiles.h                                | 10 ++++
 gdb/symfile-debug.c                           | 30 +++++-----
 ...fork-no-detach-follow-child-dlopen-shlib.c | 23 ++++++++
 .../fork-no-detach-follow-child-dlopen.c      | 40 +++++++++++++
 .../fork-no-detach-follow-child-dlopen.exp    | 57 +++++++++++++++++++
 5 files changed, 145 insertions(+), 15 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen-shlib.c
 create mode 100644 gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.c
 create mode 100644 gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.exp

diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index cb7a1357cfe..9b205c02831 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -487,6 +487,16 @@ struct objfile
     return per_bfd->gdbarch;
   }
 
+  /* Ensure that partial symbols have been read and return the "quick" (aka
+     partial) symbol functions for this symbol reader.  */
+
+  const std::forward_list<quick_symbol_functions_up> &
+    qf_require_partial_symbols ()
+      {
+	this->require_partial_symbols (true);
+	return qf;
+      }
+
   /* Return true if OBJFILE has partial symbols.  */
 
   bool has_partial_symbols ();
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index bbbcbcabfde..2af04433444 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -109,7 +109,7 @@ objfile::has_unexpanded_symtabs ()
 		objfile_debug_name (this));
 
   bool result = false;
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       if (iter->has_unexpanded_symtabs (this))
 	{
@@ -134,7 +134,7 @@ objfile::find_last_source_symtab ()
     gdb_printf (gdb_stdlog, "qf->find_last_source_symtab (%s)\n",
 		objfile_debug_name (this));
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       retval = iter->find_last_source_symtab (this);
       if (retval != nullptr)
@@ -155,7 +155,7 @@ objfile::forget_cached_source_info ()
     gdb_printf (gdb_stdlog, "qf->forget_cached_source_info (%s)\n",
 		objfile_debug_name (this));
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->forget_cached_source_info (this);
 }
 
@@ -202,7 +202,7 @@ objfile::map_symtabs_matching_filename
     return result;
   };
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       if (!iter->expand_symtabs_matching (this,
 					  match_one_filename,
@@ -271,7 +271,7 @@ objfile::lookup_symbol (block_enum kind, const char *name, domain_enum domain)
     return true;
   };
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       if (!iter->expand_symtabs_matching (this,
 					  nullptr,
@@ -302,7 +302,7 @@ objfile::print_stats (bool print_bcache)
     gdb_printf (gdb_stdlog, "qf->print_stats (%s, %d)\n",
 		objfile_debug_name (this), print_bcache);
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->print_stats (this, print_bcache);
 }
 
@@ -328,7 +328,7 @@ objfile::expand_symtabs_for_function (const char *func_name)
   lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
   lookup_name_info lookup_name = base_lookup.make_ignore_params ();
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->expand_symtabs_matching (this,
 				   nullptr,
 				   &lookup_name,
@@ -347,7 +347,7 @@ objfile::expand_all_symtabs ()
     gdb_printf (gdb_stdlog, "qf->expand_all_symtabs (%s)\n",
 		objfile_debug_name (this));
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->expand_all_symtabs (this);
 }
 
@@ -365,7 +365,7 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
     return filename_cmp (basenames ? basename : fullname, filename) == 0;
   };
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->expand_symtabs_matching (this,
 				   file_matcher,
 				   nullptr,
@@ -390,7 +390,7 @@ objfile::expand_matching_symbols
 		domain_name (domain), global,
 		host_address_to_string (ordered_compare));
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->expand_matching_symbols (this, name, domain, global,
 				   ordered_compare);
 }
@@ -417,7 +417,7 @@ objfile::expand_symtabs_matching
 		host_address_to_string (&expansion_notify),
 		search_domain_name (kind));
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     if (!iter->expand_symtabs_matching (this, file_matcher, lookup_name,
 					symbol_matcher, expansion_notify,
 					search_flags, domain, kind))
@@ -442,7 +442,7 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
 		host_address_to_string (section),
 		warn_if_readin);
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       retval = iter->find_pc_sect_compunit_symtab (this, msymbol, pc, section,
 						   warn_if_readin);
@@ -470,7 +470,7 @@ objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
 		objfile_debug_name (this),
 		need_fullname);
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     iter->map_symbol_filenames (this, fun, need_fullname);
 }
 
@@ -484,7 +484,7 @@ objfile::find_compunit_symtab_by_address (CORE_ADDR address)
 		hex_string (address));
 
   struct compunit_symtab *result = NULL;
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       result = iter->find_compunit_symtab_by_address (this, address);
       if (result != nullptr)
@@ -509,7 +509,7 @@ objfile::lookup_global_symbol_language (const char *name,
   enum language result = language_unknown;
   *symbol_found_p = false;
 
-  for (const auto &iter : qf)
+  for (const auto &iter : qf_require_partial_symbols ())
     {
       result = iter->lookup_global_symbol_language (this, name, domain,
 						    symbol_found_p);
diff --git a/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen-shlib.c b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen-shlib.c
new file mode 100644
index 00000000000..c276fef24f3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen-shlib.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 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/>.  */
+
+int add (int a, int b);
+int
+add (int a, int b)
+{
+  return a + b;
+}
diff --git a/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.c b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.c
new file mode 100644
index 00000000000..b366956df03
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.c
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 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/>.  */
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+int
+main (void)
+{
+  pid_t pid = fork ();
+  if (pid == 0)
+    {
+      void *shlib = dlopen (SHLIB_PATH, RTLD_NOW);
+      int (*add) (int, int) = dlsym (shlib, "add");
+
+      return add (-2, 2);
+    }
+
+  int wstatus;
+  if (waitpid (pid, &wstatus, 0) == -1)
+    assert (WIFEXITED (wstatus) && WEXITSTATUS (wstatus) == 0);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.exp b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.exp
new file mode 100644
index 00000000000..e8b61450b47
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fork-no-detach-follow-child-dlopen.exp
@@ -0,0 +1,57 @@
+# Copyright 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/>.
+
+# This test is a regression test for when GDB debugs a program with:
+# - set follow-fork-mode child
+# - set detach-on-fork off
+#
+# If the program forks, and the child loads a shared library (via
+# dlopen for example), GDB should still load the symtab for this objfile.
+# When a breakpoint is hit in this file, GDB should display the location
+# in the source of the shlib, and "list" should display the source where
+# the program stopped.
+
+if { [skip_shlib_tests] } {
+    return 0
+}
+
+standard_testfile .c -shlib.c
+set shlib_path [standard_output_file ${testfile}-lib.so]
+
+if { [gdb_compile_shlib $srcdir/$subdir/$srcfile2 $shlib_path {debug}] != "" } {
+    return
+}
+
+set opts [list shlib_load additional_flags=-DSHLIB_PATH="${shlib_path}"]
+if { [build_executable "failed to prepare" ${testfile} ${srcfile} $opts] } {
+    return
+}
+
+proc do_test {} {
+    clean_restart $::binfile
+    gdb_load_shlib $::shlib_path
+    gdb_test_no_output "set follow-fork-mode child"
+    gdb_test_no_output "set detach-on-fork off"
+
+    runto "add" allow-pending
+
+    # Since we have debug info in the shlib, we should have the file name available.
+    gdb_test "frame" "add \(.*\) at .*$::srcfile2:\[0-9\]+.*"
+
+    # We must also be able to display the source for the current function.
+    gdb_test "list" "return a \\+ b;.*"
+}
+
+do_test
-- 
2.25.1