[PATCH] D144602: [BOLT] Prevent unsetting unknown control flow for split jump table

[Amir Ayupov via Phabricator via llvm-commits]

Amir created this revision.
Amir added a reviewer: bolt.
Herald added a reviewer: rafauler.
Herald added subscribers: treapster, ayermolo.
Herald added a reviewer: maksfb.
Herald added a project: All.
Amir requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

In case of a function with unknown control flow but with a single jump
table and a single jump table site, we attempt to match the jump table
and a site and update block successors using jump table targets.
Restrict this behavior for split jump tables which have targets in a
fragment function.

Fixes https://github.com/llvm/llvm-project/issues/60795.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144602

Files:
  bolt/lib/Core/BinaryFunction.cpp
  bolt/test/X86/split-func-jump-table-unknown.s


Index: bolt/test/X86/split-func-jump-table-unknown.s
===================================================================
--- /dev/null
+++ bolt/test/X86/split-func-jump-table-unknown.s
@@ -0,0 +1,56 @@
+# This reproduces a bug with converting an unknown control flow jump table with
+# entries pointing to code in function and its cold fragment.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: llvm-strip --strip-unneeded %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.out --lite=0 -v=1 --strict=1 2>&1 | FileCheck %s
+
+# CHECK: BOLT-INFO: marking main.cold.1 as a fragment of main
+  .text
+  .globl main
+  .type main, %function
+  .p2align 2
+main:
+LBB0:
+  leaq JUMP_TABLE(%rip), %r8
+  andl $0xf, %ecx
+  cmpb $0x4, %cl
+  # exit through abort in main.cold.1, registers cold fragment the regular way
+  ja main.cold.1
+
+# jump table dispatch, jumping to label indexed by val in %ecx
+LBB1:
+  movzbl %cl, %ecx
+  movslq (%r8,%rcx,4), %rax
+  addq %rax, %r8
+  jmpq *%r8
+
+LBB2:
+  xorq %rax, %rax
+LBB3:
+  addq $0x8, %rsp
+  ret
+.size main, .-main
+
+  .globl main.cold.1
+  .type main.cold.1, %function
+  .p2align 2
+main.cold.1:
+  # load bearing nop: pad LBB4 so that it can't be treated
+  # as __builtin_unreachable by analyzeJumpTable
+  nop
+LBB4:
+  callq abort
+.size main.cold.1, .-main.cold.1
+
+  .rodata
+# jmp table, entries must be R_X86_64_PC32 relocs
+  .globl JUMP_TABLE
+JUMP_TABLE:
+  .long LBB2-JUMP_TABLE
+  .long LBB3-JUMP_TABLE
+  .long LBB4-JUMP_TABLE
+  .long LBB3-JUMP_TABLE
Index: bolt/lib/Core/BinaryFunction.cpp
===================================================================
--- bolt/lib/Core/BinaryFunction.cpp
+++ bolt/lib/Core/BinaryFunction.cpp
@@ -1770,6 +1770,8 @@
 bool BinaryFunction::postProcessIndirectBranches(
     MCPlusBuilder::AllocatorIdTy AllocId) {
   auto addUnknownControlFlow = [&](BinaryBasicBlock &BB) {
+    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: adding unknown control flow in " << *this
+                      << " for " << BB.getName() << "\n");
     HasUnknownControlFlow = true;
     BB.removeAllSuccessors();
     for (uint64_t PossibleDestination : ExternallyReferencedOffsets)
@@ -1877,7 +1879,11 @@
   // references, then we should be able to derive the jump table even if we
   // fail to match the pattern.
   if (HasUnknownControlFlow && NumIndirectJumps == 1 &&
-      JumpTables.size() == 1 && LastIndirectJump) {
+      JumpTables.size() == 1 && LastIndirectJump &&
+      !BC.getJumpTableContainingAddress(LastJT)->IsSplit) {
+    LLVM_DEBUG(
+        dbgs() << "BOLT-DEBUG: " << *this
+               << " has one JT, one JT site, unsetting unknown control flow\n");
     BC.MIB->setJumpTable(*LastIndirectJump, LastJT, LastJTIndexReg, AllocId);
     HasUnknownControlFlow = false;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144602.499669.patch
Type: text/x-patch
Size: 2863 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230223/b76878be/attachment.bin>