[codeview] emit debug info for indirect virtual base classes

[inglorion]

Summary

Fixes PR28281.

MSVC lists indirect virtual base classes in the field list of a class.
This change makes Clang emit the information necessary for LLVM to
emit such records.

[rnk]

lib/CodeGen/CGDebugInfo.cpp

1370–1371

Some LLVM style idiosyncracies:
http://llvm.org/docs/CodingStandards.html

• Variables start with leading capitals (I know it's weird)
• We don't usually use auto unless the RHS repeats the type in a cast, make_unique, or new.

1371

Make this a DenseSet<const CXXRecordDecl*>, so that we don't need to call getOrCreateType, which is going to do more hash lookups.

1373

IMO SeenTypes.count(...) would be more idiomatic.

1377

This should be getCodeGenOpts().EmitCodeView.

1391

You should be able to use this as the key into SeenTypes. You also want to do Base = Base->getCanonicalDecl(), though, so that we don't end up with pointers to different redeclarations of the same record. I don't think I can construct a case where this would matter, but it's probably still worth it. :)

[zturner]

lib/CodeGen/CGDebugInfo.cpp

1370–1371

3. Or the type is really gross.
4. Or it's in a range-based for.

1373

What a silly function. I wonder why it isn't called contains() and return a bool.

[ruiu]

lib/CodeGen/CGDebugInfo.cpp

1373

It's probably because std::set doesn't provide contains() but count() too.

[majnemer]

lib/CodeGen/CGDebugInfo.cpp

1373

You can use is_contained from STLExtras.h, it should be maximally idiomatic :)

[inglorion]

@rnk's comments (thanks!)

• Converted SeenTypes to a DenseSet<const CXXRecordDecl*>.
• Switched to getCodeGenOpts().EmitCodeView to check if we should emit the extra records.
• Switched to using SeenTypes.count(...) != 0 to check if we've seen a type.

[inglorion]

• Removed unused header.

[rnk]

Thanks, looks good with some nits

lib/CodeGen/CGDebugInfo.cpp

1391

So, the playing field shifted. Justin Lebar recently added the CanonicalDeclPtr template, which does the getCanonicalDecl dance for you. Probably best to use that. :)

1394–1396

Now that we've figured out the best way to test set membership, I've realized we can save a hash lookup like this:

if (SeenTypes.insert(CanonicalBase).second)
  continue;

[inglorion]

Updated to track the latest state of D25578

[inglorion]

Use insert's return value to save a set lookup, and use CanonicalDeclPtr

[inglorion]

We also need D25578 in before this can land.

lib/CodeGen/CGDebugInfo.h

 // This is the source-level debug info generator for llvm translation.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGDEBUGINFO_H
 #define LLVM_CLANG_LIB_CODEGEN_CGDEBUGINFO_H
 
 #include "CGBuilder.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Allocator.h"
 
 namespace llvm {
 class MDNode;
 }
 
 namespace clang {
-class CXXMethodDecl;
 class ClassTemplateSpecializationDecl;
 class GlobalDecl;
 class ModuleMap;
 class ObjCInterfaceDecl;
 class ObjCIvarDecl;
 class UsingDecl;
 class VarDecl;
 
#include "clang/Basic/OpenCLImageTypes.def"
 
   /// A helper function to collect debug info for C++ base
   /// classes. This is used while creating debug info entry for a
   /// Record.
   void CollectCXXBases(const CXXRecordDecl *Decl, llvm::DIFile *F,
                        SmallVectorImpl<llvm::Metadata *> &EltTys,
                        llvm::DIType *RecordTy);
 
+  /// Helper function for CollectCXXBases.
+  /// Adds debug info entries for types in Bases that are not in SeenTypes.
+  void CollectCXXBasesAux(const CXXRecordDecl *RD, llvm::DIFile *Unit,
+                          SmallVectorImpl<llvm::Metadata *> &EltTys,
+                          llvm::DIType *RecordTy,
+                          const CXXRecordDecl::base_class_const_range &Bases,
+                          llvm::DenseSet<const CXXRecordDecl *> &SeenTypes,
+                          llvm::DINode::DIFlags StartingFlags);
+
   /// A helper function to collect template parameters.
   llvm::DINodeArray CollectTemplateParams(const TemplateParameterList *TPList,
                                           ArrayRef<TemplateArgument> TAList,
                                           llvm::DIFile *Unit);
   /// A helper function to collect debug info for function template
   /// parameters.
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
                                                   llvm::DIFile *Unit);

lib/CodeGen/CGDebugInfo.cpp

 //===--- CGDebugInfo.cpp - Emit Debug Information for a Module ------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This coordinates the debug information generation while generating code.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CGDebugInfo.h"
 #include "CGBlocks.h"
