[PATCH] Remove wild .debug_aranges entries generated from unimportant labels

Richard Mitton richard at codersnotes.com
Wed Oct 2 14:00:42 PDT 2013


LGTM, although I dunno if I'm actually authorized to approve patches :)

There's code in there already that ignores labels in metadata sections, 
but it wasn't getting triggered because debug_loc labels are added after 
the test for it. And the stupid sectionless common symbols mean that it 
couldn't just ignore NULL sections either.

This looks like a fine fix.

Richard Mitton
richard at codersnotes.com

On 10/02/2013 01:33 PM, Alexey Samsonov wrote:
> Sure, here you go:
>
> $ cat tmp/debug_ranges/a.cc
> int global = 2;
> int foo(int bar) { return bar; }
> int foo2(int bar2) { return bar2; }
>
> int main() {
>   return foo(2) + foo2(1) + global;
> }
> $ ./bin/clang++ -g -O1 tmp/debug_ranges/a.cc
> $ readelf -wr a.out
> Contents of the .debug_aranges section:
>
>   Length:                   92
>   Version:                  2
>   Offset into .debug_info:  0x0
>   Pointer Size:             8
>   Segment Size:             0
>
>     Address            Length
>     0000000000000000 0000000000000001 <----- huh?
>     0000000000000036 0000000000000001 <----- hm?
>     0000000000402018 0000000000000004
>     0000000000400510 0000000000000041
>     0000000000000000 0000000000000000
>
>
> On Wed, Oct 2, 2013 at 10:23 PM, Richard Mitton 
> <richard at codersnotes.com <mailto:richard at codersnotes.com>> wrote:
>
>     How would I go about reproducing this bug? I'd really like to see
>     a test case.
>
>     Richard Mitton
>     richard at codersnotes.com <mailto:richard at codersnotes.com>
>
>
>     On 10/02/2013 07:10 AM, Alexey Samsonov wrote:
>
>         Hi echristo,
>
>         r191052 added emitting .debug_aranges to Clang, but this
>         functionality is broken: it uses all MC labels added in DWARF Asm
>         printer, including the labels for build relocations between
>         different DWARF sections, like .Lsection_line or .Ldebug_loc0.
>
>         As a result, if any DIE .debug_info would contain
>         "DW_AT_location=0x123"
>         attribute, .debug_aranges would also contain a range starting
>         from 0x123,
>         breaking tools that rely on this section.
>
>         I'm not sure this is the correct fix, please take a look.
>
>         http://llvm-reviews.chandlerc.com/D1809
>
>         Files:
>            lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>            lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>            lib/CodeGen/AsmPrinter/DwarfDebug.h
>
>         Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>         ===================================================================
>         --- lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>         +++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>         @@ -1110,8 +1110,8 @@
>             void DwarfDebug::endSections() {
>              // Filter labels by section.
>         -  for (size_t n = 0; n < Labels.size(); n++) {
>         -    const SymbolCU &SCU = Labels[n];
>         +  for (size_t n = 0; n < ArangeLabels.size(); n++) {
>         +    const SymbolCU &SCU = ArangeLabels[n];
>               if (SCU.Sym->isInSection()) {
>                 // Make a note of this symbol and it's section.
>                 const MCSection *Section = &SCU.Sym->getSection();
>         @@ -1138,10 +1138,7 @@
>               }
>                 // Insert a final terminator.
>         -    SymbolCU Entry;
>         -    Entry.CU = NULL;
>         -    Entry.Sym = Sym;
>         -    SectionMap[Section].push_back(Entry);
>         +    SectionMap[Section].push_back(SymbolCU(NULL, Sym));
>             }
>           }
>           Index: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>         ===================================================================
>         --- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>         +++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>         @@ -181,26 +181,15 @@
>                                      const MCSymbol *Label) {
>             DIEValue *Value = new (DIEValueAllocator) DIELabel(Label);
>             Die->addValue(Attribute, Form, Value);
>         -
>         -  SymbolCU Entry;
>         -  Entry.CU = this;
>         -  Entry.Sym = Label;
>         -
>         -  DD->addLabel(Entry);
>           }
>             /// addLabelAddress - Add a dwarf label attribute data and
>         value using
>           /// DW_FORM_addr or DW_FORM_GNU_addr_index.
>           ///
>           void CompileUnit::addLabelAddress(DIE *Die, uint16_t Attribute,
>                                             MCSymbol *Label) {
>         -  if (Label) {
>         -    SymbolCU Entry;
>         -    Entry.CU = this;
>         -    Entry.Sym = Label;
>         -
>         -    DD->addLabel(Entry);
>         -  }
>         +  if (Label)
>         +    DD->addArangeLabel(SymbolCU(this, Label));
>               if (!DD->useSplitDwarf()) {
>               if (Label != NULL) {
>         @@ -221,6 +210,7 @@
>           /// form given and an op of either DW_FORM_addr or
>         DW_FORM_GNU_addr_index.
>           ///
>           void CompileUnit::addOpAddress(DIE *Die, const MCSymbol *Sym) {
>         +  DD->addArangeLabel(SymbolCU(this, Sym));
>             if (!DD->useSplitDwarf()) {
>               addUInt(Die, 0, dwarf::DW_FORM_data1, dwarf::DW_OP_addr);
>               addLabel(Die, 0, dwarf::DW_FORM_udata, Sym);
>         Index: lib/CodeGen/AsmPrinter/DwarfDebug.h
>         ===================================================================
>         --- lib/CodeGen/AsmPrinter/DwarfDebug.h
>         +++ lib/CodeGen/AsmPrinter/DwarfDebug.h
>         @@ -303,6 +303,7 @@
>             /// \brief Helper used to pair up a symbol and it's DWARF
>         compile unit.
>           struct SymbolCU {
>         +  SymbolCU(CompileUnit *CU, const MCSymbol *Sym) : Sym(Sym),
>         CU(CU) {}
>             const MCSymbol *Sym;
>             CompileUnit *CU;
>           };
>         @@ -363,8 +364,8 @@
>             // separated by a zero byte, mapped to a unique id.
>             StringMap<unsigned, BumpPtrAllocator&> SourceIdMap;
>           -  // List of all labels used in the output.
>         -  std::vector<SymbolCU> Labels;
>         +  // List of all labels used in aranges generation.
>         +  std::vector<SymbolCU> ArangeLabels;
>               // Size of each symbol emitted (for those symbols that
>         have a specific size).
>             DenseMap <const MCSymbol *, uint64_t> SymSize;
>         @@ -731,7 +732,7 @@
>             void addTypeUnitType(DIE *Die) { TypeUnits.push_back(Die); }
>               /// \brief Add a label so that arange data can be
>         generated for it.
>         -  void addLabel(SymbolCU SCU) { Labels.push_back(SCU); }
>         +  void addArangeLabel(SymbolCU SCU) {
>         ArangeLabels.push_back(SCU); }
>               /// \brief For symbols that have a size designated (e.g.
>         common symbols),
>             /// this tracks that size.
>
>
>
>
>
> -- 
> Alexey Samsonov, MSK

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131002/80afb0cb/attachment.html>


More information about the llvm-commits mailing list