aboutsummaryrefslogtreecommitdiff
path: root/gdb/elfread.c
AgeCommit message (Collapse)AuthorFilesLines
2023-10-10gdb: remove target_gdbarchSimon Marchi1-2/+2
This function is just a wrapper around the current inferior's gdbarch. I find that having that wrapper just obscures where the arch is coming from, and that it's often used as "I don't know which arch to use so I'll use this magical target_gdbarch function that gets me an arch" when the arch should in fact come from something in the context (a thread, objfile, symbol, etc). I think that removing it and inlining `current_inferior ()->arch ()` everywhere will make it a bit clearer where that arch comes from and will trigger people into reflecting whether this is the right place to get the arch or not. Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-09-19Use gdb::checked_static_cast for code_breakpointTom Tromey1-1/+1
This replaces some casts to 'code_breakpoint *' with checked_static_cast. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-08-31[symtab/27831] Fix OBJF_MAINLINE assertKevin Buettner1-7/+5
This commit fixes a bug mentioned by Florian Weimer during the libpthread/ld.so load order discussion from 2021. Florian provided instructions for reproducing the bug here: https://sourceware.org/pipermail/gdb-patches/2021-April/177923.html That particular test does some interesting things involving forks, threads, and thread local storage. Fortunately, none of that is needed to reproduce the problem. I've made a new test case (which is now found in a separate commit) contained in the files gdb.base/add-symbol-file-attach.{c,exp}. The .c file is fairly simple as is the recipe for reproducing the problem. After separately starting the test case and noting the process id, start gdb (w/ no arguments), and do the following to reproduce the assertion failure - for this run, the process id of the separately started add-symbol-file-attach process is 4103218: (gdb) add-symbol-file add-symbol-file-attach add symbol table from file "add-symbol-file-attach" (y or n) y Reading symbols from add-symbol-file-attach... (gdb) attach 4103218 Attaching to process 4103218 Load new symbol table from "/tmp/add-symbol-file-attach"? (y or n) y Reading symbols from /tmp/add-symbol-file-attach... Reading symbols from /lib64/libc.so.6... (No debugging symbols found in /lib64/libc.so.6) Reading symbols from /lib64/ld-linux-x86-64.so.2... (No debugging symbols found in /lib64/ld-linux-x86-64.so.2) 0x00007f502130bf27 in pause () from /lib64/libc.so.6 (gdb) p foo symtab.c:6417: internal-error: CORE_ADDR get_msymbol_address(objfile*, const minimal_symbol*): Assertion `(objf->flags & OBJF_MAINLINE) == 0' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. The add-symbol-file command causes the symbols to be loaded without the SYMFILE_MAINLINE (and hence the OBJFILE_MAINLINE) flags being set. This, in turn, causes the "maybe_copied" flag to be set for the global symbol (named "foo" in the provided test case). The attach command will cause another objfile to be created, but it will reuse the symtabs from the objfile created by add-symbol-file, leading to a situation in which the OBJFILE_MAINLINE flag will be set for the new (attach-created) objfile, however the "maybe_copied" flag will still be set for the global symbol. Had it been loaded anew, this flag would not be set due to OBJFILE_MAINLINE being set for the objfile. At present, minimal_symbol::value_address looks like this: CORE_ADDR minimal_symbol::value_address (objfile *objfile) const { if (this->maybe_copied (objfile)) return get_msymbol_address (objfile, this); else return (CORE_ADDR (this->unrelocated_address ()) + objfile->section_offsets[this->section_index ()]); } So, we can now see the problem: When the "maybe_copied" flag is set, get_msymbol_address() will be called. However, get_msymbol_address() assumes that it won't be called with the OBF_MAINLINE flag set for the objfile in question. It, in fact, contains an assert() which makes sure that this is the case: gdb_assert ((objf->flags & OBJF_MAINLINE) == 0); (If this assert is removed, then get_msymbol_address() recurses infinitely for the case under consideration.) So, the problem here is that the maybe_copied flag is set for the symbol AND the OBJF_MAINLINE flag is set for the objfile. As noted earlier, this happens due to add-symbol-file being used; this causes the maybe_copied flag to be set. Later, when the attach is performed, OBJF_MAINLINE will be set for that objfile, leading to this unfortunate situation. My first cut at a solution involved adjusting the MSYMBOL_VALUE_ADDRESS macro (which has since been changed to be the method noted above) to include a test of the OBJFILE_MAINLINE flag. However, Simon Marchi, in his review of my patch, suggested a better solution. Simon observed that the 'maybe_copied' flag is (was, after this commit) being set/initialized in record_minimal_symbol() using using the objfile in the context in which the symbol was created. Simon further observed: Today, a single copy is created, as symtabs are shared between objfiles. This means that everything that we store into a symbol must be independent of any objfile. However, the value of the maybe_copied field is dependent on the objfile in the context of which the symbol was created. Meaning that when the symbol is re-used in the context of another objfile, the maybe_copied value is not right in the context of that objfile. So I think it means there isn't a single "is this symbol maybe copied" value, but instead "is this symbol maybe copied, in the context of this given objfile". And the answer is yes or no, depending on whether the objfile is mainline. So maybe_copied should become a method that takes an objfile and returns an answer based on that. Simon's full review can be found here: https://sourceware.org/pipermail/gdb-patches/2021-May/178855.html Simon also provided a patch which implements this suggestion. The current patch is mostly his work, though I did make some adjustments during a rebase in addition to making some changes to account for a concern from Tom Tromey. During his review of the v3 series, Tom noted, "The old approach was specific to ELF, while the new approach will be used by any object format." Tom further observed, "...it seems like it could result in an incorrect evaluation in some scenario." This seemed plausible to me, so I introduced the flag 'object_format_has_copy_relocs' to struct objfile. It is set at the end of elf_symfile_read() in elfread.c. The minimal_symbol::maybe_copied method tests this new flag, forcing this method to return false when the flag is not set. If we find that other object file formats use the same copy reloc mechanism as ELF, then 'object_format_has_copy_relocs' should be set for objfiles using those formats. Lastly, I'll note that this is a strange use case. It's far more common to either let gdb figure out which file to load by itself when attaching, i.e. (gdb) attach 4104360 Attaching to process 4104360 Reading symbols from /tmp/add-symbol-file-attach... Reading symbols from /lib64/libc.so.6... (No debugging symbols found in /lib64/libc.so.6) Reading symbols from /lib64/ld-linux-x86-64.so.2... (No debugging symbols found in /lib64/ld-linux-x86-64.so.2) 0x00007fdb1fc33f27 in pause () from /lib64/libc.so.6 (gdb) p foo $1 = 42 ...or to use the "file" command prior to the attach, like this: (gdb) file add-symbol-file-attach Reading symbols from add-symbol-file-attach... (gdb) attach 4104360 Attaching to program: /tmp/add-symbol-file-attach, process 4104360 Reading symbols from /lib64/libc.so.6... (No debugging symbols found in /lib64/libc.so.6) Reading symbols from /lib64/ld-linux-x86-64.so.2... (No debugging symbols found in /lib64/ld-linux-x86-64.so.2) 0x00007fdb1fc33f27 in pause () from /lib64/libc.so.6 Both of these more common scenarios work perfectly fine; using "add-symbol-file" to load the program to which you will attach isn't recommended as a normal use case. That said, it's bad for gdb to assert, hence this fix. Reviewed-by: Simon Marchi <simon.marchi@polymtl.ca> Co-Authored-by: Simon Marchi <simon.marchi@polymtl.ca> Approved-by: Tom Tromey <tom@tromey.com> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27831
2023-08-18Remove most includes of psymtab.hTom Tromey1-1/+0
I found that most spots including psymtab.h do not need it. This patch removes these includes, and also one unnecessary include of psympriv.h.
2023-07-15gdb: style filenames in separate debug file warningsAndrew Burgess1-9/+7
After the commit: commit 6647f05df023b63bbe056e9167e9e234172fa2ca Date: Tue Jan 24 18:13:38 2023 +0100 gdb: defer warnings when loading separate debug files It was pointed out[1] that the warnings being deferred and then later emitted lacked styling. The warnings lacked styling before the above commit, but it was suggested that the filenames in these warnings should be styled, and this commit does this. There were a couple of previous attempts[2][3][4] to solve this problem, but these all tried to extend the mechanism introduced in the above commit, the deferred warnings were placed directly into a std::vector, but now we tried to, when appropriate, style these warnings. The review feedback that this approach looked too complex. So instead, this revision adds a new helper class 'deferred_warnings' which can be used to collect a set of deferred warnings, and then emit these deferred warnings later, if needed. This helper class hides the complexity, so at the point the deferred warning is created no extra logic is required. The deferred_warnings class will style the deferred warnings only if gdb_stderr supports styling. GDB's warnings are sent to gdb_stderr, so this should ensure we only style when expected. There was also review feedback[5] that all of the warnings should be bundled into a single string_file, this has not been done. I feel pretty strongly that separate warnings should be emitted using separate "warning" calls. If we do end up with multiple warnings in this case they aren't really related, one will be about looking up debug via .gnu_debuglink, while the other will be about build-id based lookup. So I'd really rather keep the warnings separate. [1] https://inbox.sourceware.org/gdb-patches/87edr9pcku.fsf@tromey.com/ [2] https://inbox.sourceware.org/gdb-patches/20230216195604.2685177-1-ahajkova@redhat.com/ [3] https://inbox.sourceware.org/gdb-patches/20230217123547.2737612-1-ahajkova@redhat.com/ [4] https://inbox.sourceware.org/gdb-patches/20230320145638.1202335-1-ahajkova@redhat.com/ [5] https://inbox.sourceware.org/gdb-patches/87o7nh1g8h.fsf@tromey.com/ Co-Authored-By: Alexandra Hájková <ahajkova@redhat.com> Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-07-10Avoid crash with absolute symbolTom Tromey1-1/+2
A user supplied an executable and a remote logfile that could be used to crash gdb. The problem is that the BFD section for a particular symbol was null, because the section was not marked "allocated". Digging deeper, the problem was that elfread.c dropped the section for absolute symbols. This patch fixes the crash. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30431
2023-05-25gdb: add breakpoint::first_loc methodsSimon Marchi1-2/+2
Add convenience first_loc methods to struct breakpoint (const and non-const overloads). A subsequent patch changes the list of locations to be an intrusive_list and makes the actual list private, so these spots would need to change from: b->loc to something ugly like: *b->locations ().begin () That would make the code much heavier and not readable. There is a surprisingly big number of places that access the first location of breakpoints. Whether this is correct, or these spots fail to consider the possibility of multi-location breakpoints, I don't know. But anyhow, I think that using this instead: b->first_loc () conveys the intention better than the other two forms. Change-Id: Ibbefe3e4ca6cdfe570351fe7e2725f2ce11d1e95 Reviewed-By: Andrew Burgess <aburgess@redhat.com>
2023-05-25gdb: add breakpoint "has locations" methodsSimon Marchi1-2/+2
Add three convenience methods to struct breakpoint: - has_locations: returns true if the breakpoint has at least one location - has_single_location: returns true if the breakpoint has exactly one location - has_multiple_locations: returns true if the breakpoint has more than one location A subsequent patch changes the list of breakpoints to be an intrusive_list, so all these spots would need to change. But in any case, I think that this: if (b->has_multiple_locations ()) conveys the intention better than: if (b->loc != nullptr && b->loc->next != nullptr) Change-Id: Ib18c3605fd35d425ef9df82cb7aacff1606c6747 Reviewed-By: Andrew Burgess <aburgess@redhat.com>
2023-05-18gdb/elfread.c: Add plt symbol check for _PROCEDURE_LINKAGE_TABLE_Hui Li1-0/+3
In the current code, when execute the following test on LoongArch: $ make check-gdb TESTS="gdb.base/gnu-ifunc.exp" === gdb Summary === # of expected passes 111 # of unexpected failures 62 According to IFUNC's working process [1]. first time the IFUNC function is called, the dynamic linker will not simply fill the .got.plt entry with the actual address of IFUNC symbol, it will call the IFUNC resolver function and take the return address, uses it as the sym-bound address and puts it in the .got.plt entry. Initial address in .got.plt entry is not a real function addresss. Depending on the compiler implementation, some different addresses will be filled in. Most architectures will use a .plt entry address to fill in the corresponding .got.plt entry. In gdb, elf_gnu_ifunc_resolve_addr() will be called to return a real IFUNC function addresss. First check to see if the real address for the IFUNC symbol has been resolved by the following function: elf_gnu_ifunc_resolve_name (const char *name, CORE_ADDR *addr_p) { if (elf_gnu_ifunc_resolve_by_cache (name, addr_p)) return true; if (elf_gnu_ifunc_resolve_by_got (name, addr_p)) return true; return false; } in elf_gnu_ifunc_resolve_by_got(), it gets the contents of the .got.plt entry and determines if the contents is the correct address by calling elf_gnu_ifunc_record_cache(). Based on the IFUNC working principle analysis above, the address filled in the .got.plt entry is not the actual target function address initially, it would be a .plt entry address corresponding symbol like *@plt. In this case, gdb just go back to execute the resolver function and puts the return address in the .got.plt entry. After that, gdb can get a real ifun address via .got.plt entry. On LoongArch, initially, each address filled in the .got.plt entries is the first .plt entry address. Some architectures such as LoongArch define the symbol _PROCEDURE_LINKAGE_TABLE_ at the start of the .plt section. This symbol is the first plt entry, so gdb needs to check this symbol in elf_gnu_ifunc_record_cache(). On LoongArch .got.plt and .plt section as follow: $objdump -D gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-0-0-0 ... 0000000120010008 <.got.plt>: 120010008: ffffffff 0xffffffff 12001000c: ffffffff 0xffffffff ... 120010018: 20004000 ll.w $zero, $zero, 64(0x40) 12001001c: 00000001 0x00000001 120010020: 20004000 ll.w $zero, $zero, 64(0x40) 120010024: 00000001 0x00000001 120010028: 20004000 ll.w $zero, $zero, 64(0x40) 12001002c: 00000001 0x00000001 120010030: 20004000 ll.w $zero, $zero, 64(0x40) 120010034: 00000001 0x00000001 ... Disassembly of section .plt: 0000000120004000 <_PROCEDURE_LINKAGE_TABLE_>: 120004000: 1c00018e pcaddu12i $t2, 12(0xc) 120004004: 0011bdad sub.d $t1, $t1, $t3 120004008: 28c021cf ld.d $t3, $t2, 8(0x8) 12000400c: 02ff51ad addi.d $t1, $t1, -44(0xfd4) 120004010: 02c021cc addi.d $t0, $t2, 8(0x8) 120004014: 004505ad srli.d $t1, $t1, 0x1 120004018: 28c0218c ld.d $t0, $t0, 8(0x8) 12000401c: 4c0001e0 jirl $zero, $t3, 0 0000000120004020 <__libc_start_main@plt>: 120004020: 1c00018f pcaddu12i $t3, 12(0xc) 120004024: 28ffe1ef ld.d $t3, $t3, -8(0xff8) 120004028: 4c0001ed jirl $t1, $t3, 0 12000402c: 03400000 andi $zero, $zero, 0x0 0000000120004030 <abort@plt>: 120004030: 1c00018f pcaddu12i $t3, 12(0xc) 120004034: 28ffc1ef ld.d $t3, $t3, -16(0xff0) 120004038: 4c0001ed jirl $t1, $t3, 0 12000403c: 03400000 andi $zero, $zero, 0x0 0000000120004040 <gnu_ifunc@plt>: 120004040: 1c00018f pcaddu12i $t3, 12(0xc) 120004044: 28ffa1ef ld.d $t3, $t3, -24(0xfe8) 120004048: 4c0001ed jirl $t1, $t3, 0 12000404c: 03400000 andi $zero, $zero, 0x0 ... With this patch: $make check-gdb TESTS="gdb.base/gnu-ifunc.exp" === gdb Summary === #of expected passes 173 [1] https://sourceware.org/glibc/wiki/GNU_IFUNC Signed-off-by: Hui Li <lihui@loongson.cn>
2023-04-13Avoid double-free with debuginfodTom Tromey1-9/+10
PR gdb/29257 points out a possible double free when debuginfod is in use. Aside from some ugly warts in the symbol code (an ongoing issue), the underlying issue in this particular case is that elfread.c seems to assume that symfile_bfd_open will return NULL on error, whereas in reality it throws an exception. As this code isn't prepared for an exception, bad things result. This patch fixes the problem by introducing a non-throwing variant of symfile_bfd_open and using it in the affected places. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29257
2023-03-28Use unrelocated_addr in minimal symbolsTom Tromey1-9/+13
This changes minimal symbols to use unrelocated_addr. I believe this detected a latent bug in add_pe_forwarded_sym.
2023-03-08Remove OBJF_REORDEREDTom Tromey1-4/+0
OBJF_REORDERED is set for nearly every object format. And, despite the ominous warnings here and there, it does not seem very expensive. This patch removes the flag entirely. Reviewed-By: Andrew Burgess <aburgess@redhat.com>
2023-03-02[gdb/symtab] Add set/show always-read-ctf on/offTom de Vries1-1/+23
[ This is a simplified rewrite of an earlier submission "[RFC][gdb/symtab] Add maint set symbol-read-order", submitted here ( https://sourceware.org/pipermail/gdb-patches/2022-September/192044.html ). ] With the test-case included in this patch, we run into: ... (gdb) file dwarf2-and-ctf (gdb) print var_ctf^M 'var_ctf' has unknown type; cast it to its declared type^M ... The problem is that the executable contains both ctf and dwarf2, so the ctf info (which contains the type information about var_ctf) is ignored. GDB has support for handling multiple debug formats, but the common use case for ctf is to be used when dwarf2 is not present, and gdb reflects that, assuming that by reading ctf in addition there won't be any extra information, so it's not worth the additional cycles and memory. Add a new command "set/show always-read-ctf on/off", that when on forces unconditional reading of ctf, allowing us to do: ... (gdb) set always-read-ctf on (gdb) file dwarf2-and-ctf (gdb) print var_ctf^M $2 = 2^M ... The setting is off by default, preserving current behaviour. A bit of background on the relevance of reading order: the formats have a priority relationship between them, where reading earlier means lower priority. By reading the format with the most detail last, we ensure it has the highest priority, which makes sure that in case there is overlapping info, the most detailed info is found. This explains the current reading order of mdebug, stabs and dwarf2. Add the unconditional reading of ctf before dwarf2, because it's less detailed than dwarf2. The conditional reading of ctf is still done after the attempt to read dwarf2, necessarily so because we only know whether there's dwarf2 after we've tried to read it. The new command allow us to replace uses of -Wl,--strip-debug added in commit 908a926ec4e ("[gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed") by uses of "set always-read-ctf on", but I've left that for another commit. Tested on x86_64-linux. Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Tom Tromey <tom@tromey.com>
2023-02-13Introduce set_lval method on valueTom Tromey1-2/+2
This introduces the set_lval method on value, one step toward removing deprecated_lval_hack. Ultimately I think the goal should be for some of these set_* methods to be replaced with constructors; but I haven't done this, as the series is already too long. Other 'deprecated' methods can probably be handled the same way. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn allocate_value into a static "constructor"Tom Tromey1-3/+3
This changes allocate_value to be a static "constructor" of value. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_address and set_value_address functions into methodsTom Tromey1-2/+2
This changes the value_address and set_value_address functions to be methods of value. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-01gdb: defer warnings when loading separate debug filesAlexandra Hájková1-2/+11
Currently, when GDB loads debug information from a separate debug file, there are a couple of warnings that could be produced if things go wrong. In find_separate_debug_file_by_buildid (build-id.c) GDB can give a warning if the separate debug file doesn't include any actual debug information, and in separate_debug_file_exists (symfile.c) we can warn if the CRC checksum in the separate debug file doesn't match the checksum in the original executable. The problem here is that, when looking up debug information, GDB will try several different approaches, lookup by build-id, lookup by debug-link, and then a lookup from debuginfod. GDB can potentially give a warning from an earlier attempt, and then succeed with a later attempt. In the cases I have run into this is primarily a warning about some out of date debug information on my machine, but then GDB finds the correct information using debuginfod. This can be confusing to a user, they will see warnings from GDB when really everything is working just fine. For example: warning: the debug information found in "/usr/lib/debug//lib64/ld-2.32.so.debug" \ does not match "/lib64/ld-linux-x86-64.so.2" (CRC mismatch). This diagnostic was printed on Fedora 33 even when the correct debuginfo was downloaded. In this patch I propose that we defer any warnings related to looking up debug information from a separate debug file. If any of the approaches are successful then GDB will not print any of the warnings. As far as the user is concerned, everything "just worked". Only if GDB completely fails to find any suitable debug information will the warnings be printed. The crc_mismatch test compiles two executables: crc_mismatch and crc_mismatch-2 and then strips them of debuginfo creating separate debug files. The test then replaces crc_mismatch-2.debug with crc_mismatch.debug to trigger "CRC mismatch" warning. A local debuginfod server is setup to supply the correct debug file, now when GDB looks up the debug info no warning is given. The build-id-no-debug-warning.exp is similar to the previous test. It triggers the "separate debug info file has no debug info" warning by replacing the build-id based .debug file with the stripped binary and then loading it to GDB. It then also sets up local debuginfod server with the correct debug file to download to make sure no warnings are emitted.
2023-01-03Add new overload of gdbarch_return_valueTom Tromey1-2/+2
The gdbarch "return_value" can't correctly handle variably-sized types. The problem here is that the TYPE_LENGTH of such a type is 0, until the type is resolved, which requires reading memory. However, gdbarch_return_value only accepts a buffer as an out parameter. Fixing this requires letting the implementation of the gdbarch method resolve the type and return a value -- that is, both the contents and the new type. After an attempt at this, I realized I wouldn't be able to correctly update all implementations (there are ~80) of this method. So, instead, this patch adds a new method that falls back to the current method, and it updates gdb to only call the new method. This way it's possible to incrementally convert the architectures that I am able to test.
2023-01-01Update copyright year range in header of all files managed by GDBJoel Brobecker1-1/+1
This commit is the result of running the gdb/copyright.py script, which automated the update of the copyright year range for all source files managed by the GDB project to be updated to include year 2023.
2022-10-19internal_error: remove need to pass __FILE__/__LINE__Pedro Alves1-2/+1
Currently, every internal_error call must be passed __FILE__/__LINE__ explicitly, like: internal_error (__FILE__, __LINE__, "foo %d", var); The need to pass in explicit __FILE__/__LINE__ is there probably because the function predates widespread and portable variadic macros availability. We can use variadic macros nowadays, and in fact, we already use them in several places, including the related gdb_assert_not_reached. So this patch renames the internal_error function to something else, and then reimplements internal_error as a variadic macro that expands __FILE__/__LINE__ itself. The result is that we now should call internal_error like so: internal_error ("foo %d", var); Likewise for internal_warning. The patch adjusts all calls sites. 99% of the adjustments were done with a perl/sed script. The non-mechanical changes are in gdbsupport/errors.h, gdbsupport/gdb_assert.h, and gdb/gdbarch.py. Approved-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
2022-10-18gdb: update gnu ifunc resolveMarkus Metzger1-66/+87
Update elf_gnu_ifunc_resolve_by_cache() and elf_gnu_ifunc_resolve_by_got() to use gdbarch_iterate_over_objfiles_in_search_order() in order to restrict the objfile traversal to the initial namespace. In order to extend this to other namespaces, we'd need to provide context, e.g. via an objfile inside that namespace.
2022-10-12[gdb/symtab] Factor out elf_symfile_read_dwarf2Tom de Vries1-64/+78
Factor out elf_symfile_read_dwarf2 from elf_symfile_read. NFC. Tested on x86_64-linux.
2022-10-11gdb: fix auxv cachingSimon Marchi1-1/+1
There's a flaw in the interaction of the auxv caching and the fact that target_auxv_search allows reading auxv from an arbitrary target_ops (passed in as a parameter). This has consequences as explained in this thread: https://inbox.sourceware.org/gdb-patches/20220719144542.1478037-1-luis.machado@arm.com/ In summary, when loading an AArch64 core file with MTE support by passing the executable and core file names directly to GDB, we see the MTE info: $ ./gdb -nx --data-directory=data-directory -q aarch64-mte-gcore aarch64-mte-gcore.core ... Program terminated with signal SIGSEGV, Segmentation fault Memory tag violation while accessing address 0x0000ffff8ef5e000 Allocation tag 0x1 Logical tag 0x0. #0 0x0000aaaade3d0b4c in ?? () (gdb) But if we do it as two separate commands (file and core) we don't: $ ./gdb -nx --data-directory=data-directory -q -ex "file aarch64-mte-gcore" -ex "core aarch64-mte-gcore.core" ... Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000aaaade3d0b4c in ?? () (gdb) The problem with the latter is that auxv data gets improperly cached between the two commands. When executing the file command, auxv gets first queried here, when loading the executable: #0 target_auxv_search (ops=0x55555b842400 <exec_ops>, match=0x9, valp=0x7fffffffc5d0) at /home/simark/src/binutils-gdb/gdb/auxv.c:383 #1 0x0000555557e576f2 in svr4_exec_displacement (displacementp=0x7fffffffc8c0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2482 #2 0x0000555557e594d1 in svr4_relocate_main_executable () at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2878 #3 0x0000555557e5989e in svr4_solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2933 #4 0x0000555557e6e49f in solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib.c:1253 #5 0x0000555557f33e29 in symbol_file_command (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/symfile.c:1655 #6 0x00005555573319c3 in file_command (arg=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:555 #7 0x0000555556e47185 in do_simple_func (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1, c=0x612000047740) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:95 #8 0x0000555556e551c9 in cmd_func (cmd=0x612000047740, args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2543 #9 0x00005555580e63fd in execute_command (p=0x7fffffffe02c "e", from_tty=1) at /home/simark/src/binutils-gdb/gdb/top.c:692 #10 0x0000555557771913 in catch_command_errors (command=0x5555580e55ad <execute_command(char const*, int)>, arg=0x7fffffffe017 "file aarch64-mte-gcore", from_tty=1, do_bp_actions=true) at /home/simark/src/binutils-gdb/gdb/main.c:513 #11 0x0000555557771fba in execute_cmdargs (cmdarg_vec=0x7fffffffd570, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd230) at /home/simark/src/binutils-gdb/gdb/main.c:608 #12 0x00005555577755ac in captured_main_1 (context=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1299 #13 0x0000555557775c2d in captured_main (data=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1320 #14 0x0000555557775cc2 in gdb_main (args=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1345 #15 0x00005555568bdcbe in main (argc=10, argv=0x7fffffffdba8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32 Here, target_auxv_search is called on the inferior's target stack. The target stack only contains the exec target, so the query returns empty auxv data. This gets cached for that inferior in `auxv_inferior_data`. In its constructor (before it is pushed to the inferior's target stack), the core_target needs to identify the right target description from the core, and for that asks the gdbarch to read a target description from the core file. Because some implementations of gdbarch_core_read_description (such as AArch64's) need to read auxv data from the core in order to determine the right target description, the core_target passes a pointer to itself, allowing implementations to call target_auxv_search it. However, because we have previously cached (empty) auxv data for that inferior, target_auxv_search searched that cached (empty) auxv data, not auxv data read from the core. Remember that this data was obtained by reading auxv on the inferior's target stack, which only contained an exec target. The problem I see is that while target_auxv_search offers the flexibility of reading from an arbitrary (passed as an argument) target, the caching doesn't do the distinction of which target is being queried, and where the cached data came from. So, you could read auxv from a target A, it gets cached, then you try to read auxv from a target B, and it returns the cached data from target A. That sounds wrong. In our case, we expect to read different auxv data from the core target than what we have read from the target stack earlier, so it doesn't make sense to hit the cache in this case. To fix this, I propose splitting the code paths that read auxv data from an inferior's target stack and those that read from a passed-in target. The code path that reads from the target stack will keep caching, whereas the one that reads from a passed-in target won't. And since, searching in auxv data is independent from where this data came from, split the "read" part from the "search" part. From what I understand, auxv caching was introduced mostly to reduce latency on remote connections, when doing many queries. With the change I propose, only the queries done while constructing the core_target end up not using cached auxv data. This is fine, because there are just a handful of queries max, done at this point, and reading core files is local. The changes to auxv functions are: - Introduce 2 target_read_auxv functions. One reads from an explicit target_ops and doesn't do caching (to be used in gdbarch_core_read_description context). The other takes no argument, reads from the current inferior's target stack (it looks just like a standard target function wrapper) and does caching. The first target_read_auxv actually replaces get_auxv_inferior_data, since it became a trivial wrapper around it. - Change the existing target_auxv_search to not read auxv data from the target, but to accept it as a parameter (a gdb::byte_vector). This function doesn't care where the data came from, it just searches in it. It still needs to take a target_ops and gdbarch to know how to parse auxv entries. - Add a convenience target_auxv_search overload that reads auxv data from the inferior's target stack and searches in it. This overload is useful to replace the exist target_auxv_search calls that passed the `current_inferior ()->top_target ()` target and keep the call sites short. - Modify parse_auxv to accept a target_ops and gdbarch to use for parsing entries. Not strictly related to the rest of this change, but it seems like a good change in the context. Changes in architecture-specific files (tdep and nat): - In linux-tdep, linux_get_hwcap and linux_get_hwcap2 get split in two, similar to target_auxv_search. One version receives auxv data, target and arch as parameters. The other gets everything from the current inferior. The latter is for convenience, to avoid making call sites too ugly. - Call sites of linux_get_hwcap and linux_get_hwcap2 are adjusted to use either of the new versions. The call sites in gdbarch_core_read_description context explicitly read auxv data from the passed-in target and call the linux_get_hwcap{,2} function with parameters. Other call sites use the versions without parameters. - Same idea for arm_fbsd_read_description_auxv. - Call sites of target_auxv_search that passed `current_inferior ()->top_target ()` are changed to use the target_auxv_search overload that works in the current inferior. Reviewed-By: John Baldwin <jhb@FreeBSD.org> Reviewed-By: Luis Machado <luis.machado@arm.com> Change-Id: Ib775a220cf1e76443fb7da2fdff8fc631128fe66
2022-10-10Change GDB to use frame_info_ptrTom Tromey1-1/+1
This changes GDB to use frame_info_ptr instead of frame_info * The substitution was done with multiple sequential `sed` commands: sed 's/^struct frame_info;/class frame_info_ptr;/' sed 's/struct frame_info \*/frame_info_ptr /g' - which left some issues in a few files, that were manually fixed. sed 's/\<frame_info \*/frame_info_ptr /g' sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace problems. The changed files were then manually checked and some 'sed' changes undone, some constructors and some gets were added, according to what made sense, and what Tromey originally did Co-Authored-By: Bruno Larsen <blarsen@redhat.com> Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10Remove frame_id_eqTom Tromey1-1/+1
This replaces frame_id_eq with operator== and operator!=. I wrote this for a version of this series that I later abandoned; but since it simplifies the code, I left this patch in. Approved-by: Tom Tomey <tom@tromey.com>
2022-09-21gdb: remove TYPE_LENGTHSimon Marchi1-2/+2
Remove the macro, replace all uses with calls to type::length. Change-Id: Ib9bdc954576860b21190886534c99103d6a47afb
2022-09-21gdb: remove TYPE_TARGET_TYPESimon Marchi1-1/+1
Remove the macro, replace all uses by calls to type::target_type. Change-Id: Ie51d3e1e22f94130176d6abd723255282bb6d1ed
2022-08-30gdb/elfread.c: Use bfd filename instead of objfile->original_nameAaron Merey1-2/+3
The call to debuginfod_debuginfo_query in elf_symfile_read is given objfile->original_name as the filename to print when downloading the objfile's debuginfo. In some cases original_name is prefixed with gdb's working directory even though the objfile is not located in the working directory. This causes debuginfod to display the wrong path of the objfile during a download. Fix this by using the objfile's bfd filename instead.
2022-08-03Use gdb_bfd_ref_ptr in objfileTom Tromey1-21/+22
This changes struct objfile to use a gdb_bfd_ref_ptr. In addition to removing some manual memory management, this fixes a use-after-free that was introduced by the registry rewrite series. The issue there was that, in some cases, registry shutdown could refer to memory that had already been freed. This help fix the bug by delaying the destruction of the BFD reference (and thus the per-bfd object) until after the registry has been shut down.
2022-07-29gdb: introduce symtab_create_debug_printfSimon Marchi1-11/+4
Introduce symtab_create_debug_printf and symtab_create_debug_printf_v, to print the debug messages enabled by "set debug symtab-create". Change-Id: I442500903f72d4635c2dd9eaef770111f317dc04
2022-07-28Rewrite registry.hTom Tromey1-2/+2
This rewrites registry.h, removing all the macros and replacing it with relatively ordinary template classes. The result is less code than the previous setup. It replaces large macros with a relatively straightforward C++ class, and now manages its own cleanup. The existing type-safe "key" class is replaced with the equivalent template class. This approach ended up requiring relatively few changes to the users of the registry code in gdb -- code using the key system just required a small change to the key's declaration. All existing users of the old C-like API are now converted to use the type-safe API. This mostly involved changing explicit deletion functions to be an operator() in a deleter class. The old "save/free" two-phase process is removed, and replaced with a single "free" phase. No existing code used both phases. The old "free" callbacks took a parameter for the enclosing container object. However, this wasn't truly needed and is removed here as well.
2022-06-17Convert location_spec_to_string to a methodPedro Alves1-2/+1
This converts location_spec_to_string to a method of location_spec, simplifying the code using it, as it no longer has to use std::unique_ptr::get(). Change-Id: I621bdad8ea084470a2724163f614578caf8f2dd5
2022-06-17event_location -> location_specPedro Alves1-2/+2
Currently, GDB internally uses the term "location" for both the location specification the user input (linespec, explicit location, or an address location), and for actual resolved locations, like the breakpoint locations, or the result of decoding a location spec to SaLs. This is expecially confusing in the breakpoints module, as struct breakpoint has these two fields: breakpoint::location; breakpoint::loc; "location" is the location spec, and "loc" is the resolved locations. And then, we have a method called "locations()", which returns the resolved locations as range... The location spec type is presently called event_location: /* Location we used to set the breakpoint. */ event_location_up location; and it is described like this: /* The base class for all an event locations used to set a stop event in the inferior. */ struct event_location { and even that is incorrect... Location specs are used for finding actual locations in the program in scenarios that have nothing to do with stop events. E.g., "list" works with location specs. To clean all this confusion up, this patch renames "event_location" to "location_spec" throughout, and then all the variables that hold a location spec, they are renamed to include "spec" in their name, like e.g., "location" -> "locspec". Similarly, functions that work with location specs, and currently have just "location" in their name are renamed to include "spec" in their name too. Change-Id: I5814124798aa2b2003e79496e78f95c74e5eddca
2022-05-20Rename base_breakpoint -> code_breakpointPedro Alves1-3/+3
Even after the previous patches reworking the inheritance of several breakpoint types, the present breakpoint hierarchy looks a bit surprising, as we have "breakpoint" as the superclass, and then "base_breakpoint" inherits from "breakpoint". Like so, simplified: breakpoint base_breakpoint ordinary_breakpoint internal_breakpoint momentary_breakpoint ada_catchpoint exception_catchpoint tracepoint watchpoint catchpoint exec_catchpoint ... The surprising part to me is having "base_breakpoint" being a subclass of "breakpoint". I'm just refering to naming here -- I mean, you'd expect that it would be the top level baseclass that would be called "base". Just flipping the names of breakpoint and base_breakpoint around wouldn't be super great for us, IMO, given we think of every type of *point as a breakpoint at the user visible level. E.g., "info breakpoints" shows watchpoints, tracepoints, etc. So it makes to call the top level class breakpoint. Instead, I propose renaming base_breakpoint to code_breakpoint. The previous patches made sure that all code breakpoints inherit from base_breakpoint, so it's fitting. Also, "code breakpoint" contrasts nicely with a watchpoint also being typically known as a "data breakpoint". After this commit, the resulting hierarchy looks like: breakpoint code_breakpoint ordinary_breakpoint internal_breakpoint momentary_breakpoint ada_catchpoint exception_catchpoint tracepoint watchpoint catchpoint exec_catchpoint ... ... which makes a lot more sense to me. I've left this patch as last in the series in case people want to bikeshed on the naming. "code" has a nice property that it's exactly as many letters as "base", so this patch didn't require any reindentation. :-) Change-Id: Id8dc06683a69fad80d88e674f65e826d6a4e3f66
2022-05-20Move add_location(sal) to base_breakpointPedro Alves1-3/+3
After the previous patches, only base_breakpoint subclasses use add_location(sal), so we can move it to base_breakpoint (a.k.a. base class for code breakpoints). This requires a few casts here and there, but always at spots where you can see from context what the breakpoint's type actually is. I inlined new_single_step_breakpoint into its only caller exactly for this reason. I did try to propagate more use of base_breakpoint to avoid casts, but that turned out unwieldy for this patch. Change-Id: I49d959322b0fdce5a88a216bb44730fc5dd7c6f8
2022-04-11gdb: remove minimal symbol size macrosSimon Marchi1-4/+4
Remove MSYMBOL_HAS_SIZE, MSYMBOL_SIZE and SET_MSYMBOL_SIZE, replace them with equivalent methods. Change-Id: I6ee1cf82df37e58dff52ea6568ceb4649c7d7538
2022-04-11gdb: remove MSYMBOL_TYPE macroSimon Marchi1-1/+1
Add a getter and a setter for a minimal symbol's type. Remove the corresponding macro and adjust all callers. Change-Id: I89900df5ffa5687133fe1a16b2e0d4684e67a77d
2022-04-11gdb: remove symbol value macrosSimon Marchi1-2/+2
Remove all macros related to getting and setting some symbol value: #define SYMBOL_VALUE(symbol) (symbol)->value.ivalue #define SYMBOL_VALUE_ADDRESS(symbol) \ #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define SYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->value.common_block #define SYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block #define SYMBOL_VALUE_CHAIN(symbol) (symbol)->value.chain #define MSYMBOL_VALUE(symbol) (symbol)->value.ivalue #define MSYMBOL_VALUE_RAW_ADDRESS(symbol) ((symbol)->value.address + 0) #define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \ #define BMSYMBOL_VALUE_ADDRESS(symbol) \ #define SET_MSYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define MSYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define MSYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block Replace them with equivalent methods on the appropriate objects. Change-Id: Iafdab3b8eefc6dc2fd895aa955bf64fafc59ed50
2022-03-29Unify gdb printf functionsTom Tromey1-6/+6
Now that filtered and unfiltered output can be treated identically, we can unify the printf family of functions. This is done under the name "gdb_printf". Most of this patch was written by script.
2022-01-05Change how versioned symbols are recordedTom Tromey1-35/+34
A change to BFD caused a gdb regression when using the Ada "catch exception" feature. The bug is visible when a shared library throws an exception that is caught in the main executable. This was discussed here: https://sourceware.org/pipermail/binutils/2021-July/117538.html This patch implements Alan's proposed fix, namely to use VERSYM_HIDDEN rather than the name when deciding to install a version-less symbol. The internal test case is identical to the catch_ex_std.exp that is in-tree, so I haven't added a new test. I could not make that one fail on x86-64 Linux, though. It's possible that maybe I'd have to update the system linker first, but I didn't want to try that. Regression tested on x86-64 Fedora 32.
2022-01-01Automatic Copyright Year update after running gdb/copyright.pyJoel Brobecker1-1/+1
This commit brings all the changes made by running gdb/copyright.py as per GDB's Start of New Year Procedure. For the avoidance of doubt, all changes in this commits were performed by the script.
2021-10-25gdb: change functions returning value contents to use gdb::array_viewSimon Marchi1-1/+1
The bug fixed by this [1] patch was caused by an out-of-bounds access to a value's content. The code gets the value's content (just a pointer) and then indexes it with a non-sensical index. This made me think of changing functions that return value contents to return array_views instead of a plain pointer. This has the advantage that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view are checked, making bugs more apparent / easier to find. This patch changes the return types of these functions, and updates callers to call .data() on the result, meaning it's not changing anything in practice. Additional work will be needed (which can be done little by little) to make callers propagate the use of array_view and reap the benefits. [1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
2021-09-06Remove some complaints.h includesTom Tromey1-1/+0
There are a few includes of complaints.h that aren't necessary. This patch removes them. Tested by rebuilding.
2021-03-28Simplify DWARF reader initializationTom Tromey1-36/+1
Now that the quick functions are separate from the object file format, there's no need to have elfread.c push a new entry on the objfile 'qf' list. Instead, this detail can be pushed into the DWARF reader. That is what this patch implements. I wasn't sure whether lazy reading still makes sense or not. It's still only used by ELF, and only in certain situations (like vfork, I think). It may not be carrying its weight, so we may want to consider removing this in the future. Also, I'm unclear on why the various indices are only used for ELF. This seems sub-optimal. However, I haven't tried to address that here. gdb/ChangeLog 2021-03-28 Tom Tromey <tom@tromey.com> * elfread.c (can_lazily_read_symbols): Move to dwarf2/read.c. (elf_symfile_read): Simplify. * dwarf2/read.c (struct lazy_dwarf_reader): Move from elfread.c. (make_lazy_dwarf_reader): New function. (make_dwarf_gdb_index, make_dwarf_debug_names): Now static. (dwarf2_initialize_objfile): Return void. Remove index_kind parameter. Push on 'qf' list. * dwarf2/public.h (dwarf2_initialize_objfile): Change return type. Remove 'index_kind' parameter. (make_dwarf_gdb_index, make_dwarf_debug_names): Don't declare.
2021-03-27Don't declare elf_sym_fns_lazy_psymsTom Tromey1-2/+0
An earlier patch neglected to delete a forward declaration of elf_sym_fns_lazy_psyms. This is no longer defined. This patch removes it. gdb/ChangeLog 2021-03-27 Tom Tromey <tom@tromey.com> * elfread.c (elf_sym_fns_lazy_psyms): Don't declare.
2021-03-27Don't clear 'qf' in elf_symfile_readTom Tromey1-1/+0
I noticed that I forgot to make a change in my series to make it possible to attach multiple debug readers to an objfile. In one spot, elf_symfile_read still clears the 'qf' list. However, this should have been removed toward the end of that series. This patch fixes the offending spot. Tested on x86-64 Fedora 32. gdb/ChangeLog 2021-03-27 Tom Tromey <tom@tromey.com> * elfread.c (elf_symfile_read): Don't clear 'qf'.
2021-03-24gdb: remove current_top_target functionSimon Marchi1-7/+7
The current_top_target function is a hidden dependency on the current inferior. Since I'd like to slowly move towards reducing our dependency on the global current state, remove this function and make callers use current_inferior ()->top_target () There is no expected change in behavior, but this one step towards making those callers use the inferior from their context, rather than refer to the global current inferior. gdb/ChangeLog: * target.h (current_top_target): Remove, make callers use the current inferior instead. * target.c (current_top_target): Remove. Change-Id: Iccd457036f84466cdaa3865aa3f9339a24ea001d
2021-03-20Allow multiple partial symbol readers per objfileTom Tromey1-16/+3
This patch finally changes gdb so that an objfile can have multiple sources of partial symbols (or mixed partial symbols and other kinds of indices). This is done by having each symbol reader create its own psymbol_functions object and add it to the 'qf' list in the objfile. gdb/ChangeLog 2021-03-20 Tom Tromey <tom@tromey.com> * xcoffread.c (xcoff_initial_scan): Create partial symtabs. * symfile.c (syms_from_objfile_1, reread_symbols): Update. * psymtab.h (make_psymbol_functions): Don't declare. * psymtab.c (make_psymbol_functions): Remove. (maintenance_print_psymbols): Update. * psympriv.h (struct psymbol_functions): Add no-argument constructor. * objfiles.h (struct objfile) <reset_psymtabs>: Remove. <partial_symtabs>: Remove. * mdebugread.c (mdebug_build_psymtabs): Create partial symtabs. * elfread.c (read_partial_symbols): Update. (elf_symfile_read): Remove check for existing partial symbols. Don't clear "qf". * dwarf2/read.c (dwarf2_has_info): Remove check for existing partial symbols. (dwarf2_build_psymtabs): Add psymbol_functions parameter. Create partial symtabs. * dwarf2/public.h (dwarf2_build_psymtabs): Add psymbol_functions parameter. * dbxread.c (dbx_symfile_read): Create partial symtabs. * ctfread.c (elfctf_build_psymtabs): Create partial symtabs.
2021-03-20Switch objfile to hold a list of psymbol readersTom Tromey1-3/+6
This changes objfile::qf to be a forward_list, and then updates all the uses to iterate over the list. Note that there is still only ever a single element in the list; this is handled by clearing the list whenever an object is added. gdb/ChangeLog 2021-03-20 Tom Tromey <tom@tromey.com> * dwarf2/read.c (dwarf2_build_psymtabs): Update. * symfile.c (syms_from_objfile_1, reread_symbols): Update. * symfile-debug.c (objfile::has_partial_symbols) (objfile::find_last_source_symtab) (objfile::forget_cached_source_info) (objfile::map_symtabs_matching_filename, objfile::lookup_symbol) (objfile::print_stats, objfile::dump) (objfile::expand_symtabs_for_function) (objfile::expand_all_symtabs) (objfile::expand_symtabs_with_fullname) (objfile::map_matching_symbols) (objfile::expand_symtabs_matching) (objfile::find_pc_sect_compunit_symtab) (objfile::map_symbol_filenames) (objfile::find_compunit_symtab_by_address) (objfile::lookup_global_symbol_language) (objfile::require_partial_symbols): Update. * psymtab.c (maintenance_print_psymbols) (maintenance_info_psymtabs, maintenance_check_psymtabs): Update. * objfiles.h (struct objfile) <qf>: Now a forward_list. * objfiles.c (objfile_relocate1): Update. * elfread.c (elf_symfile_read): Update.
2021-03-20Remove sym_fns::sym_read_psymbolsTom Tromey1-29/+20
Partial symbols are read via the sym_fns::sym_read_psymbols function pointer. In order to separate the partial symbols from the objfile, this must instead be done via a virtual method on quick_symbol_functions. This patch implements this change. gdb/ChangeLog 2021-03-20 Tom Tromey <tom@tromey.com> * xcoffread.c (xcoff_sym_fns): Update. * symfile.h (struct sym_fns) <sym_read_psymbols>: Remove. * symfile-debug.c (objfile::has_partial_symbols): Use can_lazily_read_symbols. (debug_sym_read_psymbols): Remove. (debug_sym_fns, install_symfile_debug_logging): Update. * quick-symbol.h (struct quick_symbol_functions) <can_lazily_read_symbols, read_partial_symbols>: New methods. * psymtab.c (require_partial_symbols): Use new 'qf' methods. * mipsread.c (ecoff_sym_fns): Update. * machoread.c (macho_sym_fns): Update. * elfread.c (struct lazy_dwarf_reader): New. (elf_symfile_read): Update. (read_psyms): Now a method of lazy_dwarf_reader. (elf_sym_fns): Update. (elf_sym_fns_lazy_psyms): Remove. * dbxread.c (aout_sym_fns): Update. * coffread.c (coff_sym_fns): Update.