r228855 - Fix PR19351. While building up a composite type it is important to use

Adrian Prantl aprantl at apple.com
Wed Feb 11 11:31:38 PST 2015


> On Feb 11, 2015, at 11:27 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Feb 11, 2015 at 11:14 AM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> 
>> On Feb 11, 2015, at 11:06 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Wed, Feb 11, 2015 at 10:28 AM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
>> 
>>> On Feb 11, 2015, at 10:21 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>> 
>>> 
>>> 
>>> On Wed, Feb 11, 2015 at 9:45 AM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
>>> Author: adrian
>>> Date: Wed Feb 11 11:45:15 2015
>>> New Revision: 228855
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=228855&view=rev <http://llvm.org/viewvc/llvm-project?rev=228855&view=rev>
>>> Log:
>>> Fix PR19351. While building up a composite type it is important to use
>>> a non-uniqueable temporary node that is only turned into a permanent
>>> unique or distinct node after it is finished.
>>> Otherwise an intermediate node may get accidentally uniqued with another
>>> node as illustrated by the testcase.
>>> 
>>> Awesome - thanks!
>>>  
>> 
>> We should probably rename getOrCreateLimitedType() now, as it has nothing to do with limited debug info any more. Any suggestions?
> 
>> 
>> I didn't catch the impact your change had on that function. What does it have to do with now?
> 
> “Now” was not actually meant as “after this commit” but rather as “today’s CFE”. This is just something that I happened noticed while working on this, sorry for being misleading there :-)
> 
> 'sok. I hadn't noticed/thought about it - what's it do now compared to what it did?

I think this used to be the entry point for creating types for limited debug info, but it is really only used to create the temporary shells of what may become recursive composite types.

