r228855 - Fix PR19351. While building up a composite type it is important to use
David Blaikie
dblaikie at gmail.com
Wed Feb 11 11:35:15 PST 2015
On Wed, Feb 11, 2015 at 11:31 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
> 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> wrote:
>
>>
>> On Feb 11, 2015, at 11:06 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Wed, Feb 11, 2015 at 10:28 AM, Adrian Prantl <aprantl at apple.com>
>> wrote:
>>
>>>
>>> On Feb 11, 2015, at 10:21 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>> On Wed, Feb 11, 2015 at 9:45 AM, Adrian Prantl <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
>>>> 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,
>
Hmm, maybe... dunno.
> 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
>>>>
>>>> ==============================================================================
>>>> --- 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
>>>>
>>>> ==============================================================================
>>>> --- 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
>>>> 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/6a8dbf7e/attachment.html>
More information about the cfe-commits
mailing list