Add CodeView S_LABEL32 symbols for jump table targets (for Windows debugging) #146121
Conversation
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-debuginfo Author: None (sivadeilra) ChangesThis PR provides more information to debuggers and analysis tools on Windows. It adds More importantly, this PR allows Windows analysis tools, such as those that generate hot-patches for the Windows kernel, to use these labels to distinguish code basic blocks from data blocks. Microsoft's analysis tools (similar to Bolt) rely on being able to identify all code blocks, so that the tools can traverse all instructions and verify that important requirements for hot-patching are met. This PR has no effect on code generation. It only affects the CodeView symbols that are emitted into OBJ files, which the linker then repackages into PDB files. Full diff: https://github.com/llvm/llvm-project/pull/146121.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 5e1b313b4d2fa..91cf7c17f7cb0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -3566,15 +3566,38 @@ void CodeViewDebug::collectDebugInfoForJumpTables(const MachineFunction *MF,
break;
}
- CurFn->JumpTables.push_back(
- {EntrySize, Base, BaseOffset, Branch,
- MF->getJTISymbol(JumpTableIndex, MMI->getContext()),
- JTI.getJumpTables()[JumpTableIndex].MBBs.size()});
+ const MachineJumpTableEntry &JTE = JTI.getJumpTables()[JumpTableIndex];
+
+ JumpTableInfo CVJTI{EntrySize,
+ Base,
+ BaseOffset,
+ Branch,
+ MF->getJTISymbol(JumpTableIndex, MMI->getContext()),
+ JTE.MBBs.size()};
+
+ for (const auto &MBB : JTE.MBBs) {
+ CVJTI.Cases.push_back(MBB->getSymbol());
+ }
+
+ CurFn->JumpTables.push_back(std::move(CVJTI));
});
}
void CodeViewDebug::emitDebugInfoForJumpTables(const FunctionInfo &FI) {
- for (auto JumpTable : FI.JumpTables) {
+ // Emit S_LABEL32 records for each jump target
+ for (const auto &JumpTable : FI.JumpTables) {
+ for (const auto &CaseSym : JumpTable.Cases) {
+ MCSymbol *LabelEnd = beginSymbolRecord(SymbolKind::S_LABEL32);
+ OS.AddComment("Offset and segment");
+ OS.emitCOFFSecRel32(CaseSym, 0);
+ OS.AddComment("Flags");
+ OS.emitInt8(0);
+ emitNullTerminatedSymbolName(OS, CaseSym->getName());
+ endSymbolRecord(LabelEnd);
+ }
+ }
+
+ for (const auto &JumpTable : FI.JumpTables) {
MCSymbol *JumpTableEnd = beginSymbolRecord(SymbolKind::S_ARMSWITCHTABLE);
if (JumpTable.Base) {
OS.AddComment("Base offset");
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index c862802d835d7..c2b878e52e1c3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -146,6 +146,7 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
const MCSymbol *Branch;
const MCSymbol *Table;
size_t TableSize;
+ std::vector<const MCSymbol *> Cases;
};
// For each function, store a vector of labels to its instructions, as well as
diff --git a/llvm/test/DebugInfo/COFF/jump-table.ll b/llvm/test/DebugInfo/COFF/jump-table.ll
index 3eda2438ea88a..be1de2be55788 100644
--- a/llvm/test/DebugInfo/COFF/jump-table.ll
+++ b/llvm/test/DebugInfo/COFF/jump-table.ll
@@ -118,6 +118,15 @@
; CV: GlobalProcIdSym {
; CV: DisplayName: func
; CV-NOT: GlobalProcIdSym
+; CV: LabelSym {
+; CV-NEXT: Kind: S_LABEL32 (0x1105)
+; CV-NEXT: CodeOffset: 0xC0
+; CV-NEXT: Segment: 0x0
+; CV-NEXT: Flags: 0x0
+; CV-NEXT: Flags [ (0x0)
+; CV-NEXT: ]
+; CV-NEXT: DisplayName:
+; CV-NEXT: }
; CV: JumpTableSym {
; CV-NEXT: Kind: S_ARMSWITCHTABLE (0x1159)
; CV-NEXT: BaseOffset: 0x0
|
There was a problem hiding this comment.
We usually don't add blank lines after variable declarations.
There was a problem hiding this comment.
omit braces for singe-line simple body https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
PR feedback, style
sivadeilra
force-pushed
the
user/ardavis/jump-table-labels
branch
from
f3881a5 to
1beede6
Compare
|
@MaskRay Any more feedback on this PR? |
|
Post-build jobs show a warning in CodeViewDebug.cpp. It's a one-line fix; PR for fix in just a few minutes. |
llvm#146121 caused a build warning in a post-CI job. The fix should be trivial; provide an extra initializer element when initializing a class, for the additional field.
llvm#146121 caused a build warning in a post-CI job. The fix should be trivial; provide an extra initializer element when initializing a class, for the additional field.
4 participants
This PR provides more information to debuggers and analysis tools on Windows. It adds
S_LABEL32symbols for each target BB of each jump table. This allows debuggers to insert symbolic labels when disassembling code.S_LABEL32symbol records indicate that a location is definitely code, and can optionally associate a string label with the code location. BBs generated for jump tables may or may not have string labels, so it is acceptable for the "name" field withinS_LABEL32symbols to be an empty string.More importantly, this PR allows Windows analysis tools, such as those that generate hot-patches for the Windows kernel, to use these labels to distinguish code basic blocks from data blocks. Microsoft's analysis tools (similar to Bolt) rely on being able to identify all code blocks, so that the tools can traverse all instructions and verify that important requirements for hot-patching are met.
This PR has no effect on code generation. It only affects the CodeView symbols that are emitted into OBJ files, which the linker then repackages into PDB files.