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

[David Blaikie]

On Wed, Oct 2, 2013 at 2:00 PM, Richard Mitton <richard at codersnotes.com>wrote:

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

If you wouldn't or couldn't commit a patch yourself, then you can't really
sign off on that patch. (good that you pointed this out, though - I'm just
clarifying so you have a better understanding of how this works) that's not
to say you can't comment on a patch, just helps to be clear that you're not
suggesting this person commit based on your response.


>
> 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 Mittonrichard 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>wrote:
>
>> How would I go about reproducing this bug? I'd really like to see a test
>> case.
>>
>> Richard Mitton
>> 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/a5dd517a/attachment.html>