[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