[PATCH] Emit DWARF info for all code section in an assembly file

Eric Christopher echristo at gmail.com
Thu Apr 3 04:45:29 PDT 2014


  Could you pull the patch setting the dwarf version for the compile unit out into a separate patch? It seems pretty easily separable.

  Some additional comments inline as well.

  Thanks for all of the work here, it's shaping up nicely!

  -eric


================
Comment at: include/llvm/MC/MCContext.h:391
@@ +390,3 @@
+    }
+    void FinalizeDwarfSections(MCStreamer &MCOS) {
+      MCContext &context = MCOS.getContext();
----------------
Function names:

http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

Also these functions appear to be a bit heavyweight to be defined inline. Bring them out of line?

================
Comment at: include/llvm/MC/MCContext.h:138
@@ +137,3 @@
+    /// Symbols created for the start and end of each section, used for
+    /// generating the debug_aranges section
+    DenseMap<const MCSection*, MCSymbol*> GenDwarfSectionStartSyms;
----------------
Do you mean debug_aranges or debug_ranges? You use aranges here and then DW_AT_ranges below.

================
Comment at: tools/llvm-mc/llvm-mc.cpp:157
@@ +156,3 @@
+static cl::opt<int>
+DwarfVersion("dwarf-version", cl::desc("Dwarf version"), cl::init(3));
+
----------------
3 is a bit odd in that it's not the same as the defaults for any target :)

================
Comment at: lib/MC/MCDwarf.cpp:690
@@ +689,3 @@
+    // AT_ranges, the 4 byte offset from the start of the .debug_ranges section
+    // to the address range list for this compilation unit.
+    MCOS->EmitSymbolValue(RangesSectionSymbol, 4);
----------------
You still need a DW_AT_low_pc as the offset for the for the ranges section. Take a look at the code that does this for compile units in DwarfDebug.cpp (as of right now line 986) for a bit of an explanation.

================
Comment at: lib/MC/MCDwarf.cpp:812
@@ +811,3 @@
+// When generating dwarf for assembly source files this emits the data for
+// .debug_ranges section. We currently only emit one range list, which spans
+// all of the executable sections of this file.
----------------
I don't think we're likely to ever emit more than one range list for assembly files are we?

================
Comment at: lib/MC/MCDwarf.cpp:522
@@ -519,3 +521,3 @@
   EmitAbbrev(MCOS, dwarf::DW_AT_stmt_list, dwarf::DW_FORM_data4);
-  EmitAbbrev(MCOS, dwarf::DW_AT_low_pc, dwarf::DW_FORM_addr);
-  EmitAbbrev(MCOS, dwarf::DW_AT_high_pc, dwarf::DW_FORM_addr);
+  if (UseRangesSection) {
+    EmitAbbrev(MCOS, dwarf::DW_AT_ranges, dwarf::DW_FORM_data4);
----------------
Same comment on ranges as below.

================
Comment at: lib/MC/MCDwarf.cpp:513
@@ -510,3 +512,3 @@
 // the data for .debug_abbrev section which contains three DIEs.
-static void EmitGenDwarfAbbrev(MCStreamer *MCOS) {
+static void EmitGenDwarfAbbrev(MCStreamer *MCOS, bool UseRangesSection) {
   MCContext &context = MCOS->getContext();
----------------
Is the argument necessary or can you use the same lookup you do in other places?


http://llvm-reviews.chandlerc.com/D2697



More information about the llvm-commits mailing list