r208015 - Build debug info for ObjC interface types at the end of the translation unit to ensure all ivars are included.
Jordan Rose
jordan_rose at apple.com
Mon May 5 16:39:31 PDT 2014
This comment for ObjCImplementationDecl is totally bogus now, BTW.
/// Typically, instance variables are specified in the class interface,
/// *not* in the implementation. Nevertheless (for legacy reasons), we
/// allow instance variables to be specified in the implementation. When
/// specified, they need to be *identical* to the interface.
///
On May 5, 2014, at 16:38, Jordan Rose <jordan_rose at apple.com> wrote:
> @interface is a definition of an ObjC class, but it doesn't contain all the ivars. @implementation can also contain ivars, but it's not in the redeclaration chain for the @interface/@class. Is this going to mess things up at all?
>
> Jordan
>
>
> On May 5, 2014, at 16:23, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Author: dblaikie
>> Date: Mon May 5 18:23:53 2014
>> New Revision: 208015
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=208015&view=rev
>> Log:
>> Build debug info for ObjC interface types at the end of the translation unit to ensure all ivars are included.
>>
>> This takes a different approach than the
>> completedType/requiresCompleteType work which relies on AST callbacks to
>> upgrade the type declaration to a definition. Instead, just defer
>> constructing the definition to the end of the translation unit.
>>
>> This works because the definition is never needed by other debug info
>> (so far as I know), whereas the definition of a struct may be needed by
>> other debug info before the end of the translation unit (such as
>> emitting the definition of a member function which must refer to that
>> member function's declaration).
>>
>> If we had a callback for whenever an IVar was added to an ObjC interface
>> we could use that, and remove the need for the ObjCInterfaceCache, which
>> might be nice. (also would need a callback for when it was more than
>> just a declaration so we could get properties, etc).
>>
>> A side benefit is that we also don't need the CompletedTypeCache
>> anymore. Just rely on the declaration-ness of a type to decide whether
>> its definition is yet to be emitted.
>>
>> There's still the PR19562 memory leak, but this should hopefully make
>> that a bit easier to approach.
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> cfe/trunk/lib/CodeGen/CGDebugInfo.h
>> cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=208015&r1=208014&r2=208015&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon May 5 18:23:53 2014
>> @@ -1453,11 +1453,13 @@ void CGDebugInfo::completeClassData(cons
>> return;
>> QualType Ty = CGM.getContext().getRecordType(RD);
>> void* TyPtr = Ty.getAsOpaquePtr();
>> - if (CompletedTypeCache.count(TyPtr))
>> + auto I = TypeCache.find(TyPtr);
>> + if (I != TypeCache.end() &&
>> + !llvm::DIType(cast<llvm::MDNode>(static_cast<llvm::Value *>(I->second)))
>> + .isForwardDecl())
>> return;
>> llvm::DIType Res = CreateTypeDefinition(Ty->castAs<RecordType>());
>> assert(!Res.isForwardDecl());
>> - CompletedTypeCache[TyPtr] = Res;
>> TypeCache[TyPtr] = Res;
>> }
>>
>> @@ -1545,9 +1547,6 @@ llvm::DIType CGDebugInfo::CreateTypeDefi
>> LexicalBlockStack.push_back(&*FwdDecl);
>> RegionMap[Ty->getDecl()] = llvm::WeakVH(FwdDecl);
>>
>> - // Add this to the completed-type cache while we're completing it recursively.
>> - CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = FwdDecl;
>> -
>> // Convert all the elements.
>> SmallVector<llvm::Value *, 16> EltTys;
>> // what about nested types?
>> @@ -1624,15 +1623,24 @@ llvm::DIType CGDebugInfo::CreateType(con
>> // If this is just a forward declaration return a special forward-declaration
>> // debug type since we won't be able to lay out the entire type.
>> ObjCInterfaceDecl *Def = ID->getDefinition();
>> - if (!Def) {
>> + if (!Def || !Def->getImplementation()) {
>> llvm::DIType FwdDecl =
>> DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
>> ID->getName(), TheCU, DefUnit, Line,
>> RuntimeLang);
>> + ObjCInterfaceCache.push_back(ObjCInterfaceCacheEntry(Ty, FwdDecl, Unit));
>> return FwdDecl;
>> }
>>
>> - ID = Def;
>> +
>> + return CreateTypeDefinition(Ty, Unit);
>> +}
>> +
>> +llvm::DIType CGDebugInfo::CreateTypeDefinition(const ObjCInterfaceType *Ty, llvm::DIFile Unit) {
>> + ObjCInterfaceDecl *ID = Ty->getDecl();
>> + llvm::DIFile DefUnit = getOrCreateFile(ID->getLocation());
>> + unsigned Line = getLineNumber(ID->getLocation());
>> + unsigned RuntimeLang = TheCU.getLanguage();
>>
>> // Bit size, align and offset of the type.
>> uint64_t Size = CGM.getContext().getTypeSize(Ty);
>> @@ -1647,10 +1655,8 @@ llvm::DIType CGDebugInfo::CreateType(con
>> Line, Size, Align, Flags,
>> llvm::DIType(), llvm::DIArray(), RuntimeLang);
>>
>> - // Otherwise, insert it into the CompletedTypeCache so that recursive uses
>> - // will find it and we're emitting the complete type.
>> - QualType QualTy = QualType(Ty, 0);
>> - CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl;
>> + QualType QTy(Ty, 0);
>> + TypeCache[QTy.getAsOpaquePtr()] = RealDecl;
>>
>> // Push the struct on region stack.
>> LexicalBlockStack.push_back(static_cast<llvm::MDNode*>(RealDecl));
>> @@ -1774,12 +1780,6 @@ llvm::DIType CGDebugInfo::CreateType(con
>> llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);
>> RealDecl.setTypeArray(Elements);
>>
>> - // If the implementation is not yet set, we do not want to mark it
>> - // as complete. An implementation may declare additional
>> - // private ivars that we would miss otherwise.
>> - if (ID->getImplementation() == 0)
>> - CompletedTypeCache.erase(QualTy.getAsOpaquePtr());
>> -
>> LexicalBlockStack.pop_back();
>> return RealDecl;
>> }
>> @@ -2002,14 +2002,6 @@ llvm::DIType CGDebugInfo::getTypeOrNull(
>> // Unwrap the type as needed for debug information.
>> Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
>>
>> - // Check for existing entry.
>> - if (Ty->getTypeClass() == Type::ObjCInterface) {
>> - llvm::Value *V = getCachedInterfaceTypeOrNull(Ty);
>> - if (V)
>> - return llvm::DIType(cast<llvm::MDNode>(V));
>> - else return llvm::DIType();
>> - }
>> -
>> llvm::DenseMap<void *, llvm::WeakVH>::iterator it =
>> TypeCache.find(Ty.getAsOpaquePtr());
>> if (it != TypeCache.end()) {
>> @@ -2021,27 +2013,6 @@ llvm::DIType CGDebugInfo::getTypeOrNull(
>> return llvm::DIType();
>> }
>>
>> -/// getCompletedTypeOrNull - Get the type from the cache or return null if it
>> -/// doesn't exist.
>> -llvm::DIType CGDebugInfo::getCompletedTypeOrNull(QualType Ty) {
>> -
>> - // Unwrap the type as needed for debug information.
>> - Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
>> -
>> - // Check for existing entry.
>> - llvm::Value *V = 0;
>> - llvm::DenseMap<void *, llvm::WeakVH>::iterator it =
>> - CompletedTypeCache.find(Ty.getAsOpaquePtr());
>> - if (it != CompletedTypeCache.end())
>> - V = it->second;
>> - else {
>> - V = getCachedInterfaceTypeOrNull(Ty);
>> - }
>> -
>> - // Verify that any cached debug info still exists.
>> - return llvm::DIType(cast_or_null<llvm::MDNode>(V));
>> -}
>> -
>> void CGDebugInfo::completeTemplateDefinition(
>> const ClassTemplateSpecializationDecl &SD) {
>> if (DebugKind <= CodeGenOptions::DebugLineTablesOnly)
>> @@ -2053,22 +2024,6 @@ void CGDebugInfo::completeTemplateDefini
>> RetainedTypes.push_back(CGM.getContext().getRecordType(&SD).getAsOpaquePtr());
>> }
>>
>> -/// getCachedInterfaceTypeOrNull - Get the type from the interface
>> -/// cache, unless it needs to regenerated. Otherwise return null.
>> -llvm::Value *CGDebugInfo::getCachedInterfaceTypeOrNull(QualType Ty) {
>> - // Is there a cached interface that hasn't changed?
>> - llvm::DenseMap<void *, std::pair<llvm::WeakVH, unsigned > >
>> - ::iterator it1 = ObjCInterfaceCache.find(Ty.getAsOpaquePtr());
>> -
>> - if (it1 != ObjCInterfaceCache.end())
>> - if (ObjCInterfaceDecl* Decl = getObjCInterfaceDecl(Ty))
>> - if (Checksum(Decl) == it1->second.second)
>> - // Return cached forward declaration.
>> - return it1->second.first;
>> -
>> - return 0;
>> -}
>> -
>> /// getOrCreateType - Get the type from the cache or create a new
>> /// one if necessary.
>> llvm::DIType CGDebugInfo::getOrCreateType(QualType Ty, llvm::DIFile Unit) {
>> @@ -2078,7 +2033,7 @@ llvm::DIType CGDebugInfo::getOrCreateTyp
>> // Unwrap the type as needed for debug information.
>> Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
>>
>> - if (llvm::DIType T = getCompletedTypeOrNull(Ty))
>> + if (llvm::DIType T = getTypeOrNull(Ty))
>> return T;
>>
>> // Otherwise create the type.
>> @@ -2088,37 +2043,6 @@ llvm::DIType CGDebugInfo::getOrCreateTyp
>> // And update the type cache.
>> TypeCache[TyPtr] = Res;
>>
>> - // FIXME: this getTypeOrNull call seems silly when we just inserted the type
>> - // into the cache - but getTypeOrNull has a special case for cached interface
>> - // types. We should probably just pull that out as a special case for the
>> - // "else" block below & skip the otherwise needless lookup.
>> - llvm::DIType TC = getTypeOrNull(Ty);
>> - if (ObjCInterfaceDecl *Decl = getObjCInterfaceDecl(Ty)) {
>> - // Interface types may have elements added to them by a
>> - // subsequent implementation or extension, so we keep them in
>> - // the ObjCInterfaceCache together with a checksum. Instead of
>> - // the (possibly) incomplete interface type, we return a forward
>> - // declaration that gets RAUW'd in CGDebugInfo::finalize().
>> - std::pair<llvm::WeakVH, unsigned> &V = ObjCInterfaceCache[TyPtr];
>> - if (V.first)
>> - return llvm::DIType(cast<llvm::MDNode>(V.first));
>> - TC = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
>> - Decl->getName(), TheCU, Unit,
>> - getLineNumber(Decl->getLocation()),
>> - TheCU.getLanguage());
>> - // Store the forward declaration in the cache.
>> - V.first = TC;
>> - V.second = Checksum(Decl);
>> -
>> - // Register the type for replacement in finalize().
>> - ReplaceMap.push_back(std::make_pair(TyPtr, static_cast<llvm::Value*>(TC)));
>> -
>> - return TC;
>> - }
>> -
>> - if (!Res.isForwardDecl())
>> - CompletedTypeCache[TyPtr] = Res;
>> -
>> return Res;
>> }
>>
>> @@ -2282,13 +2206,6 @@ llvm::DICompositeType CGDebugInfo::Creat
>>
>> // If we ended up creating the type during the context chain construction,
>> // just return that.
>> - // FIXME: this could be dealt with better if the type was recorded as
>> - // completed before we started this (see the CompletedTypeCache usage in
>> - // CGDebugInfo::CreateTypeDefinition(const RecordType*) - that would need to
>> - // be pushed to before context creation, but after it was known to be
>> - // destined for completion (might still have an issue if this caller only
>> - // required a declaration but the context construction ended up creating a
>> - // definition)
>> llvm::DICompositeType T(getTypeOrNull(CGM.getContext().getRecordType(RD)));
>> if (T && (!T.isForwardDecl() || !RD->getDefinition()))
>> return T;
>> @@ -3393,6 +3310,11 @@ void CGDebugInfo::finalize() {
>> Ty.replaceAllUsesWith(RepTy);
>> }
>>
>> + for (auto E : ObjCInterfaceCache)
>> + E.Decl.replaceAllUsesWith(E.Type->getDecl()->getDefinition()
>> + ? CreateTypeDefinition(E.Type, E.Unit)
>> + : E.Decl);
>> +
>> // We keep our own list of retained types, because we need to look
>> // up the final type in the type cache.
>> for (std::vector<void *>::const_iterator RI = RetainedTypes.begin(),
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=208015&r1=208014&r2=208015&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon May 5 18:23:53 2014
>> @@ -67,16 +67,22 @@ class CGDebugInfo {
>> /// TypeCache - Cache of previously constructed Types.
>> llvm::DenseMap<void *, llvm::WeakVH> TypeCache;
>>
>> + struct ObjCInterfaceCacheEntry {
>> + const ObjCInterfaceType *Type;
>> + llvm::DIType Decl;
>> + llvm::DIFile Unit;
>> + ObjCInterfaceCacheEntry(const ObjCInterfaceType *Type, llvm::DIType Decl,
>> + llvm::DIFile Unit)
>> + : Type(Type), Decl(Decl), Unit(Unit) {}
>> + };
>> +
>> /// ObjCInterfaceCache - Cache of previously constructed interfaces
>> - /// which may change. Storing a pair of DIType and checksum.
>> - llvm::DenseMap<void *, std::pair<llvm::WeakVH, unsigned> > ObjCInterfaceCache;
>> + /// which may change.
>> + llvm::SmallVector<ObjCInterfaceCacheEntry, 32> ObjCInterfaceCache;
>>
>> /// RetainedTypes - list of interfaces we want to keep even if orphaned.
>> std::vector<void *> RetainedTypes;
>>
>> - /// CompleteTypeCache - Cache of previously constructed complete RecordTypes.
>> - llvm::DenseMap<void *, llvm::WeakVH> CompletedTypeCache;
>> -
>> /// ReplaceMap - Cache of forward declared types to RAUW at the end of
>> /// compilation.
>> std::vector<std::pair<void *, llvm::WeakVH> >ReplaceMap;
>> @@ -120,6 +126,7 @@ class CGDebugInfo {
>> llvm::DICompositeType CreateLimitedType(const RecordType *Ty);
>> void CollectContainingType(const CXXRecordDecl *RD, llvm::DICompositeType CT);
>> llvm::DIType CreateType(const ObjCInterfaceType *Ty, llvm::DIFile F);
>> + llvm::DIType CreateTypeDefinition(const ObjCInterfaceType *Ty, llvm::DIFile F);
>> llvm::DIType CreateType(const ObjCObjectType *Ty, llvm::DIFile F);
>> llvm::DIType CreateType(const VectorType *Ty, llvm::DIFile F);
>> llvm::DIType CreateType(const ArrayType *Ty, llvm::DIFile F);
>> @@ -130,7 +137,6 @@ class CGDebugInfo {
>> llvm::DIType CreateEnumType(const EnumType *Ty);
>> llvm::DIType CreateSelfType(const QualType &QualTy, llvm::DIType Ty);
>> llvm::DIType getTypeOrNull(const QualType);
>> - llvm::DIType getCompletedTypeOrNull(const QualType);
>> llvm::DICompositeType getOrCreateMethodType(const CXXMethodDecl *Method,
>> llvm::DIFile F);
>> llvm::DICompositeType getOrCreateInstanceMethodType(
>>
>> Modified: cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m?rev=208015&r1=208014&r2=208015&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m (original)
>> +++ cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m Mon May 5 18:23:53 2014
>> @@ -3,6 +3,14 @@
>> // Make sure we generate debug symbols for an indirectly referenced
>> // extension to an interface.
>>
>> +// This happens to be the order the members are emitted in... I'm assuming it's
>> +// not meaningful/important, so if something causes the order to change, feel
>> +// free to update the test to reflect the new order.
>> +// CHECK: ; [ DW_TAG_member ] [a]
>> +// CHECK: ; [ DW_TAG_member ] [d]
>> +// CHECK: ; [ DW_TAG_member ] [c]
>> +// CHECK: ; [ DW_TAG_member ] [b]
>> +
>> @interface I
>> {
>> @public int a;
>> @@ -29,7 +37,6 @@ void gorf (struct S* s) {
>> int _b = s->i->b;
>> }
>>
>> -// CHECK: ; [ DW_TAG_member ] [b]
>>
>> I *source();
>>
>> @@ -39,8 +46,14 @@ I *source();
>> }
>> @end
>>
>> -// CHECK: ; [ DW_TAG_member ] [c]
>> -
>> void use() {
>> int _c = source()->c;
>> }
>> +
>> + at interface I()
>> +{
>> + @public int d;
>> +}
>> + at end
>> +
>> +I *x();
>>
>>
>> _______________________________________________
>> 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/20140505/bd8d7292/attachment.html>
More information about the cfe-commits
mailing list