[BOLT] Fix creation of invalid CFG in presence of dead code

When there is a direct jump right after an indirect one, in
the absence of code jumpting to this direct jump, this is obviously
dead code. However, BOLT was failing to recognize that by mistakenly
placing both jmp instructions in the same basic block, and creating
wrong successor edges. Fix that, so we can safely run UCE on
that. This bug also causes validateCFG to fail and BOLT to crash if it
is running ICP on that function.

Reviewed By: #bolt, Amir

Differential Revision: https://reviews.llvm.org/D148055

2 files changed, 67 insertions, 0 deletions

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 14a10be62d20..89ba3627cdd7 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -2038,6 +2038,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       MCInst *PrevInstr = PrevBB->getLastNonPseudoInstr();
       assert(PrevInstr && "no previous instruction for a fall through");
       if (MIB->isUnconditionalBranch(Instr) &&
+          !MIB->isIndirectBranch(*PrevInstr) &&
           !MIB->isUnconditionalBranch(*PrevInstr) &&
           !MIB->getConditionalTailCall(*PrevInstr) &&
           !MIB->isReturn(*PrevInstr)) {
diff --git a/bolt/test/X86/unreachable-jmp.s b/bolt/test/X86/unreachable-jmp.s
new file mode 100644
index 000000000000..201e99990736
--- /dev/null
+++ b/bolt/test/X86/unreachable-jmp.s
@@ -0,0 +1,66 @@
+# This checks that we don't create an invalid CFG when there is an
+# unreachable direct jump right after an indirect one.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: link_fdata %s %t.o %t.fdata
+# RUN: llvm-strip --strip-unneeded %t.o
+# RUN: %clang %cflags -no-pie %t.o -o %t.exe -Wl,-q -nostdlib
+# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata \
+# RUN:     --eliminate-unreachable --print-cfg | FileCheck %s
+
+  .globl _start
+  .type _start, %function
+_start:
+  .cfi_startproc
+# FDATA: 0 [unknown] 0 1 _start 0 0 1
+  push  %rbp
+  mov   %rsp, %rbp
+  push  %rbx
+  push  %r14
+  subq  $0x20, %rsp
+  movq  %rdi, %rcx
+b:
+  jmpq  *JUMP_TABLE(,%rcx,8)
+# FDATA: 1 _start #b# 1 _start #hotpath# 0 20
+# Unreachable direct jump here. Our CFG should still make sense and properly
+# place this instruction in a new basic block.
+  jmp  .lbb2
+.lbb1:  je  .lexit
+.lbb2:
+  xorq  %rax, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+hotpath:
+  movq  $2, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+.lexit:
+  movq  $1, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+  .cfi_endproc
+  .size _start, .-_start
+
+  .rodata
+  .globl JUMP_TABLE
+JUMP_TABLE:
+  .quad .lbb1
+  .quad .lbb2
+  .quad hotpath
+
+# No basic blocks above should have 4 successors! That is a bug.
+# CHECK-NOT:   Successors: {{.*}} (mispreds: 0, count: 20), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0)
+# Check successful removal of stray direct jmp
+#  CHECK:  UCE removed 1 block