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