r205952 - Add global static variables for anonymous union fields. This makes
Eric Christopher
echristo at gmail.com
Fri Apr 11 14:08:52 PDT 2014
On Thu, Apr 10, 2014 at 9:52 AM, David Blaikie <dblaikie at gmail.com> wrote:
> All that being said, I have been a proponent of "whatever works for
> the debugger(s)" in the past - though this might meet my "infrequent
> used feature that I'd rather not carry the legacy of compatibility for
> & instead let the debugger bear the burden of bugs/fixing". But
> perhaps that's just personal bias due to the particular area of
> impact.
>
> I get that/assume this also has interesting interactions with pubnames
> (& thus representing these as global variables makes it easier for
> pubnames to get the names given our implementation, etc).
>
Right, while it seems possible for the debugger to have guessed "print
a" as "there's no a, but hey, is a an anonymous union at global scope
that could have injected its contents into the current scope let's go
through every member of the type and see if it's there", but that
seems a bit much to assume on their part. :)
As far as the source fidelity thing, I don't have a particular color
for the shed here and don't mind either way. Ditto with whether or not
we put a variable with no name for the union itself.
-eric
>
> On Thu, Apr 10, 2014 at 9:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Wed, Apr 9, 2014 at 10:20 PM, Eric Christopher <echristo at gmail.com> wrote:
>>> Author: echristo
>>> Date: Thu Apr 10 00:20:00 2014
>>> New Revision: 205952
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=205952&view=rev
>>> Log:
>>> Add global static variables for anonymous union fields.
>>
>> This loses source fidelity though, doesn't it? We no longer describe
>> which of these fields are mutually exclusive, etc.
>>
>> We manage to get this correct for member variables that are anonymous
>> unions - why wouldn't the same behavior be correct for globals?
>>
>>> This makes
>>> sure that a debugger can find them when stepping through code,
>>> for example from the included testcase:
>>
>> Sounds like this is a debugger bug if it couldn't manage it with the
>> previous representation, no?
>>
>> Well, actually - this looks like it might just be a compiler bug that
>> we accept this entity at all... gcc doesn't seem to accept this code.
>> I don't know why we would either. GCC 4.6, 4.7, 4.8.1, and ToT all
>> warn that the static anonymous struct "defines an unnamed struct/union
>> with no instances" (rather than introducing a global unnamed
>> struct/union).
>>
>> Hmm, nope - just a piece of C++11 that GCC doesn't implement.
>>
>> Though GCC and Clang do both implement anonymous unions at function
>> scope and both implement the debug info as this patch does for
>> globals. (with the same loss in source fidelity)
>>
>> Wait, no, GCC does something weird - it includes variables directly in
>> the function's lexical block /and/ it includes the union:
>>
>> void f() {
>> union { int x, y; };
>> ...
>> }
>>
>> looks like:
>> subprogram x
>> variable int x at location foo
>> variable int y at location foo
>> variable anon at location foo
>> union anon
>> member int x
>> member int y
>>
>> So we've got some inconsistency here... :/
>>
>> Sounds like GCC was working around the same debugger limitation but
>> with a little better source fidelity. If possible, I'd rather we just
>> fix the debugger and only produce the dwarf that describes the
>> source...
>>
>> (haven't looked in detail at the code for this patch yet - just the
>> change in behavior)
>>
>>>
>>> 12 int test_it() {
>>> 13 c = 1;
>>> 14 d = 2;
>>> -> 15 a = 4;
>>> 16 return (c == 1);
>>> 17 }
>>> 18
>>> (lldb) p a
>>> (int) $0 = 2
>>> (lldb) p c
>>> (int) $1 = 2
>>> (lldb) p d
>>> (int) $2 = 2
>>>
>>> and a, c, d are all part of the file static anonymous union:
>>>
>>> static union {
>>> int c;
>>> int d;
>>> union {
>>> int a;
>>> };
>>> struct {
>>> int b;
>>> };
>>> };
>>>
>>> Fixes PR19221.
>>>
>>> Added:
>>> cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp
>>> Modified:
>>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> cfe/trunk/lib/CodeGen/CGDebugInfo.h
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205952&r1=205951&r2=205952&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Apr 10 00:20:00 2014
>>> @@ -3180,6 +3180,37 @@ CGDebugInfo::getOrCreateStaticDataMember
>>> return T;
>>> }
>>>
>>> +/// Recursively collect all of the member fields of a global anonymous decl and
>>> +/// create static variables for them. The first time this is called it needs
>>> +/// to be on a union and then from there we can have additional unnamed fields.
>>> +llvm::DIGlobalVariable
>>> +CGDebugInfo::CollectAnonRecordDecls(const RecordDecl *RD, llvm::DIFile Unit,
>>> + unsigned LineNo, StringRef LinkageName,
>>> + llvm::GlobalVariable *Var,
>>> + llvm::DIDescriptor DContext) {
>>> + llvm::DIGlobalVariable GV;
>>> +
>>> + for (const auto *Field : RD->fields()) {
>>> + llvm::DIType FieldTy = getOrCreateType(Field->getType(), Unit);
>>> + StringRef FieldName = Field->getName();
>>> +
>>> + // Ignore unnamed fields, but recurse into anonymous records.
>>> + if (FieldName.empty()) {
>>> + const RecordType *RT = dyn_cast<RecordType>(Field->getType());
>>> + if (RT)
>>> + GV = CollectAnonRecordDecls(RT->getDecl(), Unit, LineNo, LinkageName,
>>> + Var, DContext);
>>> + continue;
>>> + }
>>> + // Use VarDecl's Tag, Scope and Line number.
>>> + GV = DBuilder.createStaticVariable(DContext, FieldName, LinkageName, Unit,
>>> + LineNo, FieldTy,
>>> + Var->hasInternalLinkage(), Var,
>>> + llvm::DIDerivedType());
>>> + }
>>> + return GV;
>>> +}
>>> +
>>> /// EmitGlobalVariable - Emit information about a global variable.
>>> void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
>>> const VarDecl *D) {
>>> @@ -3200,19 +3231,35 @@ void CGDebugInfo::EmitGlobalVariable(llv
>>> T = CGM.getContext().getConstantArrayType(ET, ConstVal,
>>> ArrayType::Normal, 0);
>>> }
>>> +
>>> StringRef DeclName = D->getName();
>>> StringRef LinkageName;
>>> - if (D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext())
>>> - && !isa<ObjCMethodDecl>(D->getDeclContext()))
>>> + if (D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext()) &&
>>> + !isa<ObjCMethodDecl>(D->getDeclContext()))
>>> LinkageName = Var->getName();
>>> if (LinkageName == DeclName)
>>> LinkageName = StringRef();
>>> +
>>> llvm::DIDescriptor DContext =
>>> getContextDescriptor(dyn_cast<Decl>(D->getDeclContext()));
>>> - llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
>>> - DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
>>> - Var->hasInternalLinkage(), Var,
>>> - getOrCreateStaticDataMemberDeclarationOrNull(D));
>>> +
>>> + // Attempt to store one global variable for the declaration - even if we
>>> + // emit a lot of fields.
>>> + llvm::DIGlobalVariable GV;
>>> +
>>> + // If this is an anonymous union then we'll want to emit a global
>>> + // variable for each member of the anonymous union so that it's possible
>>> + // to find the name of any field in the union.
>>> + if (T->isUnionType() && DeclName.empty()) {
>>> + const RecordDecl *RD = cast<RecordType>(T)->getDecl();
>>> + assert(RD->isAnonymousStructOrUnion() && "unnamed non-anonymous struct or union?");
>>> + GV = CollectAnonRecordDecls(RD, Unit, LineNo, LinkageName, Var, DContext);
>>> + } else {
>>> + GV = DBuilder.createStaticVariable(
>>> + DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
>>> + Var->hasInternalLinkage(), Var,
>>> + getOrCreateStaticDataMemberDeclarationOrNull(D));
>>> + }
>>> DeclCache.insert(std::make_pair(D->getCanonicalDecl(), llvm::WeakVH(GV)));
>>> }
>>>
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=205952&r1=205951&r2=205952&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Apr 10 00:20:00 2014
>>> @@ -358,6 +358,13 @@ private:
>>> llvm::DIDerivedType
>>> getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl *D);
>>>
>>> + /// Return a global variable that represents one of the collection of
>>> + /// global variables created for an anonmyous union.
>>> + llvm::DIGlobalVariable
>>> + CollectAnonRecordDecls(const RecordDecl *RD, llvm::DIFile Unit, unsigned LineNo,
>>> + StringRef LinkageName, llvm::GlobalVariable *Var,
>>> + llvm::DIDescriptor DContext);
>>> +
>>> /// getFunctionName - Get function name for the given FunctionDecl. If the
>>> /// name is constructed on demand (e.g. C++ destructor) then the name
>>> /// is stored on the side.
>>>
>>> Added: cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp?rev=205952&view=auto
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp (added)
>>> +++ cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp Thu Apr 10 00:20:00 2014
>>> @@ -0,0 +1,28 @@
>>> +// RUN: %clang_cc1 -emit-llvm -gdwarf-4 -triple x86_64-linux-gnu %s -o - | FileCheck %s
>>> +
>>> +// Make sure that we emit a global variable for each of the members of the
>>> +// anonymous union.
>>> +
>>> +static union {
>>> + int c;
>>> + int d;
>>> + union {
>>> + int a;
>>> + };
>>> + struct {
>>> + int b;
>>> + };
>>> +};
>>> +
>>> +int test_it() {
>>> + c = 1;
>>> + d = 2;
>>> + a = 4;
>>> + return (c == 1);
>>> +}
>>> +
>>> +// CHECK: [[FILE:.*]] = {{.*}}[ DW_TAG_file_type ] [{{.*}}debug-info-anon-union-vars.cpp]
>>> +// CHECK: [[FILE]]{{.*}}[ DW_TAG_variable ] [c] [line 6] [local] [def]
>>> +// CHECK: [[FILE]]{{.*}}[ DW_TAG_variable ] [d] [line 6] [local] [def]
>>> +// CHECK: [[FILE]]{{.*}}[ DW_TAG_variable ] [a] [line 6] [local] [def]
>>> +// CHECK: [[FILE]]{{.*}}[ DW_TAG_variable ] [b] [line 6] [local] [def]
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list