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

[David Blaikie]

On Sep 18, 2014 9:58 AM, "Frédéric Riss" <friss at apple.com> wrote:
>
>
>> 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> 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).

Sounds good - thanks!

>
> Fred
>
>>
>>>
>>>
>>> Fred
>>>
>>>> On 18 Sep 2014, at 17:48, David Blaikie <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> wrote:
>>>>>
>>>>>
>>>>>> On 18 Sep 2014, at 17:23, David Blaikie <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
>>>>>>
>>>>>> (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>
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
>>>>>>> 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
>>>>>>>
>>>>>>> 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
>>>>>>>
==============================================================================
>>>>>>> --- 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
>>>>>>> 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/882a4ddc/attachment.html>