<p dir="ltr"><br>
On Sep 18, 2014 9:58 AM, "Frédéric Riss" <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
><br>
><br>
>> On 18 Sep 2014, at 18:53, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Thu, Sep 18, 2014 at 9:51 AM, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
>>><br>
>>> I pushed a partial revert in r218041 that should fix it.<br>
>><br>
>><br>
>> Thanks - helps to get the bots green sooner rather than later.<br>
>><br>
>> Are you planning to investigate a deeper fix here? (at some point, at least a test case would be good so we can catch this with the regression suite, rather than waiting for the bot to pick it up)<br>
><br>
><br>
> I was planning on adding tests for the added declarations when I get the forward declaration stuff totally in tree. But Now that I’ve broken this, I kinda feel a moral obligation to add a test for it :-) <br>
> But I might first investigate if we cannot simply remove the definition/declaration split for variables not in the unit scope (which would change this code anyway).</p>
<p dir="ltr">Sounds good - thanks!</p>
<p dir="ltr">><br>
> Fred<br>
><br>
>> <br>
>>><br>
>>><br>
>>> Fred<br>
>>><br>
>>>> On 18 Sep 2014, at 17:48, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>>><br>
>>>> Here's a simple example:<br>
>>>><br>
>>>> namespace x {<br>
>>>> int i = 3;<br>
>>>> }<br>
>>>> int j = x::i;<br>
>>>><br>
>>>> LLVM will emit a declaration for 'i' in namespace x and a definition at the CU scope referencing the declaration. With your change, we end up omitting the DW_AT_declaration on the declaration of 'i'. The debugger then thinks it's a definition without a location, so any attempt to use/print the variable produces "this value has been optimized away".<br>
>>>><br>
>>>> A couple of side notes:<br>
>>>><br>
>>>> 1) We should ideally not use "Verify()" for any debug info functionality - it should be enough to test "isBlah" methods. Any time isBlah is true but Verify fails is a bug we should fix at some point - I think that's a valid/useful invariant, but I could be wrong.<br>
>>>><br>
>>>> 2) we don't actually need to separate declaration and definition in this way, just because the variable is inside a namespace - so if you happen to fix this by removing the separation of declaration from definition, that's OK too (& a minor win to reduce debug info size)<br>
>>>><br>
>>>> On Thu, Sep 18, 2014 at 8:28 AM, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
>>>>><br>
>>>>><br>
>>>>>> On 18 Sep 2014, at 17:23, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>>>>><br>
>>>>>> Looks like this might've regressed some debug scenarios:<br>
>>>>>><br>
>>>>>> <a href="http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/17377">http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/17377</a><br>
>>>>>><br>
>>>>>> (could be r218040, but it seems like the less likely candidate of the two on the blame list)<br>
>>>>><br>
>>>>><br>
>>>>> I’ve seen it. I sent a mail to the review thread asking for some help from someone that can run the test suite.<br>
>>>>><br>
>>>>> Fred<br>
>>>>><br>
>>>>>> On Thu, Sep 18, 2014 at 2:38 AM, Frederic Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
>>>>>>><br>
>>>>>>> Author: friss<br>
>>>>>>> Date: Thu Sep 18 04:38:23 2014<br>
>>>>>>> New Revision: 218041<br>
>>>>>>><br>
>>>>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218041&view=rev">http://llvm.org/viewvc/llvm-project?rev=218041&view=rev</a><br>
>>>>>>> Log:<br>
>>>>>>> Always emit DW_AT_declaration attribute when the variable isn't a definition.<br>
>>>>>>><br>
>>>>>>> Summary:<br>
>>>>>>> This doesn't show up today as we don't emit decalration only variables. This<br>
>>>>>>> will be tested when the followup patches implementing import of forward<br>
>>>>>>> declared entities lands in clang.<br>
>>>>>>><br>
>>>>>>> Reviewers: echristo, dblaikie, aprantl<br>
>>>>>>><br>
>>>>>>> Subscribers: llvm-commits<br>
>>>>>>><br>
>>>>>>> Differential Revision: <a href="http://reviews.llvm.org/D5382">http://reviews.llvm.org/D5382</a><br>
>>>>>>><br>
>>>>>>> Modified:<br>
>>>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
>>>>>>><br>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
>>>>>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=218041&r1=218040&r2=218041&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=218041&r1=218040&r2=218041&view=diff</a><br>
>>>>>>> ==============================================================================<br>
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)<br>
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Thu Sep 18 04:38:23 2014<br>
>>>>>>> @@ -1670,6 +1670,9 @@ void DwarfCompileUnit::createGlobalVaria<br>
>>>>>>> DD->addArangeLabel(SymbolCU(this, Sym));<br>
>>>>>>> addOpAddress(*Loc, Sym);<br>
>>>>>>> }<br>
>>>>>>> + // A static member's declaration is already flagged as such.<br>
>>>>>>> + if (!SDMDecl.Verify() && !GV.isDefinition())<br>
>>>>>>> + addFlag(*VariableDIE, dwarf::DW_AT_declaration);<br>
>>>>>>> // Do not create specification DIE if context is either compile unit<br>
>>>>>>> // or a subprogram.<br>
>>>>>>> if (GVContext && GV.isDefinition() && !GVContext.isCompileUnit() &&<br>
>>>>>>> @@ -1678,9 +1681,6 @@ void DwarfCompileUnit::createGlobalVaria<br>
>>>>>>> VariableSpecDIE = &createAndAddDIE(dwarf::DW_TAG_variable, UnitDie);<br>
>>>>>>> addDIEEntry(*VariableSpecDIE, dwarf::DW_AT_specification, *VariableDIE);<br>
>>>>>>> addBlock(*VariableSpecDIE, dwarf::DW_AT_location, Loc);<br>
>>>>>>> - // A static member's declaration is already flagged as such.<br>
>>>>>>> - if (!SDMDecl.Verify())<br>
>>>>>>> - addFlag(*VariableDIE, dwarf::DW_AT_declaration);<br>
>>>>>>> } else {<br>
>>>>>>> addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);<br>
>>>>>>> }<br>
>>>>>>><br>
>>>>>>><br>
>>>>>>> _______________________________________________<br>
>>>>>>> llvm-commits mailing list<br>
>>>>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>>>>><br>
>>>>>><br>
>>>>><br>
>>>><br>
>>><br>
>><br>
><br>
</p>