From fcf8e814206f9f5a0a90b2f509fe12d841fa5163 Mon Sep 17 00:00:00 2001 From: Lancelot SIX Date: Tue, 24 May 2022 13:32:18 +0100 Subject: [PATCH] gdb: Require psymtab before calling quick_functions in objfile 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: . 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 | 13 +++++ 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, 148 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..9da12ff12e0 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -612,6 +612,19 @@ public: this->section_offsets[idx] = offset; } +private: + + /* Ensure that partial symbols have been read and return the "quick" (aka + partial) symbol functions for this symbol reader. */ + const std::forward_list & + qf_require_partial_symbols () + { + this->require_partial_symbols (true); + return qf; + } + +public: + /* The object file's original name as specified by the user, made absolute, and tilde-expanded. However, it is not canonicalized (i.e., it has not been passed through gdb_realpath). 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 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 . */ + +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 . */ + +#include +#include +#include +#include + +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 . + +# 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.30.2