[PATCH] Support for generating DWARF .debug_aranges sections.

Richard Mitton richard at codersnotes.com
Tue Sep 17 18:15:46 PDT 2013


Thanks for the comments! I'll update the patch, and I'll add some 
comments as requested.
> ================
> 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.
Not entirely sure what you mean there, it already includes text and data 
sections if they exist in the output.

> ================
> Comment at: lib/MC/MCStreamer.cpp:200
> @@ +199,3 @@
> +
> +  SymbolOrdering[Symbol] = 1 + SymbolOrdering.size();
> +}
> ----------------
> Magic numbers are magic. What's with the +1? :)
It's to keep zero reserved as 'invalid'. I should comment that :)

Richard Mitton
richard at codersnotes.com

On 09/17/2013 04:50 PM, Eric Christopher wrote:
>    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