[llvm] r218041 - Always emit DW_AT_declaration attribute when the variable isn't a definition.

[Frédéric Riss]

> On 18 Sep 2014, at 18:53, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Thu, Sep 18, 2014 at 9:51 AM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
> I pushed a partial revert in r218041 that should fix it.
> 
> Thanks - helps to get the bots green sooner rather than later.
> 
> 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)

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 :-) 
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).

Fred

>  
> 
> Fred
> 
>> On 18 Sep 2014, at 17:48, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> Here's a simple example:
>> 
>>   namespace x {
>>     int i = 3;
>>   }
>>   int j = x::i;
>> 
>> 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".
>> 
>> A couple of side notes:
>> 
>> 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.
>> 
>> 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)
>> 
>> On Thu, Sep 18, 2014 at 8:28 AM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> 
>>> On 18 Sep 2014, at 17:23, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>> 
>>> Looks like this might've regressed some debug scenarios:
>>> 
>>> 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>
>>> 
>>> (could be r218040, but it seems like the less likely candidate of the two on the blame list)
>> 
>> I’ve seen it. I sent a mail to the review thread asking for some help from someone that can run the test suite.
>> 
>> Fred
>> 
>>> On Thu, Sep 18, 2014 at 2:38 AM, Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>>> Author: friss
>>> Date: Thu Sep 18 04:38:23 2014
>>> New Revision: 218041
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=218041&view=rev <http://llvm.org/viewvc/llvm-project?rev=218041&view=rev>
>>> Log:
>>> Always emit DW_AT_declaration attribute when the variable isn't a definition.
>>> 
>>> Summary:
>>> This doesn't show up today as we don't emit decalration only variables. This
>>> will be tested when the followup patches implementing import of forward
>>> declared entities lands in clang.
>>> 
>>> Reviewers: echristo, dblaikie, aprantl
>>> 
>>> Subscribers: llvm-commits
>>> 
>>> Differential Revision: http://reviews.llvm.org/D5382 <http://reviews.llvm.org/D5382>
>>> 
>>> Modified:
>>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>> 
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>> URL: 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>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Thu Sep 18 04:38:23 2014
>>> @@ -1670,6 +1670,9 @@ void DwarfCompileUnit::createGlobalVaria
>>>        DD->addArangeLabel(SymbolCU(this, Sym));
>>>        addOpAddress(*Loc, Sym);
>>>      }
>>> +    // A static member's declaration is already flagged as such.
>>> +    if (!SDMDecl.Verify() && !GV.isDefinition())
>>> +      addFlag(*VariableDIE, dwarf::DW_AT_declaration);
>>>      // Do not create specification DIE if context is either compile unit
>>>      // or a subprogram.
>>>      if (GVContext && GV.isDefinition() && !GVContext.isCompileUnit() &&
>>> @@ -1678,9 +1681,6 @@ void DwarfCompileUnit::createGlobalVaria
>>>        VariableSpecDIE = &createAndAddDIE(dwarf::DW_TAG_variable, UnitDie);
>>>        addDIEEntry(*VariableSpecDIE, dwarf::DW_AT_specification, *VariableDIE);
>>>        addBlock(*VariableSpecDIE, dwarf::DW_AT_location, Loc);
>>> -      // A static member's declaration is already flagged as such.
>>> -      if (!SDMDecl.Verify())
>>> -        addFlag(*VariableDIE, dwarf::DW_AT_declaration);
>>>      } else {
>>>        addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
>>>      }
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>> 
>> 
>> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140918/3ca4d6de/attachment.html>