[CodeView] Add support for emitting S_UDT for typedefs

[majnemer]

Summary

Emit a S_UDT record for typedefs. We still need to do something for
class types.

[rnk]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

Why do we need this condition? Don't we want to emit typdefs at global scope? I'd expect we can reference function local typedefs from outside the function, with something like return type deduction:

auto f() {
  typedef int myty;
  struct A {
    myty a;
  };
  return A();
}
decltype(f()) gv;

Maybe that's contrived, but the reliance on type emission order here feels fragile.

I was envisioning that we'd have a vector of plain DIType*'s, and we'd iterate them at end of TU and call getCompleteTypeIndex on them, which would trigger emission of all non-forward decl record types.

[majnemer]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

MSVC emits typedefs nested within functions in the same symbol section as the one for the function, I was trying to make sure that this would happen.
Consider if f where extern inline: where should the S_UDT go? I imagine MSVC will put it with the ProcSym for f which will be in a different section than the one for gv.

That being said, I think we can just never use the COMDAT section for the S_UDT records and always just stick them in the default section.
WTDYT?

[rnk]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

What if we staple UDTs into the relevant FunctionInfo? Currently FnDebugInfo is indexed by Function*, but it could easily be changed to DISubprogram since the Function points to that.

[majnemer]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

Hmm, I'm not sure how this helps. Today, we add functions to FnDebugInfo at beginFunction and we process them at endModule.

When would we be doing the stapling? We will only see the types when we are processing entities like locals. What if the local we are processing references a type that was defined in a prior FnDebugInfo entry?

[aaboud]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

Not sure how this is possible.
But assume it is, I am not sure how your code above solve the case!
You are preventing emitting S_UDT in the wrong function, but you are ending up losing S_UDT because we are not processing types before we emitting symbols, which seems as a wrong design.

[majnemer]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

780 ↗

