[BOLT] Ignore false function references

Summary:
A relocation can have an addend that makes it look as the relocated
value is in a different section from the symbol being relocated.
E.g., a relocation against a variable in .rodata could have a negative
offset that will make it look like it is against a symbol in .text
(a section that typically precedes .rodata).

Unless the relocation is against a section symbol, we know
exactly the symbol that is being relocated and there is no issue.
However, when the linker leaves only a section relocation (i.e. a
relocation against a section symbol when a temporary original symbol
gets deleted), we have to guess the relocated symbol, and can falsely
detect a function reference in the case described above.

The fix is to keep a section relocation if the corresponding
relocated value falls into a different section, and to detect and
ignore false function reference.

(cherry picked from FBD16030791)

bolt/src/BinaryContext.cpp

@@ -1336,18 +1336,10 @@ ErrorOr<BinarySection&> BinaryContext::getSectionForAddress(uint64_t Address) {
   auto SI = AddressToSection.upper_bound(Address);
   if (SI != AddressToSection.begin()) {
     --SI;
-    if (SI->first + SI->second->getSize() > Address)
-      return *SI->second;
-  }
-  return std::make_error_code(std::errc::bad_address);
-}
-
-ErrorOr<const BinarySection &>
-BinaryContext::getSectionForAddress(uint64_t Address) const {
-  auto SI = AddressToSection.upper_bound(Address);
-  if (SI != AddressToSection.begin()) {
-    --SI;
-    if (SI->first + SI->second->getSize() > Address)
+    auto UpperBound = SI->first + SI->second->getSize();
+    if (!SI->second->getSize())
+      UpperBound += 1;
+    if (UpperBound > Address)
       return *SI->second;
   }
   return std::make_error_code(std::errc::bad_address);

bolt/src/BinaryContext.h

@@ -785,7 +785,9 @@ public:
   /// functions only work for allocatable sections, i.e. ones with non-zero
   /// addresses.
   ErrorOr<BinarySection &> getSectionForAddress(uint64_t Address);
-  ErrorOr<const BinarySection &> getSectionForAddress(uint64_t Address) const;
+  ErrorOr<const BinarySection &> getSectionForAddress(uint64_t Address) const {
+    return const_cast<BinaryContext *>(this)->getSectionForAddress(Address);
+  }
 
   /// Return section(s) associated with given \p Name.
   iterator_range<NameToSectionMapType::iterator>

bolt/src/BinarySection.h

@@ -284,7 +284,8 @@ public:
   /// Does this section contain the given \p Address?
   /// Note: this is in terms of the original mapped binary addresses.
   bool containsAddress(uint64_t Address) const {
-    return getAddress() <= Address && Address < getEndAddress();
+    return (getAddress() <= Address && Address < getEndAddress()) ||
+           (getSize() == 0 && getAddress() == Address);
   }
 
   /// Does this section contain the range [\p Address, \p Address + \p Size)?

bolt/src/RewriteInstance.cpp

@@ -587,12 +587,6 @@ bool refersToReorderedSection(ErrorOr<BinarySection &> Section) {
   return Itr != opts::ReorderData.end();
 }
 
-StringRef getSectionName(SectionRef Section) {
-  StringRef SectionName;
-  Section.getName(SectionName);
-  return SectionName;
-}
-
 /// Create BinaryContext for a given architecture \p ArchName and
 /// triple \p TripleName.
 std::unique_ptr<BinaryContext>
@@ -1486,12 +1480,12 @@ void RewriteInstance::discoverFileObjects() {
       BF->addAlternativeName(UniqueName);
     } else {
       auto Section = BC->getSectionForAddress(Address);
+      assert(Section && "section for functions must be registered");
 
-      // Skip zero-size symbol in zero-size section
-      if (!Section && !SymbolSize)
+      // Skip symbols from zero-sized sections.
+      if (!Section->getSize())
         continue;
       
-      assert(Section && "section for functions must be registered.");
       BF = BC->createBinaryFunction(UniqueName, *Section, Address,
                                     SymbolSize, IsSimple);
     }
@@ -2031,9 +2025,7 @@ bool RewriteInstance::analyzeRelocation(const RelocationRef &Rel,
   bool SkipVerification = false;
   auto SymbolIter = Rel.getSymbol();
   if (SymbolIter == InputFile->symbol_end()) {
-    SymbolAddress = ExtractedValue - Addend;
-    if (IsPCRelative)
-      SymbolAddress += PCRelOffset;
+    SymbolAddress = ExtractedValue - Addend + PCRelOffset;
     auto *RelSymbol = BC->getOrCreateGlobalSymbol(SymbolAddress, "RELSYMat");
     SymbolName = RelSymbol->getName();
     IsSectionRelocation = false;
@@ -2044,23 +2036,19 @@ bool RewriteInstance::analyzeRelocation(const RelocationRef &Rel,
     SkipVerification = (cantFail(Symbol.getType()) == SymbolRef::ST_Other);
     // Section symbols are marked as ST_Debug.
     IsSectionRelocation = (cantFail(Symbol.getType()) == SymbolRef::ST_Debug);
-    if (IsSectionRelocation) {
-      auto Section = Symbol.getSection();
-      if (Section && *Section != InputFile->section_end()) {
-        SymbolName = "section " + std::string(getSectionName(**Section));
-        if (!IsAArch64) {
-          assert(SymbolAddress == (*Section)->getAddress() &&
-                 "section symbol address must be the same as section address");
-          // Convert section symbol relocations to regular relocations inside
-          // non-section symbols.
-          if (IsPCRelative) {
-            Addend = ExtractedValue - (SymbolAddress - PCRelOffset);
-          } else {
-            SymbolAddress = ExtractedValue;
-            Addend = 0;
-          }
-        }
-      }
+  }
+
+  if (IsSectionRelocation && !IsAArch64) {
+    auto Section = BC->getSectionForAddress(SymbolAddress);
+    assert(Section && "section expected for section relocation");
+    SymbolName = "section " + std::string(Section->getName());
+    // Convert section symbol relocations to regular relocations inside
+    // non-section symbols.
+    if (Section->containsAddress(ExtractedValue) && !IsPCRelative) {
+      SymbolAddress = ExtractedValue;
+      Addend = 0;
+    } else {
+      Addend = ExtractedValue - (SymbolAddress - PCRelOffset);
     }
   }
 
@@ -2299,6 +2287,12 @@ void RewriteInstance::readRelocations(const SectionRef &Section) {
           ReferencedBF = BF;
         }
       }
+    } else if (ReferencedBF) {
+      assert(RefSection && "section expected for section relocation");
+      if (ReferencedBF->getSection() != *RefSection) {
+        DEBUG(dbgs() << "BOLT-DEBUG: ignoring false function reference\n");
+        ReferencedBF = nullptr;
+      }
     }
 
     uint64_t RefFunctionOffset = 0;