From: Andrew Burgess Date: Sat, 31 Aug 2019 20:46:27 +0000 (+0100) Subject: gdb: Don't fault for 'maint print psymbols' when using an index X-Git-Url: http://git.efficios.com/?a=commitdiff_plain;h=3dd9bb462012df685d6d41300dacedae1c81e28a;p=deliverable%2Fbinutils-gdb.git gdb: Don't fault for 'maint print psymbols' when using an index I found that these tests: make check-gdb RUNTESTFLAGS="--target_board=cc-with-gdb-index gdb.base/maint.exp" make check-gdb RUNTESTFLAGS="--target_board=cc-with-debug-names gdb.base/maint.exp" were causing GDB to segfault. It turns out that this test runs this command: maint print psymbols -pc main /path/to/some/file which tries to lookup the partial_symtab for 'main'. The problem is that there is no partial_symtab for 'main' as we are using the .gdb_index or .debug_names instead of partial_symtabs. What happens is that maintenance_print_symbols calls find_pc_sect_psymtab, which looks for the partial_symtab in the objfile's objfile->partial_symtabs->psymtabs_addrmap. This is a problem because when we are using the indexes psymtabs_addrmap is reused to hold things other than partial_symtabs, this can be seen in dwarf2read.c in create_addrmap_from_index and create_addrmap_from_aranges. If we then lookup in psymtabs_addrmap we end up returning a pointer to something that isn't really a partial_symtab, after which everything goes wrong. Initially I simply added a check at the start of find_pc_sect_psymtab that the objfile had some partial_symtabs, like: if (objfile->partial_symtabs->psymtabs == NULL) return NULL; Figuring that if there were no partial_symtabs at all then this function should always return NULL, however, this caused a failure in the test gdb.python/py-event.exp which I didn't dig into too deeply, but seems to be that in this tests there are initially no psymtabs, but the second part of find_pc_sect_psymtab does manage to read some in from somewhere, with the check I added the test fails as we returned NULL here and this caused GDB to load in the full symtabs earlier than was expected. Instead I chose to guard only the access to psymtabs_addrmap with a check that the function has some psymtabs. This allows my original tests to pass, and the py-event.exp test to pass too. Now, a good argument can be made that we simply should never call find_pc_sect_psymtab on an objfile that is using indexes instead of partial_symtabs. I did consider this approach, we could easily add an assert into find_pc_sect_psymtab that if we find a partial_symtab in psymtabs_addrmap then the psymtabs pointer must be non-null. The responsibility would then be on the user of find_pc_sect_psymtab to ensure that the objfile being checked is suitable. In the end I didn't take this approach as the check in find_pc_sect_psymtab is cheap and this ensures that any future miss-uses of the function will not cause problems. I also extended the comment on psymtabs_addrmap to indicate that it holds more than just partial_symtabs as this was not at all clear from the original comment, and caused me some confusion when I was initially debugging this problem. gdb/ChangeLog: * psymtab.c (find_pc_sect_psymtab): Move baseaddr local into more inner scope, add check that the objfile has psymtabs before checking psymtabs_addrmap. * psymtab.h (psymtab_storage) : Extend comment. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 986a701c2f..9540c4f120 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2019-09-12 Andrew Burgess + + * psymtab.c (find_pc_sect_psymtab): Move baseaddr local into more + inner scope, add check that the objfile has psymtabs before + checking psymtabs_addrmap. + * psymtab.h (psymtab_storage) : Extend comment. + 2019-09-12 Philippe Waroquiers * NEWS: Announce that Ada task names are now shown at more places, diff --git a/gdb/psymtab.c b/gdb/psymtab.c index e9cc8c3bc6..031dbd9c1e 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -301,14 +301,24 @@ find_pc_sect_psymtab (struct objfile *objfile, CORE_ADDR pc, struct obj_section *section, struct bound_minimal_symbol msymbol) { - CORE_ADDR baseaddr = ANOFFSET (objfile->section_offsets, - SECT_OFF_TEXT (objfile)); - - /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity - than the later used TEXTLOW/TEXTHIGH one. */ - - if (objfile->partial_symtabs->psymtabs_addrmap != NULL) + /* Try just the PSYMTABS_ADDRMAP mapping first as it has better + granularity than the later used TEXTLOW/TEXTHIGH one. However, we need + to take care as the PSYMTABS_ADDRMAP can hold things other than partial + symtabs in some cases. + + This function should only be called for objfiles that are using partial + symtabs, not for objfiles that are using indexes (.gdb_index or + .debug_names), however 'maintenance print psymbols' calls this function + directly for all objfiles. If we assume that PSYMTABS_ADDRMAP contains + partial symtabs then we will end up returning a pointer to an object + that is not a partial_symtab, which doesn't end well. */ + + if (objfile->partial_symtabs->psymtabs != NULL + && objfile->partial_symtabs->psymtabs_addrmap != NULL) { + CORE_ADDR baseaddr = ANOFFSET (objfile->section_offsets, + SECT_OFF_TEXT (objfile)); + struct partial_symtab *pst = ((struct partial_symtab *) addrmap_find (objfile->partial_symtabs->psymtabs_addrmap, diff --git a/gdb/psymtab.h b/gdb/psymtab.h index aed686258d..0ad2b49d9a 100644 --- a/gdb/psymtab.h +++ b/gdb/psymtab.h @@ -109,7 +109,11 @@ public: /* Map addresses to the entries of PSYMTABS. It would be more efficient to have a map per the whole process but ADDRMAP cannot selectively remove its items during FREE_OBJFILE. This mapping is already present even for - PARTIAL_SYMTABs which still have no corresponding full SYMTABs read. */ + PARTIAL_SYMTABs which still have no corresponding full SYMTABs read. + + The DWARF parser reuses this addrmap to store things other than + psymtabs in the cases where debug information is being read from, for + example, the .debug-names section. */ struct addrmap *psymtabs_addrmap = nullptr;