Oh, and just for the fun of it, this is a yak I discovered (& is somewhat related to) while trying to fix PR19598, a memory leak in Clang.
On Sun, May 4, 2014 at 5:09 PM, David Blaikie <[email protected]> wrote: > On Wed, Mar 6, 2013 at 2:03 PM, Adrian Prantl <[email protected]> wrote: >> Author: adrian >> Date: Wed Mar 6 16:03:30 2013 >> New Revision: 176584 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=176584&view=rev >> Log: >> Ensure that DIType is regenerated after we visit an implementation >> that adds ivars to an interface. >> >> Fixes rdar://13175234 >> >> This is an update to r176116 that performs a smart caching of interfaces. > > *performs necromancy* > > So, I was looking at this code and how we've refactored type > completion code in the last year or two (using complete type and > requires complete type callbacks, etc) and I was going to change > getOrCreateType to simply bail if getTypeOrNull returned non-null, > rather than using getCompleteTypeOrNull. > > But that causes these 3 tests to fail, because these tests expect to > attempt to rebuild the type every time it's referenced if it isn't > complete. We don't need to do this for C++ types (rather than waiting > for a use after the type becomes complete to create the complete debug > info, we use the callback to inform us that the type is complete). > > I think it will simplify (marginally speed up, but I doubt that's > relevant) the implementation of getTypeOrNull if we could do something > similar for IVars - indeed, it would also be more correct. Try adding > this to the end of debug-info-ivars-indirect.m: > > > I *source(); > > @interface I() > { > @public int c; > } > @end > > // CHECK: ; [ DW_TAG_member ] [c] > > void use() { > int _c = source()->c; > } > > Since we never generate debug info for the 'I' type after it is > extended to have the 'c' member, we never generate debug info for that > member. > > Is there a way we could do this in a callback fashion - thus being > more correct and efficient? > > Alternatively: we could kill the callback system, make a list of types > we've emitted declarations for, and walk that list at the end of the > translation unit, building whichever types we need (that way we'd be > sure to find all the ivars, for example). That depends whether it's > worth visiting all the types to check if they need to be upgraded to > definitions at the end. > > >> >> Added: >> cfe/trunk/test/CodeGenObjC/debug-info-ivars-extension.m >> cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m >> cfe/trunk/test/CodeGenObjC/debug-info-ivars-private.m >> Modified: >> cfe/trunk/include/clang/AST/DeclObjC.h >> cfe/trunk/lib/AST/DeclObjC.cpp >> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp >> cfe/trunk/lib/CodeGen/CGDebugInfo.h >> >> Modified: cfe/trunk/include/clang/AST/DeclObjC.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=176584&r1=176583&r2=176584&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/DeclObjC.h (original) >> +++ cfe/trunk/include/clang/AST/DeclObjC.h Wed Mar 6 16:03:30 2013 >> @@ -651,6 +651,10 @@ class ObjCInterfaceDecl : public ObjCCon >> /// completed by the external AST source when required. >> mutable bool ExternallyCompleted : 1; >> >> + /// \brief Indicates that the ivar cache does not yet include ivars >> + /// declared in the implementation. >> + mutable bool IvarListMissingImplementation : 1; >> + >> /// \brief The location of the superclass, if any. >> SourceLocation SuperClassLoc; >> >> @@ -660,7 +664,8 @@ class ObjCInterfaceDecl : public ObjCCon >> SourceLocation EndLoc; >> >> DefinitionData() : Definition(), SuperClass(), CategoryList(), >> IvarList(), >> - ExternallyCompleted() { } >> + ExternallyCompleted(), >> + IvarListMissingImplementation(true) { } >> }; >> >> ObjCInterfaceDecl(DeclContext *DC, SourceLocation atLoc, IdentifierInfo >> *Id, >> >> Modified: cfe/trunk/lib/AST/DeclObjC.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=176584&r1=176583&r2=176584&view=diff >> ============================================================================== >> --- cfe/trunk/lib/AST/DeclObjC.cpp (original) >> +++ cfe/trunk/lib/AST/DeclObjC.cpp Wed Mar 6 16:03:30 2013 >> @@ -1093,38 +1093,51 @@ namespace { >> /// all_declared_ivar_begin - return first ivar declared in this class, >> /// its extensions and its implementation. Lazily build the list on first >> /// access. >> +/// >> +/// Caveat: The list returned by this method reflects the current >> +/// state of the parser. The cache will be updated for every ivar >> +/// added by an extension or the implementation when they are >> +/// encountered. >> +/// See also ObjCIvarDecl::Create(). >> ObjCIvarDecl *ObjCInterfaceDecl::all_declared_ivar_begin() { >> // FIXME: Should make sure no callers ever do this. >> if (!hasDefinition()) >> return 0; >> >> - if (data().IvarList) >> - return data().IvarList; >> - >> ObjCIvarDecl *curIvar = 0; >> - if (!ivar_empty()) { >> - ObjCInterfaceDecl::ivar_iterator I = ivar_begin(), E = ivar_end(); >> - data().IvarList = *I; ++I; >> - for (curIvar = data().IvarList; I != E; curIvar = *I, ++I) >> - curIvar->setNextIvar(*I); >> - } >> + if (!data().IvarList) { >> + if (!ivar_empty()) { >> + ObjCInterfaceDecl::ivar_iterator I = ivar_begin(), E = ivar_end(); >> + data().IvarList = *I; ++I; >> + for (curIvar = data().IvarList; I != E; curIvar = *I, ++I) >> + curIvar->setNextIvar(*I); >> + } >> >> - for (ObjCInterfaceDecl::known_extensions_iterator >> - Ext = known_extensions_begin(), >> - ExtEnd = known_extensions_end(); >> - Ext != ExtEnd; ++Ext) { >> - if (!Ext->ivar_empty()) { >> - ObjCCategoryDecl::ivar_iterator I = Ext->ivar_begin(),E = >> Ext->ivar_end(); >> - if (!data().IvarList) { >> - data().IvarList = *I; ++I; >> - curIvar = data().IvarList; >> + for (ObjCInterfaceDecl::known_extensions_iterator >> + Ext = known_extensions_begin(), >> + ExtEnd = known_extensions_end(); >> + Ext != ExtEnd; ++Ext) { >> + if (!Ext->ivar_empty()) { >> + ObjCCategoryDecl::ivar_iterator >> + I = Ext->ivar_begin(), >> + E = Ext->ivar_end(); >> + if (!data().IvarList) { >> + data().IvarList = *I; ++I; >> + curIvar = data().IvarList; >> + } >> + for ( ;I != E; curIvar = *I, ++I) >> + curIvar->setNextIvar(*I); >> } >> - for ( ;I != E; curIvar = *I, ++I) >> - curIvar->setNextIvar(*I); >> } >> + data().IvarListMissingImplementation = true; >> } >> + >> + // cached and complete! >> + if (!data().IvarListMissingImplementation) >> + return data().IvarList; >> >> if (ObjCImplementationDecl *ImplDecl = getImplementation()) { >> + data().IvarListMissingImplementation = false; >> if (!ImplDecl->ivar_empty()) { >> SmallVector<SynthesizeIvarChunk, 16> layout; >> for (ObjCImplementationDecl::ivar_iterator I = ImplDecl->ivar_begin(), >> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=176584&r1=176583&r2=176584&view=diff >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Mar 6 16:03:30 2013 >> @@ -1343,7 +1343,7 @@ llvm::DIType CGDebugInfo::CreateType(con >> LexicalBlockStack.push_back(FwdDeclNode); >> RegionMap[Ty->getDecl()] = llvm::WeakVH(FwdDecl); >> >> - // Add this to the completed types cache since we're completing it. >> + // Add this to the completed-type cache while we're completing it >> recursively. >> CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = FwdDecl; >> >> // Convert all the elements. >> @@ -1436,7 +1436,8 @@ llvm::DIType CGDebugInfo::CreateType(con >> >> // Otherwise, insert it into the CompletedTypeCache so that recursive uses >> // will find it and we're emitting the complete type. >> - CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = RealDecl; >> + QualType QualTy = QualType(Ty, 0); >> + CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl; >> // Push the struct on region stack. >> llvm::TrackingVH<llvm::MDNode> FwdDeclNode(RealDecl); >> >> @@ -1561,6 +1562,12 @@ llvm::DIType CGDebugInfo::CreateType(con >> >> llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys); >> FwdDeclNode->replaceOperandWith(10, 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 llvm::DIType(FwdDeclNode); >> @@ -1790,13 +1797,27 @@ llvm::DIType CGDebugInfo::getCompletedTy >> 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()) { >> - // Verify that the debug info still exists. >> - if (llvm::Value *V = it->second) >> - return llvm::DIType(cast<llvm::MDNode>(V)); >> - } >> + if (it != CompletedTypeCache.end()) >> + V = it->second; >> + else { >> + // 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 type. >> + V = it1->second.first; >> + } >> + } >> + >> + // Verify that any cached debug info still exists. >> + if (V != 0) >> + return llvm::DIType(cast<llvm::MDNode>(V)); >> >> return llvm::DIType(); >> } >> @@ -1824,6 +1845,16 @@ llvm::DIType CGDebugInfo::getOrCreateTyp >> ReplaceMap.push_back(std::make_pair(Ty.getAsOpaquePtr(), >> static_cast<llvm::Value*>(TC))); >> >> + // Do not cache the type if it may be incomplete. >> + if (ObjCInterfaceDecl* Decl = getObjCInterfaceDecl(Ty)) { >> + // clang::ParseAST handles each TopLevelDecl immediately after it was >> parsed. >> + // A subsequent implementation may add more ivars to an interface, >> which is >> + // why we cache it together with a checksum to see if it changed. >> + ObjCInterfaceCache[Ty.getAsOpaquePtr()] = >> + std::make_pair(Res, Checksum(Decl)); >> + return Res; >> + } >> + >> // And update the type cache. >> TypeCache[Ty.getAsOpaquePtr()] = Res; >> >> @@ -1833,6 +1864,26 @@ llvm::DIType CGDebugInfo::getOrCreateTyp >> return Res; >> } >> >> +/// Currently the checksum merely consists of the number of ivars. >> +unsigned CGDebugInfo::Checksum(const ObjCInterfaceDecl >> + *InterfaceDecl) { >> + unsigned IvarNo = 0; >> + for (const ObjCIvarDecl *Ivar = InterfaceDecl->all_declared_ivar_begin(); >> + Ivar != 0; Ivar = Ivar->getNextIvar()) ++IvarNo; >> + return IvarNo; >> +} >> + >> +ObjCInterfaceDecl *CGDebugInfo::getObjCInterfaceDecl(QualType Ty) { >> + switch (Ty->getTypeClass()) { >> + case Type::ObjCObjectPointer: >> + return >> getObjCInterfaceDecl(cast<ObjCObjectPointerType>(Ty)->getPointeeType()); >> + case Type::ObjCInterface: >> + return cast<ObjCInterfaceType>(Ty)->getDecl(); >> + default: >> + return 0; >> + } >> +} >> + >> /// CreateTypeNode - Create a new debug type node. >> llvm::DIType CGDebugInfo::CreateTypeNode(QualType Ty, llvm::DIFile Unit) { >> // Handle qualifiers, which recursively handles what they refer to. >> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=176584&r1=176583&r2=176584&view=diff >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original) >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Wed Mar 6 16:03:30 2013 >> @@ -32,6 +32,7 @@ namespace clang { >> class CXXMethodDecl; >> class VarDecl; >> class ObjCInterfaceDecl; >> + class ObjCIvarDecl; >> class ClassTemplateSpecializationDecl; >> class GlobalDecl; >> >> @@ -60,6 +61,11 @@ class CGDebugInfo { >> /// TypeCache - Cache of previously constructed Types. >> llvm::DenseMap<void *, llvm::WeakVH> TypeCache; >> >> + /// 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; >> + >> /// CompleteTypeCache - Cache of previously constructed complete >> RecordTypes. >> llvm::DenseMap<void *, llvm::WeakVH> CompletedTypeCache; >> >> @@ -89,6 +95,7 @@ class CGDebugInfo { >> llvm::DenseMap<const Decl *, llvm::WeakVH> StaticDataMemberCache; >> >> /// Helper functions for getOrCreateType. >> + unsigned Checksum(const ObjCInterfaceDecl *InterfaceDecl); >> llvm::DIType CreateType(const BuiltinType *Ty); >> llvm::DIType CreateType(const ComplexType *Ty); >> llvm::DIType CreateQualifiedType(QualType Ty, llvm::DIFile F); >> @@ -299,6 +306,10 @@ private: >> /// CreateTypeNode - Create type metadata for a source language type. >> llvm::DIType CreateTypeNode(QualType Ty, llvm::DIFile F); >> >> + /// getObjCInterfaceDecl - return the underlying ObjCInterfaceDecl >> + /// if Ty is an ObjCInterface or a pointer to one. >> + ObjCInterfaceDecl* getObjCInterfaceDecl(QualType Ty); >> + >> /// CreateLimitedTypeNode - Create type metadata for a source language >> /// type, but only partial types for records. >> llvm::DIType CreateLimitedTypeNode(QualType Ty, llvm::DIFile F); >> >> Added: cfe/trunk/test/CodeGenObjC/debug-info-ivars-extension.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/debug-info-ivars-extension.m?rev=176584&view=auto >> ============================================================================== >> --- cfe/trunk/test/CodeGenObjC/debug-info-ivars-extension.m (added) >> +++ cfe/trunk/test/CodeGenObjC/debug-info-ivars-extension.m Wed Mar 6 >> 16:03:30 2013 >> @@ -0,0 +1,28 @@ >> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -g %s -o - | >> FileCheck %s >> + >> +// Make sure we generate debug symbols for ivars added by a class extension. >> + >> +@interface I >> +{ >> + @public int a; >> +} >> +@end >> + >> +void foo(I* pi) { >> + // poking into pi for primary class ivars. >> + int _a = pi->a; >> +} >> + >> +@interface I() >> +{ >> + @public int b; >> +} >> +@end >> + >> +void gorf (I* pg) { >> + // poking into pg for ivars for class extension >> + int _b = pg->b; >> +} >> + >> +// CHECK: metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata !"a", >> metadata !{{[0-9]*}}, i32 7, i64 32, i64 32, i64 0, i32 0, metadata >> !{{[0-9]*}}, null} ; [ DW_TAG_member ] [a] [line 7, size 32, align 32, >> offset 0] [from int] >> +// CHECK: metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata !"b", >> metadata !{{[0-9]*}}, i32 18, i64 32, i64 32, i64 0, i32 0, metadata >> !{{[0-9]*}}, null} ; [ DW_TAG_member ] [b] [line 18, size 32, align 32, >> offset 0] [from int] >> >> Added: 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=176584&view=auto >> ============================================================================== >> --- cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m (added) >> +++ cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m Wed Mar 6 >> 16:03:30 2013 >> @@ -0,0 +1,32 @@ >> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -g %s -o - | >> FileCheck %s >> + >> +// Make sure we generate debug symbols for an indirectly referenced >> +// extension to an interface. >> + >> +@interface I >> +{ >> + @public int a; >> +} >> +@end >> + >> +void foo(I* pi) { >> + int _a = pi->a; >> +} >> + >> +// another layer of indirection >> +struct S >> +{ >> + I* i; >> +}; >> + >> +@interface I() >> +{ >> + @public int b; >> +} >> +@end >> + >> +void gorf (struct S* s) { >> + int _b = s->i->b; >> +} >> + >> +// CHECK: metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata !"b", >> metadata !{{[0-9]*}}, i32 24, i64 32, i64 32, i64 0, i32 0, metadata >> !{{[0-9]*}}, null} ; [ DW_TAG_member ] [b] [line 24, size 32, align 32, >> offset 0] [from int] >> >> Added: cfe/trunk/test/CodeGenObjC/debug-info-ivars-private.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/debug-info-ivars-private.m?rev=176584&view=auto >> ============================================================================== >> --- cfe/trunk/test/CodeGenObjC/debug-info-ivars-private.m (added) >> +++ cfe/trunk/test/CodeGenObjC/debug-info-ivars-private.m Wed Mar 6 >> 16:03:30 2013 >> @@ -0,0 +1,36 @@ >> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -g %s -o - | >> FileCheck %s >> + >> +// Debug symbols for private ivars. This test ensures that we are >> +// generating debug info for ivars added by the implementation. >> +__attribute((objc_root_class)) @interface NSObject { >> + id isa; >> +} >> +@end >> + >> +@protocol Protocol >> +@end >> + >> +@interface Delegate : NSObject<Protocol> { >> + @protected int foo; >> +} >> +@end >> + >> +@interface Delegate(NSObject) >> +- (void)f; >> +@end >> + >> +@implementation Delegate(NSObject) >> +- (void)f { return; } >> +@end >> + >> +@implementation Delegate { >> + int bar; >> +} >> + >> +- (void)g:(NSObject*) anObject { >> + bar = foo; >> +} >> +@end >> + >> +// CHECK: metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata !"foo", >> metadata !{{[0-9]*}}, i32 14, i64 32, i64 32, i64 0, i32 2, metadata >> !{{[0-9]*}}, null} ; [ DW_TAG_member ] [foo] [line 14, size 32, align 32, >> offset 0] [protected] [from int] >> +// CHECK: metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata !"bar", >> metadata !{{[0-9]*}}, i32 27, i64 32, i64 32, i64 0, i32 1, metadata >> !{{[0-9]*}}, null} ; [ DW_TAG_member ] [bar] [line 27, size 32, align 32, >> offset 0] [private] [from int] >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
