Reland "[codeview] Reference types in type parent scopes"

[akhuang]

Summary

Original description (https://reviews.llvm/org/D69924)
Without this change, when a nested tag type of any kind (enum, class,
struct, union) is used as a variable type, it is emitted without
emitting the parent type. In CodeView, parent types point to their inner
types, and inner types do not point back to their parents. We already
walk over all of the parent scopes to build the fully qualified name.
This change simply requests their type indices as we go along to enusre
they are all emitted.

Now, while walking over the parent scopes, check whether the type is already
in the process of being emitted before requesting its type index, to avoid
trying to record the same type index twice.

Fixes PR43905

[rnk]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

323

Can you add a test that fails if this check is removed? You should be able to reduce one out of the crash bug Hans filed.

325–327

As an alternative, I think we should be able to add this type to the DeferredCompleteTypes list. We should get the same behavior without the extra conditional.

All this type index lowering code is based on the idea that it is impossible to make a cycle in the type graph without looking through a complete struct type. i.e. you cannot have a type which points to itself. You can make a linked list that points to the struct, but that is implemented by pointing to a forward declaration.

[akhuang]

Add test case that errors without this check

[akhuang]

Change to add type to DeferredCompleteTypes

[rnk]

lgtm, thanks!

[akhuang]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

323

Yep, not sure why I didn't add a test before

325–327

Oh, ok. Using DeferredCompleteTypes looks good.

[akhuang]

remove stray whitespace

[amccarth]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

336

What's the advantage of making this file-scope static function into a method? It doesn't seem to access any member variables.

If you just want to group it with related methods, can it be class-static or at least const?

Making this one non-static seems to ripple through the next two methods as well.

[rnk]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

336

I think you're right, this one doesn't need to be a method, it can remain a static helper. The other ones call collectParentScopes which does access a member now.

[akhuang]

change formatNestedName back to a static function.

[amccarth]

Thanks!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
   std::tuple<codeview::TypeIndex, codeview::TypeIndex, unsigned, bool>
   lowerRecordFieldList(const DICompositeType *Ty);
 
   /// Inserts {{Node, ClassTy}, TI} into TypeIndices and checks for duplicates.
   codeview::TypeIndex recordTypeIndexForDINode(const DINode *Node,
                                                codeview::TypeIndex TI,
                                                const DIType *ClassTy = nullptr);
 
+  /// Collect the names of parent scopes, innermost to outermost. Return the
+  /// innermost subprogram scope if present. Ensure that parent type scopes are
+  /// inserted into the type table.
+  const DISubprogram *
+  collectParentScopeNames(const DIScope *Scope,
+                          SmallVectorImpl<StringRef> &ParentScopeNames);
+  std::string getFullyQualifiedName(const DIScope *Scope, StringRef Name);
+  std::string getFullyQualifiedName(const DIScope *Scope);
+
   unsigned getPointerSizeInBytes();
 
 protected:
   /// Gather pre-function debug information.
   void beginFunctionImpl(const MachineFunction *MF) override;
 
   /// Gather post-function debug information.
   void endFunctionImpl(const MachineFunction *) override;

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

case dwarf::DW_TAG_union_type:
     return "<unnamed-tag>";
   case dwarf::DW_TAG_namespace:
     return "`anonymous namespace'";
   }
 
   return StringRef();
 }
 
