[llvm] Implement S_INLINEES debug symbol #67490

dpaoliello · 2023-09-26T21:14:59Z

The S_INLINEES debug symbol is used to record all the functions that are directly inlined within the current function (nested inlining is ignored).

This change implements support for emitting the S_INLINEES debug symbol in LLVM, and cleans up how the S_INLINEES and S_CALLEES debug symbols are dumped.

llvmbot · 2023-09-26T21:16:00Z

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-llvm-binary-utilities

Changes

The S_INLINEES debug symbol is used to record all the functions that are directly inlined within the current function (nested inlining is ignored).

This change implements support for emitting the S_INLINEES debug symbol in LLVM, and cleans up how the S_INLINEES and S_CALLEES debug symbols are dumped.

Full diff: https://github.com/llvm/llvm-project/pull/67490.diff

11 Files Affected:

(modified) llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp (+28-1)
(modified) llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h (+6)
(modified) llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp (+16-1)
(modified) llvm/test/DebugInfo/COFF/inlining-files.ll (+6)
(modified) llvm/test/DebugInfo/COFF/inlining-header.ll (+7)
(modified) llvm/test/DebugInfo/COFF/inlining-levels.ll (+6)
(modified) llvm/test/DebugInfo/COFF/inlining-padding.ll (+9)
(modified) llvm/test/DebugInfo/COFF/inlining-same-name.ll (+6)
(modified) llvm/test/DebugInfo/COFF/inlining.ll (+6)
(modified) llvm/test/tools/llvm-readobj/COFF/codeview-inlinees.test (+1-1)
(modified) llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp (+16-1)

diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index b25a38b26344138..411f7343f27bf0a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -250,7 +250,10 @@ CodeViewDebug::getInlineSite(const DILocation *InlinedAt,
         InlinedAt->getLine(), InlinedAt->getColumn(), SMLoc());
     Site->Inlinee = Inlinee;
     InlinedSubprograms.insert(Inlinee);
-    getFuncIdForSubprogram(Inlinee);
+    auto InlineeIdx = getFuncIdForSubprogram(Inlinee);
+
+    if (ParentFuncId == CurFn->FuncId)
+      CurFn->Inlinees.insert(InlineeIdx);
   }
   return *Site;
 }
@@ -1194,6 +1197,7 @@ void CodeViewDebug::emitDebugInfoForFunction(const Function *GV,
     OS.emitInt32(uint32_t(FI.FrameProcOpts));
     endSymbolRecord(FrameProcEnd);
 
+    emitInlinees(FI.Inlinees);
     emitLocalVariableList(FI, FI.Locals);
     emitGlobalVariableList(FI.Globals);
     emitLexicalBlockList(FI.ChildBlocks, FI);
@@ -3588,3 +3592,26 @@ void CodeViewDebug::emitDebugInfoForJumpTables(const FunctionInfo &FI) {
     endSymbolRecord(JumpTableEnd);
   }
 }
+
+void CodeViewDebug::emitInlinees(const SmallSet<codeview::TypeIndex, 1> &Inlinees) {
+  // Divide the list of inlinees into chunks such that each chunk fits within one record.
+  constexpr auto ChunkSize = (MaxRecordLength - sizeof(SymbolKind) - sizeof(int)) / sizeof(int);
+
+  SmallVector<TypeIndex> SortedInlinees{ Inlinees.begin(), Inlinees.end() };
+  llvm::sort(SortedInlinees);
+
+  uint64_t CurrentIndex = 0;
+  while (CurrentIndex < SortedInlinees.size()) {
+    auto Symbol = beginSymbolRecord(SymbolKind::S_INLINEES);
+    auto CurrentChunkSize = std::min(ChunkSize, SortedInlinees.size() - CurrentIndex);
+    OS.AddComment("Count");
+    OS.emitInt32(CurrentChunkSize);
+
+    const uint64_t CurrentChunkEnd = CurrentIndex + CurrentChunkSize;
+    for (; CurrentIndex < CurrentChunkEnd; ++CurrentIndex) {
+      OS.AddComment("Inlinee");
+      OS.emitInt32(SortedInlinees[CurrentIndex].getIndex());
+    }
+    endSymbolRecord(Symbol);
+  }
+}
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index eb274d25de9197a..4c03bf79d04da75 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/DbgEntityHistoryCalculator.h"
 #include "llvm/CodeGen/DebugHandlerBase.h"
@@ -158,6 +159,9 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
     /// Ordered list of top-level inlined call sites.
     SmallVector<const DILocation *, 1> ChildSites;
 
