[llvm] [BOLT] Skip function instead of aborting on jump table analysis failure (PR #206742)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: nitro (NitroAshi)

<details>
<summary>Changes</summary>

`analyzeJumpTable()` runs twice on a candidate jump table: once from
`analyzeMemoryAt` while the referencing function is being disassembled, and
again from `populateJumpTables()` after disassembly. The second run is stricter,
because the check that an entry points at an instruction
(`getInstructionAtOffset`) only fires once the target function reaches the
`Disassembled` state. A table can therefore be accepted the first time and
rejected the second -- for instance when an entry lands at an address with no
instruction (a symbol whose declared size is larger than its disassembled
extent). `populateJumpTables()` reached `llvm_unreachable("jump table heuristic
failure")` and aborted in that case.

Handle it the way BOLT already handles indirect branches it can't analyze: drop
the table and skip the owning function(s) via `addFragmentsToSkip()`. The
`JumpTable` is deliberately left in the maps so it is still freed by
`~BinaryContext` (an earlier attempt at this, 52cd00ca, was reverted in 468d4f6d
because it erased the table without deallocating it).

New test `jump-table-failed-reanalysis.s` builds such a table; before this change
BOLT aborts, after it the function is skipped and the rest of the binary is
processed normally.


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


2 Files Affected:

- (modified) bolt/lib/Core/BinaryContext.cpp (+9-1) 
- (added) bolt/test/X86/jump-table-failed-reanalysis.s (+51) 


``````````diff
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index eb9caca3ea16e..88cca51b31107 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -781,6 +781,9 @@ void BinaryContext::populateJumpTables() {
         analyzeJumpTable(JT->getAddress(), JT->Type, *(JT->Parents[0]),
                          NextJTAddress, &JT->EntriesAsAddress, &JT->IsSplit);
     if (!Success) {
+      // Re-analysis here is stricter than during disassembly (the referenced
+      // function is now disassembled), so it may fail on a table we accepted
+      // earlier. Skip the function(s) instead of aborting.
       LLVM_DEBUG({
         dbgs() << "failed to analyze ";
         JT->print(dbgs());
@@ -789,7 +792,12 @@ void BinaryContext::populateJumpTables() {
           NextJTI->second->print(dbgs());
         }
       });
-      llvm_unreachable("jump table heuristic failure");
+      JT->EntriesAsAddress.clear();
+      JT->IsSplit = false;
+      // Keep JT in the map so it is still freed by ~BinaryContext.
+      for (BinaryFunction *Frag : JT->Parents)
+        addFragmentsToSkip(Frag);
+      continue;
     }
     for (BinaryFunction *Frag : JT->Parents) {
       if (JT->IsSplit)
diff --git a/bolt/test/X86/jump-table-failed-reanalysis.s b/bolt/test/X86/jump-table-failed-reanalysis.s
new file mode 100644
index 0000000000000..8996cb9cd99ff
--- /dev/null
+++ b/bolt/test/X86/jump-table-failed-reanalysis.s
@@ -0,0 +1,51 @@
+## A jump table entry whose target is not an instruction boundary is accepted
+## while the function is being disassembled but rejected when the table is
+## re-analyzed afterwards. Check that BOLT skips the function instead of aborting
+## with "jump table heuristic failure".
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -no-pie -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.out 2>&1 | FileCheck %s
+
+# CHECK-NOT: jump table heuristic failure
+# CHECK: BOLT-WARNING: skipped {{.*}} function{{.*}} due to cold fragments
+
+	.text
+	.globl	func
+	.type	func, @function
+func:
+	.cfi_startproc
+	jmp	*JT(,%rdi,8)
+.Lc0:
+	movl	$10, %eax          # 5-byte movl (b8 0a 00 00 00); .Lc0+1 is mid-instruction
+	ret
+.Lc1:
+	movl	$11, %eax
+	ret
+.Lc2:
+	movl	$12, %eax
+	ret
+	.cfi_endproc
+.Lfunc_end:
+	.size	func, .Lfunc_end - func
+
+	.globl	main
+	.type	main, @function
+main:
+	.cfi_startproc
+	xorl	%edi, %edi
+	call	func
+	xorl	%eax, %eax
+	ret
+	.cfi_endproc
+	.size	main, .-main
+
+	.section	.rodata
+	.align	8
+JT:
+	.quad	.Lc0               # entry 0: real instruction boundary
+	.quad	.Lc0 + 1           # entry 1: MID-INSTRUCTION - no instruction at this offset
+	.quad	.Lc1
+	.quad	.Lc2

``````````

</details>


https://github.com/llvm/llvm-project/pull/206742