r188739 - Revert "Revert "Revert "Revert "DebugInfo: Omit debug info for dynamic classes in TUs that do not have the vtable for that class""""

Michael Gottesman mgottesman at apple.com
Mon Aug 19 21:38:14 PDT 2013


No, Thanks to Dave for being patient with my reproduction especially given the revert, revert, revert, revert sequence that has occurred.

= ).

Michael

On Aug 19, 2013, at 6:28 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Mon Aug 19 20:28:15 2013
> New Revision: 188739
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=188739&view=rev
> Log:
> Revert "Revert "Revert "Revert "DebugInfo: Omit debug info for dynamic classes in TUs that do not have the vtable for that class""""
> 
> This reverts commit r188687 (reverts r188642 (reverts 188600 (reverts
> 188576))).
> 
> With added test coverage & fix for -gline-tables-only.
> 
> Thanks Michael Gottesman for reverting this patch when it demonstrated
> problems & providing a reproduction/details to help me track this down.
> 
> Modified:
>    cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>    cfe/trunk/lib/CodeGen/CGDebugInfo.h
>    cfe/trunk/lib/CodeGen/CGVTables.cpp
>    cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
>    cfe/trunk/test/CodeGenCXX/debug-info-gline-tables-only.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=188739&r1=188738&r2=188739&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Aug 19 20:28:15 2013
> @@ -644,8 +644,24 @@ llvm::DIDescriptor CGDebugInfo::createCo
> 
>   if (const RecordDecl *RD = dyn_cast<RecordDecl>(Context)) {
>     if (!RD->isDependentType()) {
> -      llvm::DIType Ty = getOrCreateLimitedType(
> -          CGM.getContext().getRecordType(RD)->castAs<RecordType>(), getOrCreateMainFile());
> +      llvm::DICompositeType T(getTypeOrNull(CGM.getContext().getRecordType(RD)));
> +      llvm::DICompositeType Ty(getOrCreateLimitedType(
> +          CGM.getContext().getRecordType(RD)->castAs<RecordType>(),
> +          getOrCreateMainFile()));
> +      if (!Ty.getTypeArray().getNumElements()) {
> +        if (T) {
> +          llvm::DIArray PrevMem = T.getTypeArray();
> +          unsigned NumElements = PrevMem.getNumElements();
> +          if (NumElements == 1 && !PrevMem.getElement(0))
> +            NumElements = 0;
> +          SmallVector<llvm::Value *, 16> EltTys;
> +          EltTys.reserve(NumElements);
> +          for (unsigned i = 0; i != NumElements; ++i)
> +            EltTys.push_back(PrevMem.getElement(i));
> +          llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);
> +          Ty.setTypeArray(Elements);
> +        }
> +      }
>       return llvm::DIDescriptor(Ty);
>     }
>   }
> @@ -865,7 +881,7 @@ CollectRecordLambdaFields(const CXXRecor
>   }
> }
> 
> -/// CollectRecordStaticField - Helper for CollectRecordFields.
> +/// Helper for CollectRecordFields.
> llvm::DIDerivedType
> CGDebugInfo::CreateRecordStaticField(const VarDecl *Var,
>                                      llvm::DIType RecordTy) {
> @@ -948,7 +964,7 @@ void CGDebugInfo::CollectRecordFields(co
>     for (RecordDecl::decl_iterator I = record->decls_begin(),
>            E = record->decls_end(); I != E; ++I)
>       if (const VarDecl *V = dyn_cast<VarDecl>(*I))
> -        elements.push_back(CreateRecordStaticField(V, RecordTy));
> +        elements.push_back(getOrCreateStaticDataMemberDeclaration(V, RecordTy));
>       else if (FieldDecl *field = dyn_cast<FieldDecl>(*I)) {
>         CollectRecordNormalField(field, layout.getFieldOffset(fieldNo),
>                                  tunit, elements, RecordTy);
> @@ -1123,8 +1139,14 @@ CollectCXXMemberFunctions(const CXXRecor
>     if (D->isImplicit())
>       continue;
> 
> -    if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
> -      EltTys.push_back(CreateCXXMemberFunction(Method, Unit, RecordTy));
> +    if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
> +      llvm::DenseMap<const FunctionDecl *, llvm::WeakVH>::iterator MI =
> +          SPCache.find(Method->getCanonicalDecl());
> +      if (MI == SPCache.end())
> +        EltTys.push_back(CreateCXXMemberFunction(Method, Unit, RecordTy));
> +      else
> +        EltTys.push_back(MI->second);
> +    }
>   }
> }
> 
> @@ -1408,10 +1430,20 @@ void CGDebugInfo::completeType(const Rec
> }
> 
> void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
> +  if (const CXXRecordDecl *CXXDecl = dyn_cast<CXXRecordDecl>(RD))
> +    if (CXXDecl->isDynamicClass())
> +      return;
> +
>   QualType Ty = CGM.getContext().getRecordType(RD);
>   llvm::DIType T = getTypeOrNull(Ty);
> -  if (!T || !T.isForwardDecl())
> +  if (T && T.isForwardDecl())
> +    completeClassData(RD);
> +}
> +
> +void CGDebugInfo::completeClassData(const RecordDecl *RD) {
> +  if (DebugKind <= CodeGenOptions::DebugLineTablesOnly)
>     return;
> +  QualType Ty = CGM.getContext().getRecordType(RD);
>   void* TyPtr = Ty.getAsOpaquePtr();
>   if (CompletedTypeCache.count(TyPtr))
>     return;
> @@ -1424,14 +1456,23 @@ void CGDebugInfo::completeRequiredType(c
> /// CreateType - get structure or union type.
> llvm::DIType CGDebugInfo::CreateType(const RecordType *Ty, bool Declaration) {
>   RecordDecl *RD = Ty->getDecl();
> +  const CXXRecordDecl *CXXDecl = dyn_cast<CXXRecordDecl>(RD);
>   // Limited debug info should only remove struct definitions that can
>   // safely be replaced by a forward declaration in the source code.
> -  if (DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration &&
> -      !RD->isCompleteDefinitionRequired() && CGM.getLangOpts().CPlusPlus) {
> +  if ((DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration &&
> +       !RD->isCompleteDefinitionRequired() && CGM.getLangOpts().CPlusPlus) ||
> +      (CXXDecl && CXXDecl->hasDefinition() && CXXDecl->isDynamicClass())) {
>     llvm::DIDescriptor FDContext =
>       getContextDescriptor(cast<Decl>(RD->getDeclContext()));
>     llvm::DIType RetTy = getOrCreateRecordFwdDecl(RD, FDContext);
> -    return RetTy;
> +    // 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;
>   }
> 
>   return CreateTypeDefinition(Ty);
> @@ -1469,6 +1510,7 @@ llvm::DIType CGDebugInfo::CreateTypeDefi
> 
>   // Convert all the elements.
>   SmallVector<llvm::Value *, 16> EltTys;
> +  // what about nested types?
> 
>   // Note: The split of CXXDecl information here is intentional, the
>   // gdb tests will depend on a certain ordering at printout. The debug
> @@ -2310,7 +2352,7 @@ llvm::DISubprogram CGDebugInfo::getFunct
>   llvm::DenseMap<const FunctionDecl *, llvm::WeakVH>::iterator
>     MI = SPCache.find(FD->getCanonicalDecl());
>   if (MI == SPCache.end()) {
> -    if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
> +    if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD->getCanonicalDecl())) {
>       llvm::DICompositeType T(S);
>       llvm::DISubprogram SP = CreateCXXMemberFunction(MD, getOrCreateFile(MD->getLocation()), T);
>       T.addMember(SP);
> @@ -3025,19 +3067,35 @@ void CGDebugInfo::EmitDeclareOfBlockLite
>   DbgDecl->setDebugLoc(llvm::DebugLoc::get(line, column, scope));
> }
> 
> -/// getStaticDataMemberDeclaration - If D is an out-of-class definition of
> -/// a static data member of a class, find its corresponding in-class
> -/// declaration.
> -llvm::DIDerivedType CGDebugInfo::getStaticDataMemberDeclaration(const VarDecl *D) {
> -  if (D->isStaticDataMember()) {
> -    llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator
> -      MI = StaticDataMemberCache.find(D->getCanonicalDecl());
> -    if (MI != StaticDataMemberCache.end())
> -      // Verify the info still exists.
> -      if (llvm::Value *V = MI->second)
> -        return llvm::DIDerivedType(cast<llvm::MDNode>(V));
> +/// If D is an out-of-class definition of a static data member of a class, find
> +/// its corresponding in-class declaration.
> +llvm::DIDerivedType
> +CGDebugInfo::getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl *D) {
> +  if (!D->isStaticDataMember())
> +    return llvm::DIDerivedType();
> +  llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator MI =
> +      StaticDataMemberCache.find(D->getCanonicalDecl());
> +  if (MI != StaticDataMemberCache.end()) {
> +    assert(MI->second && "Static data member declaration should still exist");
> +    return llvm::DIDerivedType(cast<llvm::MDNode>(MI->second));
> +  }
> +  llvm::DICompositeType Ctxt(
> +      getContextDescriptor(cast<Decl>(D->getDeclContext())));
> +  llvm::DIDerivedType T = CreateRecordStaticField(D, Ctxt);
> +  Ctxt.addMember(T);
> +  return T;
> +}
> +
> +llvm::DIDerivedType
> +CGDebugInfo::getOrCreateStaticDataMemberDeclaration(const VarDecl *D,
> +                                            llvm::DICompositeType Ctxt) {
> +  llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator MI =
> +      StaticDataMemberCache.find(D->getCanonicalDecl());
> +  if (MI != StaticDataMemberCache.end()) {
> +    assert(MI->second && "Static data member declaration should still exist");
> +    return llvm::DIDerivedType(cast<llvm::MDNode>(MI->second));
>   }
> -  return llvm::DIDerivedType();
> +  return CreateRecordStaticField(D, Ctxt);
> }
> 
> /// EmitGlobalVariable - Emit information about a global variable.
> @@ -3069,11 +3127,10 @@ void CGDebugInfo::EmitGlobalVariable(llv
>     LinkageName = StringRef();
>   llvm::DIDescriptor DContext =
>     getContextDescriptor(dyn_cast<Decl>(D->getDeclContext()));
> -  llvm::DIGlobalVariable GV =
> -      DBuilder.createStaticVariable(DContext, DeclName, LinkageName, Unit,
> -                                    LineNo, getOrCreateType(T, Unit),
> -                                    Var->hasInternalLinkage(), Var,
> -                                    getStaticDataMemberDeclaration(D));
> +  llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
> +      DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
> +      Var->hasInternalLinkage(), Var,
> +      getOrCreateStaticDataMemberDeclarationOrNull(D));
>   DeclCache.insert(std::make_pair(D->getCanonicalDecl(), llvm::WeakVH(GV)));
> }
> 
> @@ -3121,7 +3178,7 @@ void CGDebugInfo::EmitGlobalVariable(con
>     return;
>   llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
>       Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init,
> -      getStaticDataMemberDeclaration(cast<VarDecl>(VD)));
> +      getOrCreateStaticDataMemberDeclarationOrNull(cast<VarDecl>(VD)));
>   DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV)));
> }
> 
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=188739&r1=188738&r2=188739&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon Aug 19 20:28:15 2013
> @@ -291,7 +291,7 @@ public:
> 
>   void completeType(const RecordDecl *RD);
>   void completeRequiredType(const RecordDecl *RD);
> -
> +  void completeClassData(const RecordDecl *RD);
> 
> private:
>   /// EmitDeclare - Emit call to llvm.dbg.declare for a variable declaration.
> @@ -354,10 +354,13 @@ private:
>   /// declaration for the given method definition.
>   llvm::DISubprogram getFunctionDeclaration(const Decl *D);
> 
> -  /// getStaticDataMemberDeclaration - Return debug info descriptor to
> -  /// describe in-class static data member declaration for the given
> -  /// out-of-class definition.
> -  llvm::DIDerivedType getStaticDataMemberDeclaration(const VarDecl *D);
> +  /// Return debug info descriptor to describe in-class static data member
> +  /// declaration for the given out-of-class definition.
> +  llvm::DIDerivedType
> +  getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl *D);
> +  llvm::DIDerivedType
> +  getOrCreateStaticDataMemberDeclaration(const VarDecl *D,
> +                                         llvm::DICompositeType Ctxt);
> 
>   /// getFunctionName - Get function name for the given FunctionDecl. If the
>   /// name is constructred on demand (e.g. C++ destructor) then the name
> 
> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=188739&r1=188738&r2=188739&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Mon Aug 19 20:28:15 2013
> @@ -828,6 +828,9 @@ CodeGenVTables::GenerateClassData(const
>     VFTContext->getVFPtrOffsets(RD);
>   }
> 
> +  if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
> +    DI->completeClassData(RD);
> +
>   // First off, check whether we've already emitted the v-table and
>   // associated stuff.
>   llvm::GlobalVariable *VTable = GetAddrOfVTable(RD);
> 
> 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=188739&r1=188738&r2=188739&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-class.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class.cpp Mon Aug 19 20:28:15 2013
> @@ -13,7 +13,39 @@ public:
>   virtual ~B();
> };
> 
> +B::~B() {
> +}
> +
> struct C {
> +  static int s;
> +  virtual ~C();
> +};
> +
> +C::~C() {
> +}
> +
> +struct D {
> +  D();
> +  virtual ~D();
> +  void func() {
> +  }
> +};
> +
> +struct E {
> +  E();
> +  virtual ~E();
> +  virtual void func() {
> +  }
> +};
> +
> +struct F {
> +  struct inner {
> +  };
> +  static const int i = 2;
> +  virtual ~F();
> +};
> +
> +struct G {
>   virtual void func();
>   struct inner {
>     int j;
> @@ -29,10 +61,17 @@ struct A {
>   }
> };
> 
> +void f1() {
> +  D x;
> +  x.func();
> +  E y;
> +  int i = F::i;
> +  F::inner z;
> +}
> 
> int main(int argc, char **argv) {
>   B b;
> -  C::inner c_i;
> +  G::inner c_i;
>   if (argc) {
>     A a;
>   }
> @@ -49,15 +88,34 @@ 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_structure_type ] [A]
> -// CHECK: HdrSize
> // CHECK: DW_TAG_class_type ] [B]
> // CHECK: metadata !"_vptr$B", {{.*}}, i32 64, metadata !{{.*}}} ; [ DW_TAG_member ]
> -// CHECK: metadata [[C_INNER_MEM:![0-9]*]], i32 0, null, null} ; [ DW_TAG_structure_type ] [inner] {{.*}} [def]
> +
> +// CHECK: [[C:![0-9]*]] = {{.*}} metadata [[C_MEM:![0-9]*]], i32 0, metadata [[C]], null} ; [ DW_TAG_structure_type ] [C] {{.*}} [def]
> +// CHECK: [[C_MEM]] = metadata !{metadata [[C_VPTR:![0-9]*]], metadata [[C_S:![0-9]*]], metadata [[C_DTOR:![0-9]*]]}
> +// CHECK: [[C_VPTR]] = {{.*}} ; [ DW_TAG_member ] [_vptr$C] {{.*}} [artificial]
> +// CHECK: [[C_S]] = {{.*}} ; [ DW_TAG_member ] [s] {{.*}} [static] [from int]
> +// CHECK: [[C_DTOR]] = {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [~C]
> +
> +// CHECK: ; [ DW_TAG_structure_type ] [A]
> +// CHECK: HdrSize
> +// CHECK: metadata [[D_MEM:![0-9]*]], i32 0, null} ; [ DW_TAG_structure_type ] [D] {{.*}} [decl]
> +// CHECK: [[D_MEM]] = metadata !{metadata [[D_FUNC:![0-9]*]]}
> +// CHECK: [[D_FUNC]] = {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [func]
> +
> +// CHECK: [[F_I_DEF:![0-9]*]] = {{.*}}, metadata [[F_I:![0-9]*]]} ; [ DW_TAG_variable ] [i]
> +// CHECK: [[F_I]] = {{.*}} ; [ DW_TAG_member ] [i]
> +// CHECK: [[F:![0-9]*]] = {{.*}} metadata [[F_MEM:![0-9]*]], i32 0, null, null} ; [ DW_TAG_structure_type ] [F] {{.*}} [def]
> +// CHECK: [[F_MEM]] = metadata !{metadata [[F_I]]}
> +
> +// CHECK: null, i32 0, null} ; [ DW_TAG_structure_type ] [E] {{.*}} [decl]
> +
> +// CHECK: metadata [[G_INNER_MEM:![0-9]*]], i32 0, null, null} ; [ DW_TAG_structure_type ] [inner] [line 50, {{.*}} [def]
> // 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} ; [ DW_TAG_structure_type ] [C] {{.*}} [def]
> -// CHECK: [[C_INNER_MEM]] = metadata !{metadata [[C_INNER_I:![0-9]*]]}
> -// CHECK: [[C_INNER_I]] = {{.*}} ; [ DW_TAG_member ] [j] {{.*}} [from int]
> -// CHECK: ![[EXCEPTLOC]] = metadata !{i32 40,
> -// CHECK: ![[RETLOC]] = metadata !{i32 39,
> +// CHECK: null, i32 0, null, null} ; [ DW_TAG_structure_type ] [G] {{.*}} [def]
> +// CHECK: [[G_INNER_MEM]] = metadata !{metadata [[G_INNER_I:![0-9]*]]}
> +// CHECK: [[G_INNER_I]] = {{.*}} ; [ DW_TAG_member ] [j] {{.*}} [from int]
> +
> +// CHECK: ![[EXCEPTLOC]] = metadata !{i32 79,
> +// CHECK: ![[RETLOC]] = metadata !{i32 78,
> 
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-gline-tables-only.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-gline-tables-only.cpp?rev=188739&r1=188738&r2=188739&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-gline-tables-only.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-gline-tables-only.cpp Mon Aug 19 20:28:15 2013
> @@ -13,9 +13,17 @@ class E : public C {
>   // CHECK-NOT: DW_TAG_reference type
>   void x(const D& d);
> };
> +struct F {
> +  enum X { };
> +  void func(X);
> +  virtual ~F();
> +};
> +F::~F() {
> +}
> }
> 
> // CHECK-NOT: DW_TAG_variable
> NS::C c;
> NS::D d;
> NS::E e;
> +NS::F f;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130819/620e2269/attachment.html>


More information about the cfe-commits mailing list