Page MenuHomePhabricator

Add supporting for dumping the COFF debug FPO section.
Needs ReviewPublic

Authored by jrmuizel on Oct 28 2014, 11:36 AM.

Details

Reviewers
majnemer
timurrrr

Diff Detail

Event Timeline

jrmuizel updated this revision to Diff 15527.Oct 28 2014, 11:36 AM
jrmuizel retitled this revision from to Add supporting for dumping the COFF debug FPO section..
jrmuizel updated this object.
jrmuizel edited the test plan for this revision. (Show Details)
jrmuizel added a reviewer: timurrrr.
jrmuizel added a subscriber: Unknown Object (MLST).
timurrrr edited edge metadata.Oct 30 2014, 8:43 AM

Hi Jeff,

I'm currently traveling, so can't look at your patch yet. I'll try to take
a look in the next couple is weeks!

Tim

вт, 28 окт. 2014, 11:36, Jeff Muizelaar <jmuizelaar@mozilla.com>:

Hi timurrrr,

http://reviews.llvm.org/D6017

Files:

include/llvm/Support/COFF.h
test/tools/llvm-readobj/codeview-linetables.test
tools/llvm-readobj/COFFDumper.cpp

Sorry for missing this, I was travelling and later overwhelmed by a backlog of breakages... Thanks for reminding!

General comments:

  • Are you working on FPO table generation in LLVM?
    • If no, why do you need to dump this?
    • Either way, do you need to dump all the fields of the FPO subsection or just a subset suffice?
  • You probably need to add more test cases specifically to cover more FPO-related corner cases rather than modify existing tests.

Adding David who knows COFF better than I do.

include/llvm/Support/COFF.h
678

This constant is not used anywhere in the tests.

test/tools/llvm-readobj/codeview-linetables.test
119

Is this format documented somewhere?
Otherwise, are you working on FPO table generation in LLVM?

123

Looks like the same ProgramStr is generated for all these test functions.

tools/llvm-readobj/COFFDumper.cpp
521

I think you've forgotten to update the comment.

537

nit: offset

582

the body of this for loop is getting hard to read. Maybe it's time to split it into a few subroutines?

jrmuizel added inline comments.Feb 2 2015, 7:20 PM
test/tools/llvm-readobj/codeview-linetables.test
119

The format is not documented anywhere. I reconstructed it from the wine dbghelp implementation and https://msdn.microsoft.com/en-us/library/zxsd009h.aspx.
I am also working on FPO table generation for LLVM.

123

I can try to get more diverse set of inputs to test the output with.

tools/llvm-readobj/COFFDumper.cpp
582

Sure.