From 3ab2929b36cc2d3b6b7d859da2e9ab44101ea09e Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks@fb.com>
Date: Wed, 20 Jun 2018 21:43:22 -0700
Subject: [PATCH] [BOLT] Fix support for PIC jump tables

Summary:
BOLT heuristics failed to work if false PIC jump table entries were
accepted when they were pointing inside a function, but not at
an instruction boundary.

This fix checks if the destination falls at instruction boundary, and
if it does not, it truncates the jump table. This, of course, still does not
guarantee that the entry corresponds to a real destination, and we can
have "false positive" entry(ies). However, it shouldn't affect
correctness of the function, but the CFG may have edges that are never
taken. We may update an incorrect jump table entry, corresponding to an
unrelated data, and for that reason we force moving of jump tables if a
PIC jump table was detected.

(cherry picked from FBD8559588)
---
 bolt/src/BinaryFunction.cpp | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/bolt/src/BinaryFunction.cpp b/bolt/src/BinaryFunction.cpp
index db441a2301e8..4d949ff092ae 100644
--- a/bolt/src/BinaryFunction.cpp
+++ b/bolt/src/BinaryFunction.cpp
@@ -854,12 +854,13 @@ IndirectBranchType BinaryFunction::processIndirectBranch(MCInst &Instruction,
         Type = IndirectBranchType::POSSIBLE_JUMP_TABLE;
       continue;
     }
-    // Potentially a switch table can contain  __builtin_unreachable() entry
+    // Potentially a switch table can contain __builtin_unreachable() entry
     // pointing just right after the function. In this case we have to check
     // another entry. Otherwise the entry is outside of this function scope
     // and it's not a switch table.
     if (Value == getAddress() + getSize()) {
-      JTOffsetCandidates.push_back(Value - getAddress());
+      JTOffsetCandidates.push_back(getSize());
+      IgnoredBranches.emplace_back(Offset, getSize());
     } else {
       break;
     }
@@ -1388,16 +1389,32 @@ add_instruction:
     addInstruction(Offset, std::move(Instruction));
   }
 
-  postProcessJumpTables();
-
   updateState(State::Disassembled);
+
+  postProcessJumpTables();
 }
 
 void BinaryFunction::postProcessJumpTables() {
   // Create labels for all entries.
   for (auto &JTI : JumpTables) {
     auto &JT = *JTI.second;
-    for (auto Offset : JT.OffsetEntries) {
+    if (JT.Type == JumpTable::JTT_PIC && opts::JumpTables == JTS_BASIC) {
+      opts::JumpTables = JTS_MOVE;
+      outs() << "BOLT-INFO: forcing -jump-tables=move as PIC jump table was "
+                "detected\n";
+    }
+    for (unsigned I = 0; I < JT.OffsetEntries.size(); ++I) {
+      auto Offset = JT.OffsetEntries[I];
+      if (Offset != getSize() && !getInstructionAtOffset(Offset)) {
+        DEBUG(dbgs() << "BOLT-DEBUG: truncating jump table " << JT.getName()
+                     << " at index " << I << " containing offset 0x"
+                     << Twine::utohexstr(Offset) << '\n');
+        assert(I > 1 && "jump table with a size smaller than 1 detected");
+        assert(JT.Type == JumpTable::JTT_PIC &&
+               "unexpected truncation of non-PIC jump table");
+        JT.OffsetEntries.resize(I);
+        break;
+      }
       auto *Label = getOrCreateLocalLabel(getAddress() + Offset,
                                           /*CreatePastEnd*/ true);
       JT.Entries.push_back(Label);