[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