[RFC] Handle qualified unnamed types in CodeView printer

[smeenai]

Summary

When attempting to compile the following Objective-C++ code with
CodeView debug info:

void (^b)(void) = []() {};

The generated debug metadata contains a structure like the following:

!43 = !DICompositeType(tag: DW_TAG_structure_type, name: "__block_literal_1", scope: !6, file: !6, line: 1, size: 168, elements: !44)
!44 = !{!45, !46, !47, !48, !49, !52}
...
!52 = !DIDerivedType(tag: DW_TAG_member, scope: !6, file: !6, line: 1, baseType: !53, size: 8, offset: 160, flags: DIFlagPublic)
!53 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !54)
!54 = !DICompositeType(tag: DW_TAG_class_type, file: !6, line: 1, flags: DIFlagFwdDecl)

Note that the member node (!52) is unnamed, but rather than pointing to
a DICompositeType directly, it points to a DIDerivedType with tag
DW_TAG_const_type, which then points to the DICompositeType. However,
the CodeView assembly printer currently assumes that the base type for
an unnamed member will always be a DICompositeType, and attempts to
perform that cast, which triggers an assertion failure, since in this
case the base type is actually a DIDerivedType, not a DICompositeType
(and we would have to get the base type of the DIDerivedType to reach
the DICompositeType). I think the debug metadata being generated by the
frontend is correct (or at least plausible), and the CodeView printer
needs to handle this case.

This patch teaches the CodeView printer to unwrap any qualifier types.
The qualifiers are just dropped for now. Ideally, they would be applied
to the added indirect members instead, but this occurs infrequently
enough that adding the logic to handle the qualifiers correctly isn't
worth it for now. A FIXME is added to note this.

Additionally, Reid pointed out that the underlying assumption that an
unnamed member must be a composite type is itself incorrect and may not
hold for all frontends. Therefore, after all qualifiers have been
stripped, check if the resulting type is in fact a DICompositeType and
just return if it isn't, rather than assuming the type and crashing if
that assumption is violated.

[rnk]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1817–1818 ↗

