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:23:53 PDT 2014


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();





More information about the cfe-commits mailing list