From 468d4f6d188d58c07f531279e2bce2f7c13093ab Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov@fb.com>
Date: Sat, 30 Jul 2022 10:35:10 -0700
Subject: [PATCH] Revert "[BOLT] Ignore functions accessing false positive jump
 tables"

This diff uncovers an ASAN leak in getOrCreateJumpTable:
```
Indirect leak of 264 byte(s) in 1 object(s) allocated from:
    #1 0x4f6e48c in llvm::bolt::BinaryContext::getOrCreateJumpTable ...
```
The removal of an assertion needs to be accompanied by proper deallocation of
a `JumpTable` object for which `analyzeJumpTable` was unsuccessful.

This reverts commit 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e.
---
 bolt/lib/Core/BinaryContext.cpp | 23 +---------------
 bolt/test/X86/fake_jump_table.s | 49 ---------------------------------
 2 files changed, 1 insertion(+), 71 deletions(-)
 delete mode 100644 bolt/test/X86/fake_jump_table.s

diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 24d1947b55a32..93316431439a1 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -622,10 +622,6 @@ bool BinaryContext::analyzeJumpTable(
 void BinaryContext::populateJumpTables() {
   LLVM_DEBUG(dbgs() << "DataPCRelocations: " << DataPCRelocations.size()
                     << '\n');
-
-  // Collect jump tables that pass the analyzeJumpTable's first check,
-  // but fail the analyzeJumpTable's second check
-  SmallVector<JumpTable *, 1> AbortedJTs;
   for (auto JTI = JumpTables.begin(), JTE = JumpTables.end(); JTI != JTE;
        ++JTI) {
     JumpTable *JT = JTI->second;
@@ -644,13 +640,6 @@ void BinaryContext::populateJumpTables() {
     const bool Success =
         analyzeJumpTable(JT->getAddress(), JT->Type, *(JT->Parents[0]),
                          NextJTAddress, &JT->EntriesAsAddress);
-    // !Success means a false positive from earlier analysis run due to
-    // different context. A possible culprit is instruction bounds check.
-    // Previous run happens during disassembly. If the target function
-    // is not disassembled, the check will be skipped, leading to a false
-    // positive
-    //
-    // Solution: Ignore fragments accessing JT that fails the check
     if (!Success) {
       LLVM_DEBUG(ListSeparator LS;
                  dbgs() << "failed to analyze jump table in function ";
@@ -670,8 +659,7 @@ void BinaryContext::populateJumpTables() {
                    dbgs() << "\n";);
         NextJTI->second->print(dbgs());
       }
-      AbortedJTs.push_back(JT);
-      continue;
+      llvm_unreachable("jump table heuristic failure");
     }
     for (BinaryFunction *Frag : JT->Parents) {
       for (uint64_t EntryAddress : JT->EntriesAsAddress)
@@ -701,15 +689,6 @@ void BinaryContext::populateJumpTables() {
         addFragmentsToSkip(Frag);
   }
 
-  // Ignore fragments accessing JT that fails analyzeJumpTable check
-  for (JumpTable *JT : AbortedJTs) {
-    for (BinaryFunction *Frag : JT->Parents) {
-      Frag->setIgnored();
-      Frag->JumpTables.erase(Frag->JumpTables.find(JT->getAddress()));
-    }
-    JumpTables.erase(JumpTables.find(JT->getAddress()));
-  }
-
   if (opts::StrictMode && DataPCRelocations.size()) {
     LLVM_DEBUG({
       dbgs() << DataPCRelocations.size()
diff --git a/bolt/test/X86/fake_jump_table.s b/bolt/test/X86/fake_jump_table.s
deleted file mode 100644
index 497b7a1906b9d..0000000000000
--- a/bolt/test/X86/fake_jump_table.s
+++ /dev/null
@@ -1,49 +0,0 @@
-# Currently disassembly is not decoupled from branch target analysis.
-# This causes a few checks related to availability of target insn to
-# fail for stripped binaries:
-#   (a) analyzeJumpTable
-#   (b) postProcessEntryPoints
-# This test checks if BOLT can safely support instruction bounds check
-# for cross-function targets.
-
-# 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 -Wl,-q
-# RUN: llvm-bolt %t.exe -o %t.out -v=1 -print-cfg
-
-  .text
-  .globl main
-  .type main, %function
-  .p2align 2
-main:
-LBB0:
-  .cfi_startproc
-  andl $0xf, %ecx
-  cmpb $0x4, %cl
-  ja .main.cold.1
-LBB1:
-  leaq FAKE_JUMP_TABLE(%rip), %r8
-  cmpq %r8, %r9
-LBB2:
-  xorq %rax, %rax
-  ret
-  .cfi_endproc
-.size main, .-main
-
-  .globl main.cold.1
-  .type main.cold.1, %function
-  .p2align 2
-main.cold.1:
-  .cfi_startproc
-  nop
-LBB3:
-  callq abort
-  .cfi_endproc
-.size main.cold.1, .-main.cold.1
-
-  .rodata
-  .globl FAKE_JUMP_TABLE
-FAKE_JUMP_TABLE:
-  .long LBB2-FAKE_JUMP_TABLE
-  .long LBB3-FAKE_JUMP_TABLE+0x1