From ef8a9518826b5b6b726eec66fd2fce6a3532ac66 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 5 May 2014 23:23:53 +0000 Subject: [PATCH] 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. llvm-svn: 208015 --- clang/lib/CodeGen/CGDebugInfo.cpp | 124 ++++-------------- clang/lib/CodeGen/CGDebugInfo.h | 18 ++- .../CodeGenObjC/debug-info-ivars-indirect.m | 19 ++- 3 files changed, 51 insertions(+), 110 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 4d763508addd..524ab589248b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1453,11 +1453,13 @@ void CGDebugInfo::completeClassData(const RecordDecl *RD) { 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(static_cast(I->second))) + .isForwardDecl()) return; llvm::DIType Res = CreateTypeDefinition(Ty->castAs()); assert(!Res.isForwardDecl()); - CompletedTypeCache[TyPtr] = Res; TypeCache[TyPtr] = Res; } @@ -1545,9 +1547,6 @@ llvm::DIType CGDebugInfo::CreateTypeDefinition(const RecordType *Ty) { 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 EltTys; // what about nested types? @@ -1624,15 +1623,24 @@ llvm::DIType CGDebugInfo::CreateType(const ObjCInterfaceType *Ty, // 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(const ObjCInterfaceType *Ty, 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(RealDecl)); @@ -1774,12 +1780,6 @@ llvm::DIType CGDebugInfo::CreateType(const ObjCInterfaceType *Ty, 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(QualType Ty) { // 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(V)); - else return llvm::DIType(); - } - llvm::DenseMap::iterator it = TypeCache.find(Ty.getAsOpaquePtr()); if (it != TypeCache.end()) { @@ -2021,27 +2013,6 @@ llvm::DIType CGDebugInfo::getTypeOrNull(QualType Ty) { 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::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(V)); -} - void CGDebugInfo::completeTemplateDefinition( const ClassTemplateSpecializationDecl &SD) { if (DebugKind <= CodeGenOptions::DebugLineTablesOnly) @@ -2053,22 +2024,6 @@ void CGDebugInfo::completeTemplateDefinition( 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 > - ::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::getOrCreateType(QualType Ty, llvm::DIFile Unit) { // 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::getOrCreateType(QualType Ty, llvm::DIFile Unit) { // 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 &V = ObjCInterfaceCache[TyPtr]; - if (V.first) - return llvm::DIType(cast(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(TC))); - - return TC; - } - - if (!Res.isForwardDecl()) - CompletedTypeCache[TyPtr] = Res; - return Res; } @@ -2282,13 +2206,6 @@ llvm::DICompositeType CGDebugInfo::CreateLimitedType(const RecordType *Ty) { // 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::const_iterator RI = RetainedTypes.begin(), diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index 2fc35d5b87c6..d69fabf292d9 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -67,16 +67,22 @@ class CGDebugInfo { /// TypeCache - Cache of previously constructed Types. llvm::DenseMap 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 > ObjCInterfaceCache; + /// which may change. + llvm::SmallVector ObjCInterfaceCache; /// RetainedTypes - list of interfaces we want to keep even if orphaned. std::vector RetainedTypes; - /// CompleteTypeCache - Cache of previously constructed complete RecordTypes. - llvm::DenseMap CompletedTypeCache; - /// ReplaceMap - Cache of forward declared types to RAUW at the end of /// compilation. std::vector >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( diff --git a/clang/test/CodeGenObjC/debug-info-ivars-indirect.m b/clang/test/CodeGenObjC/debug-info-ivars-indirect.m index 8d1ab92d7665..f9593d24dea0 100644 --- a/clang/test/CodeGenObjC/debug-info-ivars-indirect.m +++ b/clang/test/CodeGenObjC/debug-info-ivars-indirect.m @@ -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; } + +@interface I() +{ + @public int d; +} +@end + +I *x();