r190122 - PR17046, PR17092: Debug Info assert-on-valid due to member loss when context creation recreates the item the context is created for

David Blaikie dblaikie at gmail.com
Thu Sep 5 23:45:04 PDT 2013


Author: dblaikie
Date: Fri Sep  6 01:45:04 2013
New Revision: 190122

URL: http://llvm.org/viewvc/llvm-project?rev=190122&view=rev
Log:
PR17046, PR17092: Debug Info assert-on-valid due to member loss when context creation recreates the item the context is created for

By removing the possibility of strange partial definitions with no
members that older GCC's produced for the otherwise unreferenced outer
types of referenced inner types, we can simplify debug info generation
and correct this bug. Newer (4.8.1 and ToT) GCC's don't produce this
quirky debug info, and instead produce the full definition for the outer
type (except in the case where that type is dynamic and its vtable is
not emitted in this TU).

During the creation of the context for a type, we may revisit that type
(due to the need to visit template parameters, among other things) and
used to end up visiting it first there. Then when we would reach the
original code attempting to define that type, we would lose debug info
by overwriting its members.

By avoiding the possibility of latent "defined with no members" types,
we can be sure than whenever we already have a type in a cache (either a
definition or declaration), we can just return that. In the case of a
full definition, our work is done. In the case of a partial definition,
we must already be in the process of completing it. And in the case of a
declaration, the completed/vtable/etc callbacks can handle converting it
to a definition.

Modified:
    cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
    cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
    cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=190122&r1=190121&r2=190122&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Sep  6 01:45:04 2013
@@ -650,32 +650,6 @@ CGDebugInfo::getOrCreateRecordFwdDecl(co
                                     FullName);
 }
 
