From: Simon Marchi Date: Wed, 20 May 2020 19:44:24 +0000 (-0400) Subject: gdb: reset/recompute objfile section offsets in reread_symbols X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=9d428aae67a67087959822b0ffd81f7df96218c7;p=binutils-gdb.git gdb: reset/recompute objfile section offsets in reread_symbols This patch started as an investigation of this bug, where the program is re-compiled between two "start" runs: $ ./gdb -nx --data-directory=data-directory -q a.out Reading symbols from a.out... (gdb) start Temporary breakpoint 1 at 0x1131: file test.c, line 1. Starting program: /home/smarchi/build/wt/test/gdb/a.out Temporary breakpoint 1, main () at test.c:1 1 int main() { return 0; } *** re-compile a.out *** (gdb) start The program being debugged has been started already. Start it from the beginning? (y or n) y `/home/smarchi/build/wt/test/gdb/a.out' has changed; re-reading symbols. Temporary breakpoint 2 at 0x555555555129: file test.c, line 1. Starting program: /home/smarchi/build/wt/test/gdb/a.out warning: Probes-based dynamic linker interface failed. Reverting to original interface. Temporary breakpoint 2, main () at test.c:1 1 int main() { return 0; } (gdb) To reproduce the bug, a.out needs to be a position-independent executable (PIE). Here's what happens: 1) We first read the symbols of a.out. The section offsets in the objfile are all 0, so the symbols are created unrelocated. 2) The breakpoint on main is created, as you can see the breakpoint address (derived from the `main` symbol with value 0x1129) is still unrelocated (0x1131). Since the program is not yet started, we don't know at which base address the executable is going to end at. Everything good so far. 3) The execution starts, GDB finds out the executable's base address, fills the objfile's section_offsets vector with a bunch of offsets, and relocates the symbols with those offsets. The latter modifies the symbol values (the `main` symbol is changed from 0x1129 to 0x555555555129). 4) We `start` again, we detect that `a.out` has changed, the `reread_symbols` function kicks in. It tries to reset everything in the `struct objfile` corresponding to `a.out`, except that it leaves the `section_offsets` vector there. 5) `reread_symbols` reads the debug info (calls `read_symbols`). As the DWARF info is read, symbols are created using the old offsets still in `section_offsets`. For example, the `main` symbol is created with the value 0x555555555129. Even though at this point there is no process, so that address is bogus. There's probably more that depends on section_offsets that is not done correctly. 6) Something in the SVR4 solib handling goes wrong, probably because of something that went wrong in (5). I can't quite explain it (if somebody would like to provide a more complete analysis, please go ahead). But this is where it takes a wrong turn: #0 elf_locate_base () at /home/smarchi/src/wt/test/gdb/solib-svr4.c:799 #1 0x000055f0a5bee6d5 in locate_base (info=) at /home/smarchi/src/wt/test/gdb/solib-svr4.c:848 #2 0x000055f0a5bf1771 in svr4_handle_solib_event () at /home/smarchi/src/wt/test/gdb/solib-svr4.c:1955 #3 0x000055f0a5c0ff92 in handle_solib_event () at /home/smarchi/src/wt/test/gdb/solib.c:1258 In the non-working case (without this patch), elf_locate_base returns 0, whereas in the working case (with this patch) it returns a valid address, as we should expect. This patch fixes this by making reread_symbols clear the `section_offsets` vector, and re-create it by calling `sym_offsets`. This is analogous to what syms_from_objfile_1 does. I didn't seem absolutely necessary, but I also made it clear the various `sect_index_*` fields, since their values no longer make sense (they describe the old executable, and are indices in the now cleared sections/section_offsets arrays). I don't really like the approach taken by reread_symbols, trying to reset everything manually on the objfile object, instead of, for example, creating a new one from scratch. But I don't know enough yet to propose a better solution. One more reason I think this patch is needed is that the number of sections of the new executable could be different from the number of sections of the old executable. So if we don't re-create the section_offsets array, not only we'll have wrong offsets, but we could make accesses past the array. Something else that silently fails (but doesn't seem to have consequences) is the prologue analysis when we try to create the breakpoint on `main`. Since the `main` symbol has the wrong value 0x555555555129, we try to access memory in that area, which fails. This can be observed by debugging gdb and using `catch throw`. Before the process is started, we need to access the memory at its unrelocated address, 0x1129, which will read memory from the ELF file. This is now what happens, with this patch applied. It silently fails, probably because commit 46a62268b, "Catch exceptions thrown from gdbarch_skip_prologue", papered over the problem and added an empty catch clause. I'm quite sure that the root cause then was the one fixed by this patch. This fixes tests gdb.ada/exec_changed.exp and gdb.base/reread.exp for me. gdb/ChangeLog: * symfile.c (reread_symbols): Clear objfile's section_offsets vector and section indices, re-compute them by calling sym_offsets. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bef88d0df6a..38d18976110 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2020-05-20 Simon Marchi + + * symfile.c (reread_symbols): Clear objfile's section_offsets + vector and section indices, re-compute them by calling + sym_offsets. + 2020-05-20 Tom Tromey * ada-lang.c (bound_name, MAX_ADA_DIMENS): Remove. diff --git a/gdb/symfile.c b/gdb/symfile.c index dd8192a67fb..b02a9235663 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2543,6 +2543,11 @@ reread_symbols (void) will need to be called (see discussion below). */ obstack_free (&objfile->objfile_obstack, 0); objfile->sections = NULL; + objfile->section_offsets.clear (); + objfile->sect_index_bss = -1; + objfile->sect_index_data = -1; + objfile->sect_index_rodata = -1; + objfile->sect_index_text = -1; objfile->compunit_symtabs = NULL; objfile->template_symbols = NULL; objfile->static_links.reset (nullptr); @@ -2597,6 +2602,9 @@ reread_symbols (void) objfiles_changed (); + /* Recompute section offsets and section indices. */ + objfile->sf->sym_offsets (objfile, {}); + read_symbols (objfile, 0); if (!objfile_has_symbols (objfile))