r176584 - Ensure that DIType is regenerated after we visit an implementation
David Blaikie
dblaikie at gmail.com
Sun May 4 17:09:21 PDT 2014
On Wed, Mar 6, 2013 at 2:03 PM, Adrian Prantl <aprantl at apple.com> 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.
> +
> + at interface I
> +{
> + @public int a;
> +}
> + at end
> +
> +void foo(I* pi) {
> + // poking into pi for primary class ivars.
> + int _a = pi->a;
> +}
> +
> + at interface I()
> +{
> + @public int b;
> +}
> + at 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.
> +
> + at interface I
> +{
> + @public int a;
> +}
> + at end
> +
> +void foo(I* pi) {
> + int _a = pi->a;
> +}
> +
> +// another layer of indirection
> +struct S
> +{
> + I* i;
> +};
> +
> + at interface I()
> +{
> + @public int b;
> +}
> + at 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;
> +}
> + at end
> +
> + at protocol Protocol
> + at end
> +
> + at interface Delegate : NSObject<Protocol> {
> + @protected int foo;
> +}
> + at end
> +
> + at interface Delegate(NSObject)
> +- (void)f;
> + at end
> +
> + at implementation Delegate(NSObject)
> +- (void)f { return; }
> + at end
> +
> + at implementation Delegate {
> + int bar;
> +}
> +
> +- (void)g:(NSObject*) anObject {
> + bar = foo;
> +}
> + at 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list