-// Walk up the context chain and create forward decls for record decls,
-// and normal descriptors for namespaces.
-llvm::DIDescriptor CGDebugInfo::createContextChain(const Decl *Context) {
-  if (!Context)
-    return TheCU;
-
-  // See if we already have the parent.
-  llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator
-    I = RegionMap.find(Context);
-  if (I != RegionMap.end()) {
-    llvm::Value *V = I->second;
-    return llvm::DIDescriptor(dyn_cast_or_null<llvm::MDNode>(V));
-  }
-
-  // Check namespace.
-  if (const NamespaceDecl *NSDecl = dyn_cast<NamespaceDecl>(Context))
-    return llvm::DIDescriptor(getOrCreateNameSpace(NSDecl));
-
-  if (const RecordDecl *RD = dyn_cast<RecordDecl>(Context))
-    if (!RD->isDependentType())
-      return getOrCreateLimitedType(
-          CGM.getContext().getRecordType(RD)->castAs<RecordType>(),
-          getOrCreateMainFile());
-  return TheCU;
-}
-
 llvm::DIType CGDebugInfo::CreatePointerLikeType(unsigned Tag,
                                                 const Type *Ty,
                                                 QualType PointeeTy,
@@ -1507,20 +1481,23 @@ llvm::DIType CGDebugInfo::CreateType(con
   // `completeRequiredType`.
   // If the type is dynamic, only emit the definition in TUs that require class
   // data. This is handled by `completeClassData`.
-  if ((DebugKind <= CodeGenOptions::LimitedDebugInfo &&
+  llvm::DICompositeType T(getTypeOrNull(QualType(Ty, 0)));
+  // If we've already emitted the type, just use that, even if it's only a
+  // declaration. The completeType, completeRequiredType, and completeClassData
+  // callbacks will handle promoting the declaration to a definition.
+  if (T ||
+      (DebugKind <= CodeGenOptions::LimitedDebugInfo &&
+       // Under -flimit-debug-info, emit only a declaration unless the type is
+       // required to be complete.
        !RD->isCompleteDefinitionRequired() && CGM.getLangOpts().CPlusPlus) ||
-      (CXXDecl && CXXDecl->hasDefinition() && CXXDecl->isDynamicClass())) {
+      // If the class is dynamic, only emit a declaration. A definition will be
+      // emitted whenever the vtable is emitted.
+      (CXXDecl && CXXDecl->hasDefinition() && CXXDecl->isDynamicClass()) || T) {
     llvm::DIDescriptor FDContext =
       getContextDescriptor(cast<Decl>(RD->getDeclContext()));
-    llvm::DIType RetTy = getOrCreateRecordFwdDecl(Ty, FDContext);
-    // FIXME: This is conservatively correct. If we return a non-forward decl
-    // that's not a full definition (such as those created by
-    // createContextChain) then getOrCreateType will record is as a complete
-    // type and we'll never record all its members. But this means we're
-    // emitting full debug info in TUs where GCC successfully emits a partial
-    // definition of the type.
-    if (RetTy.isForwardDecl())
-      return RetTy;
+    if (!T)
+      T = getOrCreateRecordFwdDecl(Ty, FDContext);
+    return T;
   }
 
   return CreateTypeDefinition(Ty);
@@ -2271,10 +2248,7 @@ llvm::DICompositeType CGDebugInfo::Creat
   StringRef RDName = getClassName(RD);
 
   llvm::DIDescriptor RDContext;
-  if (DebugKind == CodeGenOptions::LimitedDebugInfo)
-    RDContext = createContextChain(cast<Decl>(RD->getDeclContext()));
-  else
-    RDContext = getContextDescriptor(cast<Decl>(RD->getDeclContext()));
+  RDContext = getContextDescriptor(cast<Decl>(RD->getDeclContext()));
 
   // If we ended up creating the type during the context chain construction,
   // just return that.

Modified: cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class.cpp?rev=190122&r1=190121&r2=190122&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/debug-info-class.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-class.cpp Fri Sep  6 01:45:04 2013
@@ -93,7 +93,7 @@ int main(int argc, char **argv) {
 // CHECK: DW_TAG_structure_type ] [foo]
 // CHECK: DW_TAG_class_type ] [bar]
 // CHECK: DW_TAG_union_type ] [baz]
-// CHECK: DW_TAG_class_type ] [B]
+// CHECK: DW_TAG_class_type ] [B] {{.*}} [def]
 // CHECK: metadata !"_vptr$B", {{.*}}, i32 64, metadata !{{.*}}} ; [ DW_TAG_member ]
 
 // CHECK: [[C:![0-9]*]] = {{.*}} metadata [[C_MEM:![0-9]*]], i32 0, metadata [[C]], null, metadata !"_ZTS1C"} ; [ DW_TAG_structure_type ] [C] {{.*}} [def]
@@ -107,13 +107,11 @@ int main(int argc, char **argv) {
 // CHECK: [[D_MEM]] = metadata !{metadata [[D_FUNC:![0-9]*]]}
 // CHECK: [[D_FUNC]] = {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [func]
 // CHECK: null, i32 0, null, null, metadata !"_ZTS1E"} ; [ DW_TAG_structure_type ] [E] {{.*}} [decl]
-// CHECK: [[F:![0-9]*]] = {{.*}} metadata [[F_MEM:![0-9]*]], i32 0, null, null, metadata !"_ZTS1F"} ; [ DW_TAG_structure_type ] [F] {{.*}} [def]
+// CHECK: [[F:![0-9]*]] = {{.*}} metadata [[F_MEM:![0-9]*]], i32 0, null, null, metadata !"_ZTS1F"} ; [ DW_TAG_structure_type ] [F] {{.*}} [decl]
 // CHECK: [[F_MEM]] = metadata !{metadata [[F_I:![0-9]*]]}
 // CHECK: [[F_I]] = {{.*}} ; [ DW_TAG_member ] [i]
 
-// Context chains (in Clang -flimit-debug-info and in GCC generally) contain
-// definitions without members (& without a vbase 'containing type'):
-// CHECK: null, i32 0, null, null, metadata !"_ZTS1G"} ; [ DW_TAG_structure_type ] [G] {{.*}} [def]
+// CHECK: null, i32 0, null, null, metadata !"_ZTS1G"} ; [ DW_TAG_structure_type ] [G] {{.*}} [decl]
 // CHECK: metadata [[G_INNER_MEM:![0-9]*]], i32 0, null, null, metadata !"_ZTSN1G5innerE"} ; [ DW_TAG_structure_type ] [inner] [line 50, {{.*}} [def]
 // CHECK: [[G_INNER_MEM]] = metadata !{metadata [[G_INNER_I:![0-9]*]]}
 // CHECK: [[G_INNER_I]] = {{.*}} ; [ DW_TAG_member ] [j] {{.*}} [from int]

Modified: cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp?rev=190122&r1=190121&r2=190122&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp Fri Sep  6 01:45:04 2013
@@ -23,6 +23,13 @@ inline int add3(int x) {
 // CHECK: [[FOO_FUNC_PARAMS]] = metadata !{null, metadata !{{[0-9]*}}, metadata [[OUTER_FOO_INNER:![0-9]*]]}
 // CHECK: [[OUTER_FOO_INNER]] = {{.*}} ; [ DW_TAG_structure_type ] [inner]
 
+// CHECK: metadata [[VIRT_MEM:![0-9]*]], i32 0, metadata !{{[0-9]*}}, metadata [[VIRT_TEMP_PARAM:![0-9]*]], metadata !"_ZTS4virtI4elemE"} ; [ DW_TAG_structure_type ] [virt<elem>] {{.*}} [def]
+// CHECK: [[ELEM:![0-9]*]] = {{.*}}, metadata [[ELEM_MEM:![0-9]*]], i32 0, null, null, metadata !"_ZTS4elem"} ; [ DW_TAG_structure_type ] [elem] {{.*}} [def]
+// CHECK: [[ELEM_MEM]] = metadata !{metadata [[ELEM_X:![0-9]*]]}
+// CHECK: [[ELEM_X]] = {{.*}} ; [ DW_TAG_member ] [x] {{.*}} [static] [from virt<elem>]
+// CHECK: [[VIRT_TEMP_PARAM]] = metadata !{metadata [[VIRT_T:![0-9]*]]}
+// CHECK: [[VIRT_T]] = {{.*}}, metadata !"T", metadata [[ELEM]], {{.*}} ; [ DW_TAG_template_type_parameter ]
+
 // CHECK: [[C:![0-9]*]] = {{.*}}, metadata [[C_MEM:![0-9]*]], i32 0, metadata [[C]], null, metadata !"_ZTS7MyClass"} ; [ DW_TAG_structure_type ] [MyClass]
 // CHECK: [[C_MEM]] = metadata !{metadata [[C_VPTR:![0-9]*]], metadata [[C_ADD:![0-9]*]], metadata [[C_FUNC:![0-9]*]], metadata [[C_CTOR:![0-9]*]]}
 // CHECK: [[C_VPTR]] = {{.*}} ; [ DW_TAG_member ] [_vptr$MyClass]
@@ -52,3 +59,19 @@ inline void func() {
 outer<foo>::inner x;
 
 // CHECK: metadata [[OUTER_FOO_INNER]], i32 {{[0-9]*}}, i32 {{[0-9]*}}, %"struct.outer<foo>::inner"* @x, {{.*}} ; [ DW_TAG_variable ] [x]
+
+template <typename T>
+struct virt {
+  T* values;
+  virtual ~virt();
+};
+struct elem {
+  static virt<elem> x; // ensure that completing 'elem' will require/completing 'virt<elem>'
+};
+inline void f1() {
+  elem e; // ensure 'elem' is required to be complete when it is emitted as a template argument for 'virt<elem>'
+};
+void f2() {
+  virt<elem> d; // emit 'virt<elem>'
+}
+





More information about the cfe-commits mailing list