r208015 - Build debug info for ObjC interface types at the end of the translation unit to ensure all ivars are included.
David Blaikie
dblaikie at gmail.com
Mon May 5 16:47:08 PDT 2014
On Mon, May 5, 2014 at 4:39 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> This comment for ObjCImplementationDecl is totally bogus now, BTW.
Which part of it? That support for instance variables in the
implementation is a legacy thing? (it isn't a legacy thing?)
That they have to be identical? (do they not? I guess not - I just had
3 @interface, each with a different variable, and one @implementation
with a 4th, distinct, variable)
>
> /// 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
>
>
>
More information about the cfe-commits
mailing list