+    /// Set of all functions directly inlined into this one.
+    SmallSet<codeview::TypeIndex, 1> Inlinees;
+
     SmallVector<LocalVariable, 1> Locals;
     SmallVector<CVGlobalVariable, 1> Globals;
 
@@ -371,6 +375,8 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
   void emitInlinedCallSite(const FunctionInfo &FI, const DILocation *InlinedAt,
                            const InlineSite &Site);
 
+  void emitInlinees(const SmallSet<codeview::TypeIndex, 1> &Inlinees);
+
   using InlinedEntity = DbgValueHistoryMap::InlinedEntity;
 
   void collectGlobalVariableInfo();
diff --git a/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp b/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
index c86fb244f6887b9..197001c1bdca04d 100644
--- a/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
+++ b/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
@@ -589,7 +589,22 @@ Error CVSymbolDumperImpl::visitKnownRecord(CVSymbol &CVR,
 }
 
 Error CVSymbolDumperImpl::visitKnownRecord(CVSymbol &CVR, CallerSym &Caller) {
-  ListScope S(W, CVR.kind() == S_CALLEES ? "Callees" : "Callers");
+  llvm::StringRef ScopeName;
+  switch (CVR.kind()) {
+    case S_CALLEES:
+      ScopeName = "Callees";
+      break;
+    case S_CALLERS:
+      ScopeName = "Callers";
+      break;
+    case S_INLINEES:
+      ScopeName = "Inlinees";
+      break;
+    default:
+      return llvm::make_error<CodeViewError>(
+          "Unknown CV Record type for a CallerSym object!");
+  }
+  ListScope S(W, ScopeName);
   for (auto FuncID : Caller.Indices)
     printTypeIndex("FuncID", FuncID);
   return Error::success();
diff --git a/llvm/test/DebugInfo/COFF/inlining-files.ll b/llvm/test/DebugInfo/COFF/inlining-files.ll
index 9678219eca380ee..37edc6b8878a519 100644
--- a/llvm/test/DebugInfo/COFF/inlining-files.ll
+++ b/llvm/test/DebugInfo/COFF/inlining-files.ll
@@ -21,6 +21,12 @@
 ; OBJ:    {{.*}}Proc{{.*}}Sym {
 ; OBJ:      DisplayName: f
 ; OBJ:    }
+; OBJ:     InlineesSym {
+; OBJ-NEXT:  Kind: S_INLINEES (0x1168)
+; OBJ-NEXT:  Inlinees [
+; OBJ-NEXT:    FuncID: file_change (0x1002)
+; OBJ-NEXT:  ]
+; OBJ-NEXT:}
 ; OBJ:    InlineSiteSym {
 ; OBJ:      PtrParent: 0x0
 ; OBJ:      PtrEnd: 0x0
diff --git a/llvm/test/DebugInfo/COFF/inlining-header.ll b/llvm/test/DebugInfo/COFF/inlining-header.ll
index 0e990cbb0fccac6..9a8200ca9fc56d1 100644
--- a/llvm/test/DebugInfo/COFF/inlining-header.ll
+++ b/llvm/test/DebugInfo/COFF/inlining-header.ll
@@ -75,6 +75,13 @@
 ; OBJ:     LinkageName: _main
 ; OBJ:   }
 
+; OBJ:        InlineesSym {
+; OBJ-NEXT:     Kind: S_INLINEES (0x1168)
+; OBJ-NEXT:     Inlinees [
+; OBJ-NEXT:       FuncID: g (0x1002)
+; OBJ-NEXT:     ]
+; OBJ-NEXT:   }
+
 ; Previously, g's InlineSiteSym referenced t.h, which was wasteful.
 ; OBJ:        InlineSiteSym {
 ; OBJ:          Inlinee: g (0x1002)
diff --git a/llvm/test/DebugInfo/COFF/inlining-levels.ll b/llvm/test/DebugInfo/COFF/inlining-levels.ll
index 70195e5371f5f43..8af12512d2ab30d 100644
--- a/llvm/test/DebugInfo/COFF/inlining-levels.ll
+++ b/llvm/test/DebugInfo/COFF/inlining-levels.ll
@@ -19,6 +19,12 @@
 ; OBJ: Subsection [
 ; OBJ:   SubSectionType: Symbols (0xF1)
 ; OBJ:   {{.*}}Proc{{.*}}Sym {
+; OBJ:   InlineesSym {
+; OBJ-NEXT:  Kind: S_INLINEES (0x1168)
+; OBJ-NEXT:  Inlinees [
+; OBJ-NEXT:    FuncID: h (0x1002)
+; OBJ-NEXT:  ]
+; OBJ-NEXT:}
 ; OBJ:   InlineSiteSym {
 ; OBJ:     Inlinee: h (0x1002)
 ; OBJ:   }
diff --git a/llvm/test/DebugInfo/COFF/inlining-padding.ll b/llvm/test/DebugInfo/COFF/inlining-padding.ll
index afe899db310ccec..75254b3e9a7c604 100644
--- a/llvm/test/DebugInfo/COFF/inlining-padding.ll
+++ b/llvm/test/DebugInfo/COFF/inlining-padding.ll
@@ -33,6 +33,15 @@
 ; CHECK:    )
 ; CHECK:  }
 
+; CHECK:      InlineesSym {
+; CHECK-NEXT:   Kind: S_INLINEES (0x1168)
+; CHECK-NEXT:   Inlinees [
+; CHECK-NEXT:     FuncID: a (0x1002)
+; CHECK-NEXT:     FuncID: ab (0x1003)
+; CHECK-NEXT:     FuncID: abc (0x1004)
+; CHECK-NEXT:     FuncID: abcd (0x1005)
+; CHECK-NEXT:   ]
+
 ; C++ source used to generate the IR:
 ;
 ; extern volatile int x;
diff --git a/llvm/test/DebugInfo/COFF/inlining-same-name.ll b/llvm/test/DebugInfo/COFF/inlining-same-name.ll
index da3cabcb2bf8d82..374630b94ba4b61 100644
--- a/llvm/test/DebugInfo/COFF/inlining-same-name.ll
+++ b/llvm/test/DebugInfo/COFF/inlining-same-name.ll
@@ -18,6 +18,12 @@
 ; CHECK:     {{.*}}Proc{{.*}}Sym {
 ; CHECK:       DisplayName: main
 ; CHECK:     }
+; CHECK:     InlineesSym {
+; CHECK-NEXT:  Kind: S_INLINEES (0x1168)
+; CHECK-NEXT:  Inlinees [
+; CHECK-NEXT:    FuncID: same_name (0x1002)
+; CHECK-NEXT:  ]
+; CHECK-NEXT:}
 ; CHECK:     InlineSiteSym {
 ; CHECK:       Inlinee: same_name (0x1002)
 ; CHECK:     }
diff --git a/llvm/test/DebugInfo/COFF/inlining.ll b/llvm/test/DebugInfo/COFF/inlining.ll
index d0b03cc5b21e3d3..6953abc87813a60 100644
--- a/llvm/test/DebugInfo/COFF/inlining.ll
+++ b/llvm/test/DebugInfo/COFF/inlining.ll
@@ -166,6 +166,12 @@
 ; OBJ:     DisplayName: baz
 ; OBJ:     LinkageName: ?baz@@YAXXZ
 ; OBJ:   }
+; OBJ:   InlineesSym {
+; OBJ-NEXT:  Kind: S_INLINEES (0x1168)
+; OBJ-NEXT:  Inlinees [
+; OBJ-NEXT:    FuncID: bar (0x1002)
+; OBJ-NEXT:  ]
+; OBJ-NEXT:}
 ; OBJ:   InlineSiteSym {
 ; OBJ:     PtrParent: 0x0
 ; OBJ:     PtrEnd: 0x0
diff --git a/llvm/test/tools/llvm-readobj/COFF/codeview-inlinees.test b/llvm/test/tools/llvm-readobj/COFF/codeview-inlinees.test
index 9f818dfc289908a..f8720e35641a778 100644
--- a/llvm/test/tools/llvm-readobj/COFF/codeview-inlinees.test
+++ b/llvm/test/tools/llvm-readobj/COFF/codeview-inlinees.test
@@ -28,7 +28,7 @@ CHECK:      Kind: S_INLINESITE (0x114D)
 CHECK:      Inlinee: f (0x1003)
 CHECK:    InlineesSym {
 CHECK-NEXT:      Kind: S_INLINEES (0x1168)
-CHECK-NEXT:      Callers [
+CHECK-NEXT:      Inlinees [
 CHECK-NEXT:        FuncID: f (0x1003)
 CHECK-NEXT:        FuncID: h (0x1004)
 CHECK-NEXT:      ]
diff --git a/llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp b/llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
index 6b6ba48f9214d85..111a7d0500adfdf 100644
--- a/llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
+++ b/llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
@@ -875,9 +875,24 @@ Error MinimalSymbolDumper::visitKnownRecord(CVSymbol &CVR,
 }
 
 Error MinimalSymbolDumper::visitKnownRecord(CVSymbol &CVR, CallerSym &Caller) {
+  const char *Format;
+  switch (CVR.kind()) {
+    case S_CALLEES:
+      Format = "callee: {0}";
+      break;
+    case S_CALLERS:
+      Format = "caller: {0}";
+      break;
+    case S_INLINEES:
+      Format = "inlinee: {0}";
+      break;
+    default:
+      return llvm::make_error<CodeViewError>(
+          "Unknown CV Record type for a CallerSym object!");
+  }
   AutoIndent Indent(P, 7);
   for (const auto &I : Caller.Indices) {
-    P.formatLine("callee: {0}", idIndex(I));
+    P.formatLine(Format, idIndex(I));
   }
   return Error::success();
 }

github-actions · 2023-09-26T21:26:39Z

✅ With the latest revision this PR passed the C/C++ code formatter.

rnk

Thanks!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

tru · 2023-09-27T06:40:03Z

Thanks! Do I understand correctly that the debug information will hold more information about the inlining and be able to render more functions that are inlined in Visual Studio? I spent some time on this a while back and can't remember seeing MSVC emitting these records, but I might have missed something. Good that you figured it out!

Can you check and see that it works with llvm-debuginfo-analyzer as well? I haven't looked into how it hooks into new records like this, but it's always handy to have it available there so that it can easily be debugged.

dpaoliello · 2023-09-27T18:59:22Z

Thanks! Do I understand correctly that the debug information will hold more information about the inlining and be able to render more functions that are inlined in Visual Studio? I spent some time on this a while back and can't remember seeing MSVC emitting these records, but I might have missed something. Good that you figured it out!

As far as I know, neither Visual Studio nor WinDbg use these symbols. My motivation for adding these is for an internal tool at Microsoft that performs binary analysis that requires these symbols to exist.

Can you check and see that it works with llvm-debuginfo-analyzer as well? I haven't looked into how it hooks into new records like this, but it's always handy to have it available there so that it can easily be debugged.

Added a visitKnownRecord.

rnk

lgtm, thanks!

…bol (#67607) #67490 broke 32bit builds by having mismatched types in a call to `std::min" This change standardizes on using `size_t` to avoid the mismatch.

The `S_INLINEES` debug symbol is used to record all the functions that are directly inlined within the current function (nested inlining is ignored). This change implements support for emitting the `S_INLINEES` debug symbol in LLVM, and cleans up how the `S_INLINEES` and `S_CALLEES` debug symbols are dumped.

…bol (llvm#67607) llvm#67490 broke 32bit builds by having mismatched types in a call to `std::min" This change standardizes on using `size_t` to avoid the mismatch.

llvmbot added debuginfo platform:windows llvm:binary-utilities labels Sep 26, 2023

dpaoliello force-pushed the inlinees branch from 134c307 to 4f63460 Compare September 26, 2023 21:22

dpaoliello requested a review from rnk September 26, 2023 22:20

rnk reviewed Sep 26, 2023

View reviewed changes

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp Outdated Show resolved Hide resolved

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp Outdated Show resolved Hide resolved

tru self-requested a review September 27, 2023 06:40

Implement S_INLINEES debug symbol

655932b

dpaoliello force-pushed the inlinees branch from 4f63460 to 655932b Compare September 27, 2023 18:57

dpaoliello requested a review from rnk September 27, 2023 18:59

rnk approved these changes Sep 27, 2023

View reviewed changes

tru approved these changes Sep 27, 2023

View reviewed changes

dpaoliello merged commit 050bb26 into llvm:main Sep 27, 2023

dpaoliello deleted the inlinees branch September 27, 2023 21:06

dpaoliello mentioned this pull request Sep 27, 2023

[llvm] Fix 32bit build after change to implement S_INLINEES debug symbol #67607

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llvm] Implement S_INLINEES debug symbol #67490

[llvm] Implement S_INLINEES debug symbol #67490

Uh oh!

dpaoliello commented Sep 26, 2023

Uh oh!

llvmbot commented Sep 26, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Sep 26, 2023 •

edited

Loading

Uh oh!

rnk left a comment

Uh oh!

Uh oh!

Uh oh!

tru commented Sep 27, 2023

Uh oh!

dpaoliello commented Sep 27, 2023

Uh oh!

rnk left a comment

Uh oh!

Uh oh!

[llvm] Implement S_INLINEES debug symbol #67490

[llvm] Implement S_INLINEES debug symbol #67490

Uh oh!

Conversation

dpaoliello commented Sep 26, 2023

Uh oh!

llvmbot commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rnk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tru commented Sep 27, 2023

Uh oh!

dpaoliello commented Sep 27, 2023

Uh oh!

rnk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Sep 26, 2023 •

edited

Loading

github-actions bot commented Sep 26, 2023 •

edited

Loading