-static const DISubprogram *getQualifiedNameComponents(
+const DISubprogram *CodeViewDebug::collectParentScopeNames(
     const DIScope *Scope, SmallVectorImpl<StringRef> &QualifiedNameComponents) {
   const DISubprogram *ClosestSubprogram = nullptr;
   while (Scope != nullptr) {
     if (ClosestSubprogram == nullptr)
       ClosestSubprogram = dyn_cast<DISubprogram>(Scope);
+
+    // If a type appears in a scope chain, make sure it gets emitted. The
+    // frontend will be responsible for deciding if this should be a forward
+    // declaration or a complete type.
+    if (const auto *Ty = dyn_cast<DICompositeType>(Scope))
+      DeferredCompleteTypes.push_back(Ty);
+
     StringRef ScopeName = getPrettyScopeName(Scope);
     if (!ScopeName.empty())
       QualifiedNameComponents.push_back(ScopeName);
     Scope = Scope->getScope();
   }
   return ClosestSubprogram;
 }
 
-static std::string getQualifiedName(ArrayRef<StringRef> QualifiedNameComponents,
+static std::string formatNestedName(ArrayRef<StringRef> QualifiedNameComponents,
                                     StringRef TypeName) {
   std::string FullyQualifiedName;
   for (StringRef QualifiedNameComponent :
        llvm::reverse(QualifiedNameComponents)) {
     FullyQualifiedName.append(std::string(QualifiedNameComponent));
     FullyQualifiedName.append("::");
   }
   FullyQualifiedName.append(std::string(TypeName));
   return FullyQualifiedName;
 }
 
-static std::string getFullyQualifiedName(const DIScope *Scope, StringRef Name) {
+std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Scope,
+                                                 StringRef Name) {
   SmallVector<StringRef, 5> QualifiedNameComponents;
-  getQualifiedNameComponents(Scope, QualifiedNameComponents);
-  return getQualifiedName(QualifiedNameComponents, Name);
+  collectParentScopeNames(Scope, QualifiedNameComponents);
+  return formatNestedName(QualifiedNameComponents, Name);
+}
+
+std::string CodeViewDebug::getFullyQualifiedName(const DIScope *Ty) {
+  const DIScope *Scope = Ty->getScope();
+  return getFullyQualifiedName(Scope, getPrettyScopeName(Ty));
 }
 
 struct CodeViewDebug::TypeLoweringScope {
   TypeLoweringScope(CodeViewDebug &CVD) : CVD(CVD) { ++CVD.TypeEmissionLevel; }
   ~TypeLoweringScope() {
     // Don't decrement TypeEmissionLevel until after emitting deferred types, so
     // inner TypeLoweringScopes don't attempt to emit deferred types.
     if (CVD.TypeEmissionLevel == 1)
       CVD.emitDeferredCompleteTypes();
     --CVD.TypeEmissionLevel;
   }
   CodeViewDebug &CVD;
 };
 
-static std::string getFullyQualifiedName(const DIScope *Ty) {
-  const DIScope *Scope = Ty->getScope();
-  return getFullyQualifiedName(Scope, getPrettyScopeName(Ty));
-}
-
 TypeIndex CodeViewDebug::getScopeIndex(const DIScope *Scope) {
   // No scope means global scope and that uses the zero index.
   if (!Scope || isa<DIFile>(Scope))
     return TypeIndex();
 
   assert(!isa<DIType>(Scope) && "shouldn't make a namespace scope for a type");
 
   // Check if we've already translated this scope.

 
 void CodeViewDebug::addToUDTs(const DIType *Ty) {
   // Don't record empty UDTs.
   if (Ty->getName().empty())
     return;
   if (!shouldEmitUdt(Ty))
     return;
 
-  SmallVector<StringRef, 5> QualifiedNameComponents;
+  SmallVector<StringRef, 5> ParentScopeNames;
   const DISubprogram *ClosestSubprogram =
-      getQualifiedNameComponents(Ty->getScope(), QualifiedNameComponents);
+      collectParentScopeNames(Ty->getScope(), ParentScopeNames);
 
   std::string FullyQualifiedName =
-      getQualifiedName(QualifiedNameComponents, getPrettyScopeName(Ty));
+      formatNestedName(ParentScopeNames, getPrettyScopeName(Ty));
 
   if (ClosestSubprogram == nullptr) {
     GlobalUDTs.emplace_back(std::move(FullyQualifiedName), Ty);
   } else if (ClosestSubprogram == CurrentSubprogram) {
     LocalUDTs.emplace_back(std::move(FullyQualifiedName), Ty);
   }
 
   // TODO: What if the ClosestSubprogram is neither null or the current

llvm/test/DebugInfo/COFF/parent-type-scopes.ll

+; RUN: llc < %s -filetype=obj -o %t.o
+; RUN: llvm-pdbutil dump -types %t.o | FileCheck %s
+
+; C++ source:
+; // Note that MSVC doesn't emit anything about WrapTypedef or WrapTypedef::Inner!
+; struct WrapTypedef {
+;   typedef int Inner;
+; };
+; struct WrapStruct {
+;   struct Inner { int x; };
+; };
+; struct WrapClass {
+;   class Inner { public: int x; };
+; };
+; struct WrapEnum {
+;   enum Inner { One, Two };
+; };
+; struct WrapUnion {
+;   union Inner { int x; float y; };
+; };
+; void useInnerTypes() {
+;   WrapTypedef::Inner v1;
+;   WrapStruct::Inner v2;
+;   WrapClass::Inner v3;
+;   WrapEnum::Inner v4;
+;   WrapUnion::Inner v5;
+; }
+
+; There should be two LF_STRUCTURE records for each wrapped type, forward decl
+; and complete type. For every inner record type, there should be two. Enums
+; don't get forward decls.
+
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapStruct`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapStruct`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapStruct::Inner`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapStruct::Inner`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapClass`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapClass`
+; CHECK-DAG: | LF_CLASS {{.*}} `WrapClass::Inner`
+; CHECK-DAG: | LF_CLASS {{.*}} `WrapClass::Inner`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapEnum`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapEnum`
+; CHECK-DAG: | LF_ENUM {{.*}} `WrapEnum::Inner`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapUnion`
+; CHECK-DAG: | LF_STRUCTURE {{.*}} `WrapUnion`
+; CHECK-DAG: | LF_UNION {{.*}} `WrapUnion::Inner`
+; CHECK-DAG: | LF_UNION {{.*}} `WrapUnion::Inner`
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.23.28106"
+
+%"struct.WrapStruct::Inner" = type { i32 }
+%"class.WrapClass::Inner" = type { i32 }
+%"union.WrapUnion::Inner" = type { i32 }
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @"?useInnerTypes@@YAXXZ"() #0 !dbg !15 {
+entry:
+  %v1 = alloca i32, align 4
+  %v2 = alloca %"struct.WrapStruct::Inner", align 4
+  %v3 = alloca %"class.WrapClass::Inner", align 4
+  %v4 = alloca i32, align 4
+  %v5 = alloca %"union.WrapUnion::Inner", align 4
+  call void @llvm.dbg.declare(metadata i32* %v1, metadata !19, metadata !DIExpression()), !dbg !23
+  call void @llvm.dbg.declare(metadata %"struct.WrapStruct::Inner"* %v2, metadata !24, metadata !DIExpression()), !dbg !30
+  call void @llvm.dbg.declare(metadata %"class.WrapClass::Inner"* %v3, metadata !31, metadata !DIExpression()), !dbg !37
+  call void @llvm.dbg.declare(metadata i32* %v4, metadata !38, metadata !DIExpression()), !dbg !39
+  call void @llvm.dbg.declare(metadata %"union.WrapUnion::Inner"* %v5, metadata !40, metadata !DIExpression()), !dbg !48
+  ret void, !dbg !49
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10, !11, !12, !13}
+!llvm.ident = !{!14}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ([e-mail]:llvm/llvm-project.git a8ccb48f697d3fbe85c593248ff1053fdf522a6e)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "t.cpp", directory: "C:\\src\\llvm-project\\build", checksumkind: CSK_MD5, checksum: "4228f12f516cd3d6dd76462be09ec111")
+!2 = !{!3, !3}
+!3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "Inner", scope: !4, file: !1, line: 11, baseType: !6, size: 32, elements: !7, identifier: ".?AW4Inner@WrapEnum@@")
+!4 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "WrapEnum", file: !1, line: 10, size: 8, flags: DIFlagTypePassByValue, elements: !5, identifier: ".?AUWrapEnum@@")
+!5 = !{!3}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{!8, !9}
+!8 = !DIEnumerator(name: "One", value: 0)
+!9 = !DIEnumerator(name: "Two", value: 1)
+!10 = !{i32 2, !"CodeView", i32 1}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 2}
+!13 = !{i32 7, !"PIC Level", i32 2}
+!14 = !{!"clang version 10.0.0 ([e-mail]:llvm/llvm-project.git a8ccb48f697d3fbe85c593248ff1053fdf522a6e)"}
+!15 = distinct !DISubprogram(name: "useInnerTypes", linkageName: "?useInnerTypes@@YAXXZ", scope: !1, file: !1, line: 16, type: !16, scopeLine: 16, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !18)
+!16 = !DISubroutineType(types: !17)
+!17 = !{null}
+!18 = !{}
+!19 = !DILocalVariable(name: "v1", scope: !15, file: !1, line: 17, type: !20)
+!20 = !DIDerivedType(tag: DW_TAG_typedef, name: "Inner", scope: !21, file: !1, line: 2, baseType: !6)
+!21 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "WrapTypedef", file: !1, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !22, identifier: ".?AUWrapTypedef@@")
+!22 = !{!20}
+!23 = !DILocation(line: 17, scope: !15)
+!24 = !DILocalVariable(name: "v2", scope: !15, file: !1, line: 18, type: !25)
+!25 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Inner", scope: !26, file: !1, line: 5, size: 32, flags: DIFlagTypePassByValue, elements: !28, identifier: ".?AUInner@WrapStruct@@")
+!26 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "WrapStruct", file: !1, line: 4, size: 8, flags: DIFlagTypePassByValue, elements: !27, identifier: ".?AUWrapStruct@@")
+!27 = !{!25}
+!28 = !{!29}
+!29 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !25, file: !1, line: 5, baseType: !6, size: 32)
+!30 = !DILocation(line: 18, scope: !15)
+!31 = !DILocalVariable(name: "v3", scope: !15, file: !1, line: 19, type: !32)
+!32 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "Inner", scope: !33, file: !1, line: 8, size: 32, flags: DIFlagTypePassByValue, elements: !35, identifier: ".?AVInner@WrapClass@@")
+!33 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "WrapClass", file: !1, line: 7, size: 8, flags: DIFlagTypePassByValue, elements: !34, identifier: ".?AUWrapClass@@")
+!34 = !{!32}
+!35 = !{!36}
+!36 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !32, file: !1, line: 8, baseType: !6, size: 32, flags: DIFlagPublic)
+!37 = !DILocation(line: 19, scope: !15)
+!38 = !DILocalVariable(name: "v4", scope: !15, file: !1, line: 20, type: !3)
+!39 = !DILocation(line: 20, scope: !15)
+!40 = !DILocalVariable(name: "v5", scope: !15, file: !1, line: 21, type: !41)
+!41 = distinct !DICompositeType(tag: DW_TAG_union_type, name: "Inner", scope: !42, file: !1, line: 14, size: 32, flags: DIFlagTypePassByValue, elements: !44, identifier: ".?ATInner@WrapUnion@@")
+!42 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "WrapUnion", file: !1, line: 13, size: 8, flags: DIFlagTypePassByValue, elements: !43, identifier: ".?AUWrapUnion@@")
+!43 = !{!41}
+!44 = !{!45, !46}
+!45 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !41, file: !1, line: 14, baseType: !6, size: 32)
+!46 = !DIDerivedType(tag: DW_TAG_member, name: "y", scope: !41, file: !1, line: 14, baseType: !47, size: 32)
+!47 = !DIBasicType(name: "float", size: 32, encoding: DW_ATE_float)
+!48 = !DILocation(line: 21, scope: !15)
+!49 = !DILocation(line: 22, scope: !15)

llvm/test/DebugInfo/COFF/unnamed-nested.ll

+; RUN: llc < %s -filetype=obj -o %t.o
+; RUN: llvm-pdbutil dump -types %t.o | FileCheck %s
+
+; C source to regenerate:
+; $ clang -g -gcodeview -S -emit-llvm t.c
+; $ cat t.c
+; struct {
+;   union {
+;     struct {};
+;   };
+; } S;
+
+; Test that this compiles without errors.
+
+; CHECK: LF_STRUCTURE{{.*}}<unnamed-tag>::<unnamed-tag>::<unnamed-tag>
+; CHECK: LF_UNION{{.*}}<unnamed-tag>::<unnamed-tag>
+; CHECK: LF_STRUCTURE{{.*}}<unnamed-tag>
+
+; ModuleID = 't.c'
+source_filename = "t.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.24.28316"
+
+%struct.anon = type { %union.anon }
+%union.anon = type { %struct.anon.0 }
+%struct.anon.0 = type { [4 x i8] }
+
+@S = dso_local global %struct.anon zeroinitializer, align 1, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!13, !14, !15, !16}
+!llvm.ident = !{!17}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "S", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 60d09bec7f8699728d38057430422d955d32a904)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "t.c", directory: "C:\\src\\llvm-build", checksumkind: CSK_MD5, checksum: "c31fe86676dd2fb56f847f926c0f2c71")
+!4 = !{}
+!5 = !{!0}
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !3, line: 1, size: 32, elements: !7)
+!7 = !{!8, !12}
+!8 = distinct !DICompositeType(tag: DW_TAG_union_type, scope: !6, file: !3, line: 2, size: 32, elements: !9)
+!9 = !{!10, !11}
+!10 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !8, file: !3, line: 3, size: 32, elements: !4)
+!11 = !DIDerivedType(tag: DW_TAG_member, scope: !8, file: !3, line: 3, baseType: !10, size: 32)
+!12 = !DIDerivedType(tag: DW_TAG_member, scope: !6, file: !3, line: 2, baseType: !8, size: 32)
+!13 = !{i32 2, !"CodeView", i32 1}
+!14 = !{i32 2, !"Debug Info Version", i32 3}
+!15 = !{i32 1, !"wchar_size", i32 2}
+!16 = !{i32 7, !"PIC Level", i32 2}
+!17 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 60d09bec7f8699728d38057430422d955d32a904)"}