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