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

[rafauler]

Summary

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.

[rafauler]

Make test case run UCE to check that we are really removing the
unnecessary direct jump instruction

[Amir]

Thanks for fixing it!

bolt/lib/Core/BinaryFunction.cpp

for (auto I = Instructions.begin(), E = Instructions.end(); I != E; ++I) {
     if (!InsertBB) {
       // It must be a fallthrough or unreachable code. Create a new block unless
       // we see an unconditional branch following a conditional one. The latter
       // should not be a conditional tail call.
       assert(PrevBB && "no previous basic block for a fall through");
       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)) {
         // Temporarily restore inserter basic block.
         InsertBB = PrevBB;
       } else {
         MCSymbol *Label;
         {

bolt/test/X86/unreachable-jmp.s

+# 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