[PATCH] Add supporting for dumping the COFF debug FPO section.
Timur Iskhodzhanov
timurrrr at google.com
Mon Feb 2 07:22:06 PST 2015
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.
================
Comment at: include/llvm/Support/COFF.h:678
@@ +677,3 @@
+ FPO_SYSTEM_EXCEPTION_HANDLING = 0x00000001,
+ FPO_CXX_EXCEPTION_HANDLING = 0x00000002,
+ FPO_FUNCTION_ENTRY = 0x00000004
----------------
This constant is not used anywhere in the tests.
================
Comment at: test/tools/llvm-readobj/codeview-linetables.test:119
@@ +118,3 @@
+MFUN32-NEXT: Start: 0x0
+MFUN32-NEXT: CodeSize: 0xA
+MFUN32-NEXT: LocalsSize: 0x0
----------------
Is this format documented somewhere?
Otherwise, are you working on FPO table generation in LLVM?
================
Comment at: test/tools/llvm-readobj/codeview-linetables.test:123
@@ +122,3 @@
+MFUN32-NEXT: MaxStackSize: 0x0
+MFUN32-NEXT: ProgramStr: $T0 $ebp = $eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + =
+MFUN32-NEXT: PrologSize: 0x3
----------------
Looks like the same ProgramStr is generated for all these test functions.
================
Comment at: tools/llvm-readobj/COFFDumper.cpp:521
@@ +520,3 @@
+ case COFF::DEBUG_FPO_SUBSECTION: {
+ // Holds a PC to file:line table. Some data to parse this subsection is
+ // stored in the other subsections, so just check sanity and store the
----------------
I think you've forgotten to update the comment.
================
Comment at: tools/llvm-readobj/COFFDumper.cpp:537
@@ +536,3 @@
+ W.printString("FunctionName", FunctionName);
+ if (FunctionFPOTables.count(FunctionName) != 0) {
+ // Saw debug info for this function already?
----------------
nit: offset
================
Comment at: tools/llvm-readobj/COFFDumper.cpp:582
@@ +581,3 @@
+ if (FunctionLineTables.find(Name) != FunctionLineTables.end()) {
+ ListScope S(W, "FunctionLineTable");
+ W.printString("FunctionName", Name);
----------------
the body of this `for` loop is getting hard to read. Maybe it's time to split it into a few subroutines?
http://reviews.llvm.org/D6017
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list