r205952 - Add global static variables for anonymous union fields. This makes
David Blaikie
dblaikie at gmail.com
Thu Apr 10 09:52:42 PDT 2014
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).
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