(On Diff #136031)

This seems like a flawed assumption. Other frontends might want to emit unnamed fields that aren't nested structs. I'd rather rewrite this logic so that it's a pattern match of an unnamed DICompositeType that is possibly wrapped in qualifier types. Basically, I'd just make the DICompositeType cast below a dyn_cast, and if it fails, drop the unnamed member. I have a feeling that VS and windbg will not cope with unnamed members.

[smeenai]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1817–1818 ↗

(On Diff #136031)

Thanks. Is there a better way to do that pattern match than what I have here? (In particular, should I be concerned about preserving the qualifiers in the nested types, as opposed to just dropping them, which I'm doing right now?)

[rnk]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1817–1818 ↗

(On Diff #136031)

I suppose the correct thing to do is to mark all the indirect members inside the anonymous struct here with the qualifiers on the outer struct. It would look kind of like this:

struct Outer {
  const struct {
    int inner1;
    int inner2;
  };
};
--> codeview
struct Outer {
  const int inner1;
  const int inner2;
};

That could be implemented by adding qualifiers to ClassInfo::MemberInfo, and then copying some of the logic from lowerTypeModifier to generate the right LF_MODIFIER records for it. Alternatively, we could synthesize new DIDerivedType metadata and then call getTypeIndex on it. It seems a little heroic, though, just to deal with a format limitation.

lowerTypeModifier is also about to get more complicated in D43060.

In conclusion, I'd drop them for now. Add a FIXME, maybe?

[smeenai]

Reid's comments

[rnk]

lgtm

llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 }
 
 void CodeViewDebug::collectMemberInfo(ClassInfo &Info,
                                       const DIDerivedType *DDTy) {
   if (!DDTy->getName().empty()) {
     Info.Members.push_back({DDTy, 0});
     return;
   }
-  // An unnamed member must represent a nested struct or union. Add all the
-  // indirect fields to the current record.
+
+  // An unnamed member may represent a nested struct or union. Attempt to
+  // interpret the unnamed member as a DICompositeType possibly wrapped in
+  // qualifier types. Add all the indirect fields to the current record if that
+  // succeeds, and drop the member if that fails.
   assert((DDTy->getOffsetInBits() % 8) == 0 && "Unnamed bitfield member!");
   uint64_t Offset = DDTy->getOffsetInBits();
   const DIType *Ty = DDTy->getBaseType().resolve();
-  const DICompositeType *DCTy = cast<DICompositeType>(Ty);
+  bool FullyResolved = false;
+  while (!FullyResolved) {
+    switch (Ty->getTag()) {
+    case dwarf::DW_TAG_const_type:
+    case dwarf::DW_TAG_volatile_type:
+      // FIXME: we should apply the qualifier types to the indirect fields
+      // rather than dropping them.
+      Ty = cast<DIDerivedType>(Ty)->getBaseType().resolve();
+      break;
+    default:
+      FullyResolved = true;
+      break;
+    }
+  }
+
+  const DICompositeType *DCTy = dyn_cast<DICompositeType>(Ty);
+  if (!DCTy)
+    return;
+
   ClassInfo NestedInfo = collectClassInfo(DCTy);
   for (const ClassInfo::MemberInfo &IndirectField : NestedInfo.Members)
     Info.Members.push_back(
         {IndirectField.MemberTypeNode, IndirectField.BaseOffset + Offset});
 }
 
 ClassInfo CodeViewDebug::collectClassInfo(const DICompositeType *Ty) {
   ClassInfo Info;

llvm/trunk/test/DebugInfo/COFF/const-unnamed-member.ll

+; RUN: llc -filetype=obj < %s | llvm-readobj -codeview - | FileCheck %s
+
+; Objective-C++ source demonstrating the issue:
+; void (^b)(void) = []() {};
+
+; C++ source derived and modified from:
+; struct S {
+;   struct {
+;     int a;
+;   };
+; } s;
+
+; CHECK: CodeViewTypes [
+; CHECK:  FieldList ([[S_fl:.*]]) {
+; CHECK:    TypeLeafKind: LF_FIELDLIST (0x1203)
+; CHECK:    DataMember {
+; CHECK:      Type: int (0x74)
+; CHECK:      FieldOffset: 0x0
+; CHECK:      Name: a
+; CHECK:    }
+; CHECK:  }
+; CHECK:  Struct ({{.*}}) {
+; CHECK:    TypeLeafKind: LF_STRUCTURE (0x1505)
+; CHECK:    MemberCount: 2
+; CHECK:    FieldList: <field list> ([[S_fl]])
+; CHECK:    SizeOf: 4
+; CHECK:    Name: S
+; CHECK:    LinkageName: .?AUS@@
+; CHECK:  }
+
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686--windows-msvc19.11.0"
+
+%struct.S = type { %struct.anon }
+%struct.anon = type { i32 }
+
+@"\01?s@@3US@@A" = dso_local global %struct.S zeroinitializer, align 4, !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", linkageName: "\01?s@@3US@@A", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 7.0.0 (trunk 325940) (llvm/trunk 325939)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "/tmp/ugh.cpp", directory: "/home/smeenai/llvm/build/llvm/Debug", checksumkind: CSK_MD5, checksum: "8256b51d95df0b5e42b848a3afe9cbda")
+!4 = !{}
+!5 = !{!0}
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S", file: !3, line: 1, size: 32, flags: DIFlagTypePassByValue, elements: !7, identifier: ".?AUS@@")
+!7 = !{!8, !12}
+!8 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !6, file: !3, line: 2, size: 32, flags: DIFlagTypePassByValue, elements: !9, identifier: ".?AU<unnamed-type-$S1>@S@@")
+!9 = !{!10}
+!10 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !8, file: !3, line: 3, baseType: !11, size: 32)
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DIDerivedType(tag: DW_TAG_member, scope: !6, file: !3, line: 2, baseType: !18, size: 32) ; !8 changed to !18
+!13 = !{i32 1, !"NumRegisterParameters", i32 0}
+!14 = !{i32 2, !"CodeView", i32 1}
+!15 = !{i32 2, !"Debug Info Version", i32 3}
+!16 = !{i32 1, !"wchar_size", i32 2}
+!17 = !{!"clang version 7.0.0 (trunk 325940) (llvm/trunk 325939)"}
+!18 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8) ; added manually