[PATCH] Support for generating DWARF .debug_aranges sections.

Eric Christopher echristo at gmail.com
Tue Sep 17 16:50:14 PDT 2013


  In general looks OK, needs testcases as I mentioned and could use some comments in a few places. Thanks for the work.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1050
@@ +1049,3 @@
+
+  // Add terminating symbols for each section.
+  unsigned I = 1;
----------------
Might be nice to pull in the end of text and data into this.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:303
@@ -302,1 +302,3 @@
 
+struct SymbolCU {
+  const MCSymbol *Sym;
----------------
Needs commentary.

================
Comment at: include/llvm/MC/MCStreamer.h:89
@@ -88,1 +88,3 @@
 
+  DenseMap<const MCSymbol *, unsigned> SymbolOrdering;
+
----------------
Needs a comment to say what this is used for.

================
Comment at: lib/MC/MCStreamer.cpp:200
@@ +199,3 @@
+
+  SymbolOrdering[Symbol] = 1 + SymbolOrdering.size();
+}
----------------
Magic numbers are magic. What's with the +1? :)

================
Comment at: include/llvm/Support/Dwarf.h:62
@@ -62,1 +61,3 @@
+  DW_PUBNAMES_VERSION = 2, // Section version number for .debug_pubnames.
+  DW_ARANGES_VERSION = 2   // Section version number for .debug_aranges.
 };
----------------
Make sure to explicitly test for this in the testcase.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2638
@@ +2637,3 @@
+
+    // Emit size of content not including length itself
+    unsigned ContentSize
----------------
Period at the end of a sentence.


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



More information about the llvm-commits mailing list