-- adrian
>  
> 
> -- adrian
>>  
>> 
>> -- adrian
>> 
>>> 
>>> Paired commit with LLVM.
>>> 
>>> Added:
>>>     cfe/trunk/test/CodeGen/debug-info-same-line.c
>>> Modified:
>>>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> 
>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=228855&r1=228854&r2=228855&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=228855&r1=228854&r2=228855&view=diff>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Feb 11 11:45:15 2015
>>> @@ -621,6 +621,21 @@ static SmallString<256> getUniqueTagType
>>>    return FullName;
>>>  }
>>> 
>>> +static llvm::dwarf::Tag getTagForRecord(const RecordDecl *RD) {
>>> +   llvm::dwarf::Tag Tag;
>>> +  if (RD->isStruct() || RD->isInterface())
>>> +    Tag = llvm::dwarf::DW_TAG_structure_type;
>>> +  else if (RD->isUnion())
>>> +    Tag = llvm::dwarf::DW_TAG_union_type;
>>> +  else {
>>> +    // FIXME: This could be a struct type giving a default visibility different
>>> +    // than C++ class type, but needs llvm metadata changes first.
>>> +    assert(RD->isClass());
>>> +    Tag = llvm::dwarf::DW_TAG_class_type;
>>> +  }
>>> +  return Tag;
>>> +}
>>> +
>>>  // Creates a forward declaration for a RecordDecl in the given context.
>>>  llvm::DICompositeType
>>>  CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType *Ty,
>>> @@ -632,20 +647,12 @@ CGDebugInfo::getOrCreateRecordFwdDecl(co
>>>    unsigned Line = getLineNumber(RD->getLocation());
>>>    StringRef RDName = getClassName(RD);
>>> 
>>> -  llvm::dwarf::Tag Tag;
>>> -  if (RD->isStruct() || RD->isInterface())
>>> -    Tag = llvm::dwarf::DW_TAG_structure_type;
>>> -  else if (RD->isUnion())
>>> -    Tag = llvm::dwarf::DW_TAG_union_type;
>>> -  else {
>>> -    assert(RD->isClass());
>>> -    Tag = llvm::dwarf::DW_TAG_class_type;
>>> -  }
>>> 
>>>    // Create the type.
>>>    SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);
>>> -  llvm::DICompositeType RetTy = DBuilder.createReplaceableForwardDecl(
>>> -      Tag, RDName, Ctx, DefUnit, Line, 0, 0, 0, FullName);
>>> +  llvm::DICompositeType RetTy = DBuilder.createReplaceableCompositeType(
>>> +      getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, 0, 0,
>>> +      llvm::DIDescriptor::FlagFwdDecl, FullName);
>>>    ReplaceMap.emplace_back(
>>>        std::piecewise_construct, std::make_tuple(Ty),
>>>        std::make_tuple(static_cast<llvm::Metadata *>(RetTy)));
>>> @@ -1567,7 +1574,8 @@ llvm::DIType CGDebugInfo::CreateTypeDefi
>>>    assert(FwdDecl.isCompositeType() &&
>>>           "The debug type of a RecordType should be a llvm::DICompositeType");
>>> 
>>> -  if (FwdDecl.isForwardDecl())
>>> +  const RecordDecl *D = RD->getDefinition();
>>> +  if (!D || !D->isCompleteDefinition())
>>>      return FwdDecl;
>>> 
>>>    if (const CXXRecordDecl *CXXDecl = dyn_cast<CXXRecordDecl>(RD))
>>> @@ -1602,6 +1610,10 @@ llvm::DIType CGDebugInfo::CreateTypeDefi
>>>    llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);
>>>    DBuilder.replaceArrays(FwdDecl, Elements);
>>> 
>>> +  if (FwdDecl->isTemporary())
>>> +    FwdDecl = llvm::DICompositeType(llvm::MDNode::replaceWithPermanent(
>>> +      llvm::TempMDNode(FwdDecl.get())));
>>> +
>>>    RegionMap[Ty->getDecl()].reset(FwdDecl);
>>>    return FwdDecl;
>>>  }
>>> @@ -1653,7 +1665,7 @@ llvm::DIType CGDebugInfo::CreateType(con
>>>    // debug type since we won't be able to lay out the entire type.
>>>    ObjCInterfaceDecl *Def = ID->getDefinition();
>>>    if (!Def || !Def->getImplementation()) {
>>> -    llvm::DIType FwdDecl = DBuilder.createReplaceableForwardDecl(
>>> +    llvm::DIType FwdDecl = DBuilder.createReplaceableCompositeType(
>>>          llvm::dwarf::DW_TAG_structure_type, ID->getName(), TheCU, DefUnit, Line,
>>>          RuntimeLang);
>>>      ObjCInterfaceCache.push_back(ObjCInterfaceCacheEntry(Ty, FwdDecl, Unit));
>>> @@ -1933,9 +1945,9 @@ llvm::DIType CGDebugInfo::CreateEnumType
>>>      llvm::DIFile DefUnit = getOrCreateFile(ED->getLocation());
>>>      unsigned Line = getLineNumber(ED->getLocation());
>>>      StringRef EDName = ED->getName();
>>> -    llvm::DIType RetTy = DBuilder.createReplaceableForwardDecl(
>>> +    llvm::DIType RetTy = DBuilder.createReplaceableCompositeType(
>>>          llvm::dwarf::DW_TAG_enumeration_type, EDName, EDContext, DefUnit, Line,
>>> -        0, Size, Align, FullName);
>>> +        0, Size, Align, llvm::DIDescriptor::FlagFwdDecl, FullName);
>>>      ReplaceMap.emplace_back(
>>>          std::piecewise_construct, std::make_tuple(Ty),
>>>          std::make_tuple(static_cast<llvm::Metadata *>(RetTy)));
>>> @@ -2249,19 +2261,8 @@ llvm::DICompositeType CGDebugInfo::Creat
>>> 
>>>    SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);
>>> 
>>> -  if (RD->isUnion())
>>> -    RealDecl = DBuilder.createUnionType(RDContext, RDName, DefUnit, Line, Size,
>>> -                                        Align, 0, llvm::DIArray(), 0, FullName);
>>> -  else if (RD->isClass()) {
>>> -    // FIXME: This could be a struct type giving a default visibility different
>>> -    // than C++ class type, but needs llvm metadata changes first.
>>> -    RealDecl = DBuilder.createClassType(
>>> -        RDContext, RDName, DefUnit, Line, Size, Align, 0, 0, llvm::DIType(),
>>> -        llvm::DIArray(), llvm::DIType(), llvm::DIArray(), FullName);
>>> -  } else
>>> -    RealDecl = DBuilder.createStructType(
>>> -        RDContext, RDName, DefUnit, Line, Size, Align, 0, llvm::DIType(),
>>> -        llvm::DIArray(), 0, llvm::DIType(), FullName);
>>> +  RealDecl = DBuilder.createReplaceableCompositeType(getTagForRecord(RD),
>>> +      RDName, RDContext, DefUnit, Line, 0, Size, Align, 0, FullName);
>>> 
>>>    RegionMap[Ty->getDecl()].reset(RealDecl);
>>>    TypeCache[QualType(Ty, 0).getAsOpaquePtr()].reset(RealDecl);
>>> 
>>> Added: cfe/trunk/test/CodeGen/debug-info-same-line.c
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-same-line.c?rev=228855&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-same-line.c?rev=228855&view=auto>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/debug-info-same-line.c (added)
>>> +++ cfe/trunk/test/CodeGen/debug-info-same-line.c Wed Feb 11 11:45:15 2015
>>> @@ -0,0 +1,7 @@
>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -g -o - | FileCheck %s
>>> +// Here two temporary nodes are identical (but should not get uniqued) while
>>> +// building the full debug type.
>>> +typedef struct { long x; } foo; typedef struct {  foo *x; } bar;
>>> +// CHECK: [ DW_TAG_structure_type ] [line 4, size 64,
>>> +// CHECK: [ DW_TAG_structure_type ] [line 4, size 64,
>>> +bar b;
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>>> 
>> 
>> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150211/2f6e7c13/attachment.html>


More information about the cfe-commits mailing list