This is an archive of the discontinued LLVM Phabricator instance.

[LLDB] [PECOFF] Use FindSectionByID to associate symbols to sections
ClosedPublic

Authored by mstorsjo on Oct 24 2019, 12:20 AM.

Details

Summary

The virtual container/header section caused the section list to be offset by one, but by using FindSectionByID, the layout of the section list shouldn't matter.

Diff Detail

Event Timeline

mstorsjo created this revision.Oct 24 2019, 12:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2019, 12:20 AM
mstorsjo updated this revision to Diff 226216.Oct 24 2019, 2:22 AM

Updated the testcase to check for symbols in more than one section.

labath added inline comments.Oct 24 2019, 9:35 AM
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
691

You should use FindSectionByID here. That's the API used by both ELF and MachO object files and SymbolFilePDB classes for this sort of thing. This would obviate the need for the long comment and make this patch independent of the header stuff (which I'm going to commit as soon as I figure out how that's done).

lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
3

At some point we'll probably want to add some kind of a symtab dumping mechanism to the object-file subcommand, but this is fine until then.

5

Could you also add a CHECK line with the table header, just so one gets the idea of what is the meaning of individual fields..

amccarth added inline comments.Oct 24 2019, 10:50 AM
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
691

+1 to labath's suggestion. It seems unfortunate that this already had a coupling to implementation details of the section list.

mstorsjo updated this revision to Diff 226321.Oct 24 2019, 1:56 PM
mstorsjo retitled this revision from [LLDB] [PECOFF] Fix symbols to refer to the right section to [LLDB] [PECOFF] Use FindSectionByID to associate symbols to sections.
mstorsjo edited the summary of this revision. (Show Details)

Using FindSectionByID, added a table header for readability to the test.

labath accepted this revision.Oct 24 2019, 2:05 PM
This revision is now accepted and ready to land.Oct 24 2019, 2:05 PM
This revision was automatically updated to reflect the committed changes.