From c8232201165d52cddd5256449f906c3e9dcda092 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks@fb.com>
Date: Tue, 17 Sep 2019 14:24:31 -0700
Subject: [PATCH] [BOLT] Better check for compiler de-virtualization bug

Summary:
The existing check for compiler de-virtualization bug was not working
when the relocation reference did not fall on a function boundary.
As a result, we were falsely detecting "unmarked object in code".

When running the check, the address could be arbitrary, except it
shouldn't match any existing function. Additionally, check that there's
a proper reference to the de-virtualized callee to avoid false
positives.

(cherry picked from FBD17433887)
---
 bolt/src/RewriteInstance.cpp | 40 ++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/bolt/src/RewriteInstance.cpp b/bolt/src/RewriteInstance.cpp
index a92e281caaa6..85796179ccff 100644
--- a/bolt/src/RewriteInstance.cpp
+++ b/bolt/src/RewriteInstance.cpp
@@ -2331,6 +2331,33 @@ void RewriteInstance::readRelocations(const SectionRef &Section) {
       }
     }
 
+    // Workaround for a member function pointer de-virtualization bug. We check
+    // if a non-pc-relative relocation in the code is pointing to (fptr - 1).
+    if (IsToCode && ContainingBF && !Relocation::isPCRelative(RType) &&
+        (!ReferencedBF || (ReferencedBF->getAddress() != Address))) {
+      if (const auto *RogueBF = BC->getBinaryFunctionAtAddress(Address + 1)) {
+        // Do an extra check that the function was referenced previously.
+        // It's a linear search, but it should rarely happen.
+        bool Found{false};
+        for (const auto &RelKV : ContainingBF->Relocations) {
+          const auto &Rel = RelKV.second;
+          if (Rel.Symbol == RogueBF->getSymbol() &&
+              !Relocation::isPCRelative(Rel.Type)) {
+            Found = true;
+            break;
+          }
+        }
+
+        if (Found) {
+          errs() << "BOLT-WARNING: detected possible compiler "
+                    "de-virtualization bug: -1 addend used with "
+                    "non-pc-relative relocation against function "
+                 << *RogueBF << " in function " << *ContainingBF << '\n';
+          continue;
+        }
+      }
+    }
+
     uint64_t RefFunctionOffset = 0;
     MCSymbol *ReferencedSymbol = nullptr;
     if (ForceRelocation) {
@@ -2348,19 +2375,6 @@ void RewriteInstance::readRelocations(const SectionRef &Section) {
       if (ReferencedBF->containsAddress(Address, /*UseMaxSize = */true)) {
         RefFunctionOffset = Address - ReferencedBF->getAddress();
         if (RefFunctionOffset) {
-          // Workaround for member function pointer de-virtualization bug.
-          // We check if a code non-pc-relative relocation is pointing
-          // to a (fptr - 1).
-          if (ContainingBF && !Relocation::isPCRelative(RType)) {
-            if (const auto *NextBF =
-                BC->getBinaryFunctionAtAddress(Address + 1)) {
-              errs() << "BOLT-WARNING: detected possible compiler "
-                        "de-virtualization bug: -1 addend used with "
-                        "non-pc-relative relocation against function "
-                     << *NextBF << " in function " << *ContainingBF << '\n';
-              continue;
-            }
-          }
           ReferencedSymbol =
             ReferencedBF->getOrCreateLocalLabel(Address,
                                                 /*CreatePastEnd =*/ true);