-#include "CGRecordLayout.h"
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/ModuleMap.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
for (const auto *I : RD->decls()) {
     EltTys.push_back(MI == SPCache.end()
                          ? CreateCXXMemberFunction(Method, Unit, RecordTy)
                          : static_cast<llvm::Metadata *>(MI->second));
   }
 }
 
 void CGDebugInfo::CollectCXXBases(const CXXRecordDecl *RD, llvm::DIFile *Unit,
                                   SmallVectorImpl<llvm::Metadata *> &EltTys,
                                   llvm::DIType *RecordTy) {
-  const ASTRecordLayout &RL = CGM.getContext().getASTRecordLayout(RD);
-  for (const auto &BI : RD->bases()) {
-    llvm::DINode::DIFlags BFlags = llvm::DINode::FlagZero;
-    uint64_t BaseOffset;
+  llvm::DenseSet<const CXXRecordDecl *> SeenTypes;
+  CollectCXXBasesAux(RD, Unit, EltTys, RecordTy, RD->bases(), SeenTypes,
+                     llvm::DINode::FlagZero);
 
+  // If we are generating CodeView debug info, we also need to emit records for
+  // indirect virtual base classes.
+  if (CGM.getCodeGenOpts().EmitCodeView) {
+    CollectCXXBasesAux(RD, Unit, EltTys, RecordTy, RD->vbases(), SeenTypes,
+                       llvm::DINode::FlagIndirect | llvm::DINode::FlagVirtual);
+  }
+}
+
+void CGDebugInfo::CollectCXXBasesAux(
+    const CXXRecordDecl *RD, llvm::DIFile *Unit,
+    SmallVectorImpl<llvm::Metadata *> &EltTys, llvm::DIType *RecordTy,
+    const CXXRecordDecl::base_class_const_range &Bases,
+    llvm::DenseSet<const CXXRecordDecl *> &SeenTypes,
+    llvm::DINode::DIFlags StartingFlags) {
+  const ASTRecordLayout &RL = CGM.getContext().getASTRecordLayout(RD);
+  for (const auto &BI : Bases) {
     const auto *Base =
         cast<CXXRecordDecl>(BI.getType()->getAs<RecordType>()->getDecl());
+    const auto *CanonicalBase = Base->getCanonicalDecl();
+    if (is_contained(SeenTypes, CanonicalBase))
+      continue;
+    SeenTypes.insert(CanonicalBase);
+    auto *BaseTy = getOrCreateType(BI.getType(), Unit);
+    llvm::DINode::DIFlags BFlags = StartingFlags;
+    uint64_t BaseOffset;
 
     if (BI.isVirtual()) {
       if (CGM.getTarget().getCXXABI().isItaniumFamily()) {
         // virtual base offset offset is -ve. The code generator emits dwarf
         // expression where it expects +ve number.
         BaseOffset = 0 - CGM.getItaniumVTableContext()
                              .getVirtualBaseOffsetOffset(RD, Base)
                              .getQuantity();
       } else {
         // In the MS ABI, store the vbtable offset, which is analogous to the
         // vbase offset offset in Itanium.
         BaseOffset =
             4 * CGM.getMicrosoftVTableContext().getVBTableIndex(RD, Base);
       }
-      BFlags = llvm::DINode::FlagVirtual;
+      BFlags |= llvm::DINode::FlagVirtual;
     } else
       BaseOffset = CGM.getContext().toBits(RL.getBaseClassOffset(Base));
     // FIXME: Inconsistent units for BaseOffset. It is in bytes when
     // BI->isVirtual() and bits when not.
 
     BFlags |= getAccessFlag(BI.getAccessSpecifier(), RD);
-    llvm::DIType *DTy = DBuilder.createInheritance(
-        RecordTy, getOrCreateType(BI.getType(), Unit), BaseOffset, BFlags);
+    llvm::DIType *DTy =
+        DBuilder.createInheritance(RecordTy, BaseTy, BaseOffset, BFlags);
     EltTys.push_back(DTy);
   }
 }
 
 llvm::DINodeArray
 CGDebugInfo::CollectTemplateParams(const TemplateParameterList *TPList,
                                    ArrayRef<TemplateArgument> TAList,
                                    llvm::DIFile *Unit) {

test/CodeGenCXX/debug-info-ms-vbase.cpp

 
 // CHECK: ![[SecondaryVTable_base]] = !DIDerivedType(tag: DW_TAG_inheritance, scope: ![[HasPrimaryBase]],
 // CHECK-SAME: baseType: ![[SecondaryVTable:[0-9]+]], offset: 4, flags: DIFlagVirtual)
 
 // CHECK: ![[SecondaryVTable]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "SecondaryVTable"
 
 // CHECK: ![[HasVirtualMethod_base]] = !DIDerivedType(tag: DW_TAG_inheritance, scope: ![[HasPrimaryBase]], baseType: ![[HasVirtualMethod]])
 
+// CHECK: ![[HasIndirectVirtualBase:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "HasIndirectVirtualBase"
+// CHECK-SAME: elements: ![[elements:[0-9]+]]
+
+// CHECK: !DIDerivedType(tag: DW_TAG_inheritance, scope: ![[HasIndirectVirtualBase]], baseType: ![[HasPrimaryBase]]
+// CHECK-NOT: DIFlagIndirect
+// CHECK-SAME: )
+
+// CHECK: !DIDerivedType(tag: DW_TAG_inheritance, scope: ![[HasIndirectVirtualBase]], baseType: ![[SecondaryVTable]]
+// CHECK-SAME: flags:
+// CHECK-SAME: DIFlagVirtual
+// CHECK-SAME: DIFlagIndirect
+
 // CHECK: ![[DynamicNoVFPtr:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "DynamicNoVFPtr",
 // CHECK-SAME: elements: ![[elements:[0-9]+]]
 
 // CHECK: ![[elements]] = !{![[POD_base:[0-9]+]]}
 
 // CHECK: ![[POD_base]] = !DIDerivedType(tag: DW_TAG_inheritance, scope: ![[DynamicNoVFPtr]],
 // CHECK-SAME: baseType: ![[POD:[0-9]+]], offset: 4, flags: DIFlagVirtual)
 

 NoPrimaryBase no_primary_base;
 
 struct SecondaryVTable { virtual void g(); };
 
 struct HasPrimaryBase : virtual SecondaryVTable, HasVirtualMethod { };
 
 HasPrimaryBase has_primary_base;
 
+struct HasIndirectVirtualBase : public HasPrimaryBase {};
+
+HasIndirectVirtualBase has_indirect_virtual_base;