(On Diff #60083)

Here is how it is possible:

extern inline auto f() {
  struct X { struct Y {}; };
  return X{};
}
void g() {
  decltype(f())::Y y;
}

I never claimed that my code solved the case and I'm quite aware that we are potentially dropping S_UDTs where we shouldn't. The question is: how do we fix it.

[aaboud]

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

772 ↗

(On Diff #60083)

Move this code to separate function (we will need it soon in other places).
For example:
std::string getScopedName(StringRef Name, DIScope *Scope)

780 ↗

(On Diff #60083)

We should lower types at beginModule/beginFunction/beginInstruction, and S_UDT should be part of FnDebugInfo as Reid suggested.
This means that we will need also to change the map to FnDebuginfo to be indexed by DISubprogram rather than llvm::Function.
Finally, this means that we will not need to emit debug info for functions that have no DISubprogram (any reason why we are doing that today?)

In addition, we will need one set of S_UDT for the module scope.

[rnk]

We spent some time talking about this today, and I think there's two ways to do this:

1. The hard way: compute type indices for all types used in the function's symbol substream before endFunction. Assert if someone calls getTypeIndex during .debug$S emission.
2. The easy way: Don't handle the corner cases where people reference function-internal typedefs from outside that function, and just handle typedefs at file/namespace scope and function scope. This is David's current approach.

I think we should go with approach 2, but we should also remember file/namespace scope typedefs before committing this. I think it just requires an 'else if' in the existing code and a vector on CodeViewDebug. Together that will handle 99% of all typedef usage.

[majnemer]

• Address review comments

[majnemer]

Ping.

[rnk]

lgtm

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

810 ↗

(On Diff #60188)

Can you add a FIXME or TODO about how we would handle the 'else' case? We would have to perform all type translation before beginning emission of .debug$S, and then make LocalUDTs a member of FunctionInfo.

test/DebugInfo/COFF/udts.ll

4 ↗

(On Diff #60188)

Paste the C++ source and say something about how this is testing for local UDTs? I'm guessing it's:

void f() {
  typedef int FOO;
  FOO f;
}

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

class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
   /// Map from DI metadata nodes to CodeView type indices. Primarily indexed by
   /// DIType* and DISubprogram*.
   DenseMap<const DINode *, codeview::TypeIndex> TypeIndices;
 
   /// Map from DICompositeType* to complete type index. Non-record types are
   /// always looked up in the normal TypeIndices map.
   DenseMap<const DICompositeType *, codeview::TypeIndex> CompleteTypeIndices;
 
+  const DISubprogram *CurrentSubprogram = nullptr;
+
+  // The UDTs we have seen while processing types; each entry is a pair of type
+  // index and type name.
+  std::vector<std::pair<std::string, codeview::TypeIndex>> LocalUDTs,
+      GlobalUDTs;
+
   typedef std::map<const DIFile *, std::string> FileToFilepathMapTy;
   FileToFilepathMapTy FileToFilepathMap;
   StringRef getFullFilepath(const DIFile *S);
 
   unsigned maybeRecordFile(const DIFile *F);
 
   void maybeRecordLocation(const DebugLoc &DL, const MachineFunction *MF);
 
   void clear() {
     assert(CurFn == nullptr);
     FileIdMap.clear();
     FnDebugInfo.clear();
     FileToFilepathMap.clear();
+    LocalUDTs.clear();
+    GlobalUDTs.clear();
+  }
+
+  void setCurrentSubprogram(const DISubprogram *SP) {
+    CurrentSubprogram = SP;
+    LocalUDTs.clear();
   }
 
   /// Emit the magic version number at the start of a CodeView type or symbol
   /// section. Appears at the front of every .debug$S or .debug$T section.
   void emitCodeViewMagicVersion();
 
   void emitTypeInformation();
 
   void emitInlineeLinesSubsection();
 
   void emitDebugInfoForFunction(const Function *GV, FunctionInfo &FI);
 
   void emitDebugInfoForGlobals();
 
+  void emitDebugInfoForUDTs(
+      ArrayRef<std::pair<std::string, codeview::TypeIndex>> UDTs);
+
   void emitDebugInfoForGlobal(const DIGlobalVariable *DIGV, MCSymbol *GVSym);
 
   /// Opens a subsection of the given kind in a .debug$S codeview section.
   /// Returns an end label for use with endCVSubsection when the subsection is
   /// finished.
   MCSymbol *beginCVSubsection(codeview::ModuleSubstreamKind Kind);
 
   void endCVSubsection(MCSymbol *EndLabel);

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

void CodeViewDebug::endModule() {
   emitInlineeLinesSubsection();
 
   // Emit per-function debug information.
   for (auto &P : FnDebugInfo)
     if (!P.first->isDeclarationForLinker())
       emitDebugInfoForFunction(P.first, P.second);
 
   // Emit global variable debug information.
+  setCurrentSubprogram(nullptr);
   emitDebugInfoForGlobals();
 
   // Switch back to the generic .debug$S section after potentially processing
   // comdat symbol sections.
   switchToDebugSectionForSymbol(nullptr);
 
+  // Emit UDT records for any types used by global variables.
+  if (!GlobalUDTs.empty()) {
+    MCSymbol *SymbolsEnd = beginCVSubsection(ModuleSubstreamKind::Symbols);
+    emitDebugInfoForUDTs(GlobalUDTs);
+    endCVSubsection(SymbolsEnd);
+  }
+
   // This subsection holds a file index to offset in string table table.
   OS.AddComment("File index to string table offset subsection");
   OS.EmitCVFileChecksumsDirective();
 
   // This subsection holds the string table.
   OS.AddComment("String table");
   OS.EmitCVStringTableDirective();
 
void CodeViewDebug::emitDebugInfoForFunction(const Function *GV,
   // which holds the PC to file:line table.
   const MCSymbol *Fn = Asm->getSymbol(GV);
   assert(Fn);
 
   // Switch to the to a comdat section, if appropriate.
   switchToDebugSectionForSymbol(Fn);
 
   StringRef FuncName;
-  if (auto *SP = GV->getSubprogram())
+  auto *SP = GV->getSubprogram();
+  setCurrentSubprogram(SP);
+  if (SP != nullptr)
     FuncName = SP->getDisplayName();
 
   // If our DISubprogram name is empty, use the mangled name.
   if (FuncName.empty())
     FuncName = GlobalValue::getRealLinkageName(GV->getName());
 
   // Emit a symbol subsection, required by VS2012+ to find function boundaries.
   OS.AddComment("Symbol subsection for " + Twine(FuncName));
MCSymbol *SymbolsEnd = beginCVSubsection(ModuleSubstreamKind::Symbols);
     // of their parent inline site.
     for (const DILocation *InlinedAt : FI.ChildSites) {
       auto I = FI.InlineSites.find(InlinedAt);
       assert(I != FI.InlineSites.end() &&
              "child site not in function inline site map");
       emitInlinedCallSite(FI, InlinedAt, I->second);
     }
 
+    if (SP != nullptr)
+      emitDebugInfoForUDTs(LocalUDTs);
+
     // We're done with this function.
     OS.AddComment("Record length");
     OS.EmitIntValue(0x0002, 2);
     OS.AddComment("Record kind: S_PROC_ID_END");
     OS.EmitIntValue(unsigned(SymbolKind::S_PROC_ID_END), 2);
   }
   endCVSubsection(SymbolsEnd);
 
TypeIndex CodeViewDebug::lowerType(const DIType *Ty) {
   case dwarf::DW_TAG_union_type:
     return lowerTypeUnion(cast<DICompositeType>(Ty));
   default:
     // Use the null type index.
     return TypeIndex();
   }
 }
 
+static const DISubprogram *getQualifiedNameComponents(
+    const DIScope *Scope, SmallVectorImpl<StringRef> &QualifiedNameComponents) {
+  const DISubprogram *ClosestSubprogram = nullptr;
+  while (Scope != nullptr) {
+    if (ClosestSubprogram == nullptr)
+      ClosestSubprogram = dyn_cast<DISubprogram>(Scope);
+    StringRef ScopeName = Scope->getName();
+    if (!ScopeName.empty())
+      QualifiedNameComponents.push_back(ScopeName);
+    Scope = Scope->getScope().resolve();
+  }
+  return ClosestSubprogram;
+}
+
+static std::string getQualifiedName(ArrayRef<StringRef> QualifiedNameComponents,
+                                    StringRef TypeName) {
+  std::string FullyQualifiedName;
+  for (StringRef QualifiedNameComponent : reverse(QualifiedNameComponents)) {
+    FullyQualifiedName.append(QualifiedNameComponent);
+    FullyQualifiedName.append("::");
+  }
+  FullyQualifiedName.append(TypeName);
+  return FullyQualifiedName;
+}
+
 TypeIndex CodeViewDebug::lowerTypeAlias(const DIDerivedType *Ty) {
-  // TODO: MSVC emits a S_UDT record.
   DITypeRef UnderlyingTypeRef = Ty->getBaseType();
   TypeIndex UnderlyingTypeIndex = getTypeIndex(UnderlyingTypeRef);
+  StringRef TypeName = Ty->getName();
+
+  SmallVector<StringRef, 5> QualifiedNameComponents;
+  const DISubprogram *ClosestSubprogram = getQualifiedNameComponents(
+      Ty->getScope().resolve(), QualifiedNameComponents);
+
+  if (ClosestSubprogram == nullptr) {
+    std::string FullyQualifiedName =
+        getQualifiedName(QualifiedNameComponents, TypeName);
+    GlobalUDTs.emplace_back(std::move(FullyQualifiedName), UnderlyingTypeIndex);
+  } else if (ClosestSubprogram == CurrentSubprogram) {
+    std::string FullyQualifiedName =
+        getQualifiedName(QualifiedNameComponents, TypeName);
+    LocalUDTs.emplace_back(std::move(FullyQualifiedName), UnderlyingTypeIndex);
+  }
+  // TODO: What if the ClosestSubprogram is neither null or the current
+  // subprogram?  Currently, the UDT just gets dropped on the floor.
+  //
+  // The current behavior is not desirable.  To get maximal fidelity, we would
+  // need to perform all type translation before beginning emission of .debug$S
+  // and then make LocalUDTs a member of FunctionInfo
+
   if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::Int32Long) &&
-      Ty->getName() == "HRESULT")
+      TypeName == "HRESULT")
     return TypeIndex(SimpleTypeKind::HResult);
   if (UnderlyingTypeIndex == TypeIndex(SimpleTypeKind::UInt16Short) &&
-      Ty->getName() == "wchar_t")
+      TypeName == "wchar_t")
     return TypeIndex(SimpleTypeKind::WideCharacter);
   return UnderlyingTypeIndex;
 }
 
 TypeIndex CodeViewDebug::lowerTypeArray(const DICompositeType *Ty) {
   DITypeRef ElementTypeRef = Ty->getBaseType();
   TypeIndex ElementTypeIndex = getTypeIndex(ElementTypeRef);
   // IndexType is size_t, which depends on the bitness of the target.

 }
 
 void CodeViewDebug::endCVSubsection(MCSymbol *EndLabel) {
   OS.EmitLabel(EndLabel);
   // Every subsection must be aligned to a 4-byte boundary.
   OS.EmitValueToAlignment(4);
 }
 
+void CodeViewDebug::emitDebugInfoForUDTs(
+    ArrayRef<std::pair<std::string, TypeIndex>> UDTs) {
+  for (const std::pair<std::string, codeview::TypeIndex> &UDT : UDTs) {
+    MCSymbol *UDTRecordBegin = MMI->getContext().createTempSymbol(),
+             *UDTRecordEnd = MMI->getContext().createTempSymbol();
+    OS.AddComment("Record length");
+    OS.emitAbsoluteSymbolDiff(UDTRecordEnd, UDTRecordBegin, 2);
+    OS.EmitLabel(UDTRecordBegin);
+
+    OS.AddComment("Record kind: S_UDT");
+    OS.EmitIntValue(unsigned(SymbolKind::S_UDT), 2);
+
+    OS.AddComment("Type");
+    OS.EmitIntValue(UDT.second.getIndex(), 4);
+
+    emitNullTerminatedSymbolName(OS, UDT.first);
+    OS.EmitLabel(UDTRecordEnd);
+  }
+}
+
 void CodeViewDebug::emitDebugInfoForGlobals() {
   NamedMDNode *CUs = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
   for (const MDNode *Node : CUs->operands()) {
     const auto *CU = cast<DICompileUnit>(Node);
 
     // First, emit all globals that are not in a comdat in a single symbol
     // substream. MSVC doesn't like it if the substream is empty, so only open
     // it if we have at least one global to emit.

llvm/trunk/test/DebugInfo/COFF/typedef.ll

 ; RUN: llc < %s -filetype=obj | llvm-readobj - -codeview | FileCheck %s
 
 ; CHECK: CodeViewDebugInfo [
 ; CHECK:   Subsection [
 ; CHECK:     Local {
 ; CHECK:       Type: wchar_t (0x71)
 ; CHECK:       Flags [ (0x0)
 ; CHECK:       ]
 ; CHECK:       VarName: foo
 ; CHECK:     }
+; CHECK:   Subsection [
+; CHECK:     SubSectionType: Symbols (0xF1)
+; CHECK:     SubSectionSize: 0xC
+; CHECK:     UDT {
+; CHECK:       Type: wchar_t (0x71)
+; CHECK:       UDTName: XYZ
+; CHECK:     }
+; CHECK:   ]
 
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
 target triple = "i686-pc-windows-msvc18.0.0"
 
 define void @test1() !dbg !5 {
 entry:
   %foo = alloca i16, align 2
   call void @llvm.dbg.declare(metadata i16* %foo, metadata !8, metadata !11), !dbg !12

llvm/trunk/test/DebugInfo/COFF/udts.ll

+; RUN: llc < %s -filetype=obj | llvm-readobj - -codeview | FileCheck %s
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-pc-windows-msvc18.0.0"
+
+; C++ source to regenerate:
+; $ cat t.cpp
+; void f() {
+;   typedef int FOO;
+;   FOO f;
+; }
+
+; CHECK:      ProcStart {
+; CHECK:        DisplayName: f
+; CHECK:        LinkageName: ?f@@YAXXZ
+; CHECK:      }
+; CHECK:      UDT {
+; CHECK-NEXT:   Type: int (0x74)
+; CHECK-NEXT:   UDTName: f::FOO
+; CHECK-NEXT: }
+; CHECK-NEXT: ProcEnd {
+; CHECK-NEXT: }
+
+
+; Function Attrs: nounwind
+define void @"\01?f@@YAXXZ"() #0 !dbg !6 {
+entry:
+  %f = alloca i32, align 4
+  call void @llvm.dbg.declare(metadata i32* %f, metadata !10, metadata !13), !dbg !14
+  ret void, !dbg !15
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.9.0 (trunk 272079) (llvm/trunk 271895)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "-", directory: "/usr/local/src")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version 3.9.0 (trunk 272079) (llvm/trunk 271895)"}
+!6 = distinct !DISubprogram(name: "f", linkageName: "\01?f@@YAXXZ", scope: !7, file: !7, line: 2, type: !8, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!7 = !DIFile(filename: "<stdin>", directory: "/usr/local/src")
+!8 = !DISubroutineType(types: !9)
+!9 = !{null}
+!10 = !DILocalVariable(name: "f", scope: !6, file: !7, line: 4, type: !11)
+!11 = !DIDerivedType(tag: DW_TAG_typedef, name: "FOO", scope: !6, file: !7, line: 3, baseType: !12)
+!12 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!13 = !DIExpression()
+!14 = !DILocation(line: 4, column: 5, scope: !6)
+!15 = !DILocation(line: 5, column: 1, scope: !6)