[PATCH] D130431: [BOLT] Fix split jump table issues

[Huan Nguyen via Phabricator via llvm-commits]

nhuhuan updated this revision to Diff 448197.
nhuhuan marked an inline comment as done.
nhuhuan added a comment.

Add description for AbortedJTs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130431/new/

https://reviews.llvm.org/D130431

Files:
  bolt/lib/Core/BinaryContext.cpp


Index: bolt/lib/Core/BinaryContext.cpp
===================================================================
--- bolt/lib/Core/BinaryContext.cpp
+++ bolt/lib/Core/BinaryContext.cpp
@@ -672,6 +672,10 @@
 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;
@@ -690,6 +694,13 @@
     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 ";
@@ -709,7 +720,8 @@
                    dbgs() << "\n";);
         NextJTI->second->print(dbgs());
       }
-      llvm_unreachable("jump table heuristic failure");
+      AbortedJTs.push_back(JT);
+      continue;
     }
     for (BinaryFunction *Frag : JT->Parents) {
       for (uint64_t EntryAddress : JT->EntriesAsAddress)
@@ -739,6 +751,15 @@
         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()


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130431.448197.patch
Type: text/x-patch
Size: 2180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220727/083c32ee/attachment.bin>