Page MenuHomePhabricator

Teach llvm-readobj to dump symbol records from a DEBUG_SYMBOL_SUBSECTION section.
ClosedPublic

Authored by zturner on Feb 16 2015, 1:37 AM.

Details

Summary

Subsections of type DEBUG_SYMBOL_SUBSECTION consist of a series of records. Each record begins with a 4 byte length, followed by 4 byte type, followed by <length> bytes of data.

This was already known, but we only dumped DEBUG_SYMBOL_TYPE_PROC_START and DEBUG_SYMBOL_TYPE_PROC_END. This patch makes llvm-readobj dump the Length, type, and payload of other types of entries as individaul records so that they can be studied more closely.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner updated this revision to Diff 20004.Feb 16 2015, 1:37 AM
zturner retitled this revision from to Teach llvm-readobj to dump symbol records from a DEBUG_SYMBOL_SUBSECTION section..
zturner updated this object.
zturner edited the test plan for this revision. (Show Details)
zturner added reviewers: timurrrr, majnemer.
zturner added a subscriber: Unknown Object (MLST).

The reason for removing references to "linetables" and moving towards the more generic "codeview" terminology is because the .debug$S section contains much more than just linetable information. Previously we were really only interested in the linetables, but I have interest in everything in the section now, and hope to soon be able to decipher some of the other type codes and dump them as well. Additionally, we will need to be able to dump the .debug$T section as well, which contains type information, so it seems appropriate to not call it "codeview linetable information" anymore and instead just call it "codeview debug information"

timurrrr edited edge metadata.Feb 16 2015, 8:19 AM

Changing scope and renaming: LGTM

You might want to reconsider some details after reading the comments.

tools/llvm-readobj/COFFDumper.cpp
509 ↗(On Diff #20004)

We might want to put this under an if () (see the other comment at the deifinition of CodeViewSymbols)

701 ↗(On Diff #20004)

nit: I suggest either

W.printHex("Size", Size);

or rename Size to Length?
Or SegmentSize?

tools/llvm-readobj/llvm-readobj.h
40 ↗(On Diff #20004)

We might consider reusing SectionSymbols?

If the main purpose of CodeViewSymbols is to dump subsections we can't parse/dump yet, maybe we should use CodeViewDumpUnknownParts or something?

zturner updated this revision to Diff 20062.Feb 16 2015, 4:44 PM
zturner edited edge metadata.

Update review with suggested changes.

timurrrr added inline comments.Feb 17 2015, 4:27 AM
tools/llvm-readobj/COFFDumper.cpp
509 ↗(On Diff #20062)

I meant we might want to put the

W.printBinaryBlock("Contents", Contents);

line under a CodeViewSomethingUnknownSomething condition.

tools/llvm-readobj/llvm-readobj.h
40 ↗(On Diff #20062)

might want to generalize to "CodeViewDumpUnknownRecords` or something.

zturner updated this revision to Diff 20128.Feb 17 2015, 5:37 PM

Wasn't sure if the name is getting too long / annoying to type. But since I'm reusing the variable for both the bytes of the entire subsection, as well as the byets of the individual records, I think subsection-bytes more accurately describes the intent than something about "unknown". I don't feel strongly about the name though, so let me know if you prefer something else.

timurrrr accepted this revision.Feb 18 2015, 8:35 AM
timurrrr edited edge metadata.
timurrrr added inline comments.
tools/llvm-readobj/COFFDumper.cpp
713 ↗(On Diff #20128)

this indentation looks weird. Is it how clang-format formats this?

This revision is now accepted and ready to land.Feb 18 2015, 8:35 AM

I think it's a consequence of us not indenting case labels. I'll double
check that i clang-formatted it later though

This revision was automatically updated to reflect the committed changes.

Thanks for the improvements in the dumper!