diff options
author | Elliott Hughes <enh@google.com> | 2019-12-04 13:44:28 -0800 |
---|---|---|
committer | Elliott Hughes <enh@google.com> | 2019-12-04 13:44:28 -0800 |
commit | ab7d77d861caea4f4dca0392eea25a316c328cd6 (patch) | |
tree | 8bf356a97efd8f717a43fed6e9406c750d1d0248 | |
parent | 4d64623fc5a88a041fbb0ad5f4ad9d65cb0d4b47 (diff) | |
download | gdb-ab7d77d861caea4f4dca0392eea25a316c328cd6.tar.gz |
gdb: Don't fault for 'maint print psymbols' when using an index
Cherrypick of upstream 3dd9bb462012df685d6d41300dacedae1c81e28a.
Original commit message:
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) <psymtabs_addrmap>: Extend comment.
Bug: http://b/144102122
Test: gdbclient.py -p `adb shell pgrep dialer`
Change-Id: I0ad68ac5dd29d86d5b1df06cca5fe02206afcfdb
-rw-r--r-- | gdb-8.3/gdb/psymtab.c | 24 | ||||
-rw-r--r-- | gdb-8.3/gdb/psymtab.h | 6 |
2 files changed, 22 insertions, 8 deletions
diff --git a/gdb-8.3/gdb/psymtab.c b/gdb-8.3/gdb/psymtab.c index 0a4502e4f..682696a7e 100644 --- a/gdb-8.3/gdb/psymtab.c +++ b/gdb-8.3/gdb/psymtab.c @@ -312,14 +312,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-8.3/gdb/psymtab.h b/gdb-8.3/gdb/psymtab.h index 3ee5eee0b..24be1d59f 100644 --- a/gdb-8.3/gdb/psymtab.h +++ b/gdb-8.3/gdb/psymtab.h @@ -112,7 +112,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; |