[BOLT] Remove BinaryContext::getFunctionData

Summary:
In this diff we refactor the code around getting the original binary encoding of function's body.
The main changes are: remove BinaryContext::getFunctionData, remove the parameter of the method BinaryFunction::disassemble, introduce BinaryFunction::getData.

(cherry picked from FBD19824368)

bolt/src/BinaryContext.cpp

@@ -752,7 +752,7 @@ bool BinaryContext::hasValidCodePadding(const BinaryFunction &BF) {
   if (BF.getSize() == BF.getMaxSize())
     return true;
 
-  auto FunctionData = getFunctionData(BF);
+  auto FunctionData = BF.getData();
   assert(FunctionData && "cannot get function as data");
 
   uint64_t Offset = BF.getSize();
@@ -1645,27 +1645,6 @@ void BinaryContext::printInstruction(raw_ostream &OS,
   }
 }
 
-ErrorOr<ArrayRef<uint8_t>>
-BinaryContext::getFunctionData(const BinaryFunction &Function) const {
-  auto &Section = Function.getSection();
-  assert(Section.containsRange(Function.getAddress(), Function.getMaxSize()) &&
-         "wrong section for function");
-
-  if (!Section.isText() || Section.isVirtual() || !Section.getSize()) {
-    return std::make_error_code(std::errc::bad_address);
-  }
-
-  StringRef SectionContents = Section.getContents();
-
-  assert(SectionContents.size() == Section.getSize() &&
-         "section size mismatch");
-
-  // Function offset from the section start.
-  auto FunctionOffset = Function.getAddress() - Section.getAddress();
-  auto *Bytes = reinterpret_cast<const uint8_t *>(SectionContents.data());
-  return ArrayRef<uint8_t>(Bytes + FunctionOffset, Function.getMaxSize());
-}
-
 ErrorOr<BinarySection&> BinaryContext::getSectionForAddress(uint64_t Address) {
   auto SI = AddressToSection.upper_bound(Address);
   if (SI != AddressToSection.begin()) {
@@ -1895,7 +1874,7 @@ void BinaryContext::exitWithBugReport(StringRef Message,
             "sensitive contents being shared in this dump.\n";
   errs() << "\nOffending function: " << Function.getPrintName() << "\n\n";
   ScopedPrinter SP(errs());
-  SP.printBinaryBlock("Function contents", *getFunctionData(Function));
+  SP.printBinaryBlock("Function contents", *Function.getData());
   errs() << "\n";
   Function.dump();
   errs() << "ERROR: " << Message;

bolt/src/BinaryContext.h

@@ -719,10 +719,6 @@ public:
   /// Print the global symbol table.
   void printGlobalSymbols(raw_ostream& OS) const;
 
-  /// Get the raw bytes for a given function.
-  ErrorOr<ArrayRef<uint8_t>>
-  getFunctionData(const BinaryFunction &Function) const;
-
   /// Register information about the given \p Section so we can look up
   /// sections by address.
   BinarySection &registerSection(SectionRef Section);

bolt/src/BinaryFunction.cpp

@@ -905,10 +905,32 @@ MCSymbol *BinaryFunction::getOrCreateLocalLabel(uint64_t Address,
   return Result;
 }
 
-void BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
+ErrorOr<ArrayRef<uint8_t>> BinaryFunction::getData() const {
+  auto &Section = getSection();
+  assert(Section.containsRange(getAddress(), getMaxSize()) &&
+         "wrong section for function");
+
+  if (!Section.isText() || Section.isVirtual() || !Section.getSize()) {
+    return std::make_error_code(std::errc::bad_address);
+  }
+
+  StringRef SectionContents = Section.getContents();
+
+  assert(SectionContents.size() == Section.getSize() &&
+         "section size mismatch");
+
+  // Function offset from the section start.
+  auto Offset = getAddress() - Section.getAddress();
+  auto *Bytes = reinterpret_cast<const uint8_t *>(SectionContents.data());
+  return ArrayRef<uint8_t>(Bytes + Offset, getMaxSize());
+}
+
+void BinaryFunction::disassemble() {
   NamedRegionTimer T("disassemble", "Disassemble function", "buildfuncs",
                      "Build Binary Functions", opts::TimeBuild);
-
+  ErrorOr<ArrayRef<uint8_t>> ErrorOrFunctionData = getData();
+  assert(ErrorOrFunctionData && "Function data is not available");
+  ArrayRef<uint8_t> FunctionData = *ErrorOrFunctionData;
   assert(FunctionData.size() == getMaxSize() &&
          "function size does not match raw data size");
 

bolt/src/BinaryFunction.h

@@ -775,6 +775,9 @@ public:
     return iterator_range<const_cfi_iterator>(cie_begin(), cie_end());
   }
 
+  /// Returns the raw binary encoding of this function.
+  ErrorOr<ArrayRef<uint8_t>> getData() const;
+
   BinaryFunction &updateState(BinaryFunction::State State) {
     CurrentState = State;
     return *this;
@@ -1962,14 +1965,12 @@ public:
   /// it is probably another function.
   bool isSymbolValidInScope(const SymbolRef &Symbol, uint64_t SymbolSize) const;
 
-  /// Disassemble function from raw data \p FunctionData.
+  /// Disassemble function from raw data.
   /// If successful, this function will populate the list of instructions
   /// for this function together with offsets from the function start
   /// in the input. It will also populate Labels with destinations for
   /// local branches, and TakenBranches with [from, to] info.
   ///
-  /// \p FunctionData is the set bytes representing the function body.
-  ///
   /// The Function should be properly initialized before this function
   /// is called. I.e. function address and size should be set.
   ///
@@ -1977,7 +1978,7 @@ public:
   /// state to State:Disassembled.
   ///
   /// Returns false if disassembly failed.
-  void disassemble(ArrayRef<uint8_t> FunctionData);
+  void disassemble();
 
   /// Check that entry points have an associated instruction at their
   /// offsets after disassembly.

bolt/src/Passes/IndirectCallPromotion.cpp

@@ -464,7 +464,7 @@ IndirectCallPromotion::maybeGetHotJumpTableTargets(
   }
 
   if (BaseReg == BC.MRI->getProgramCounter()) {
-    auto FunctionData = BC.getFunctionData(Function);
+    auto FunctionData = Function.getData();
     const uint64_t Address = Function.getAddress() + DataOffset.get();
     MCInst OrigJmp;
     uint64_t Size;

bolt/src/RewriteInstance.cpp

@@ -2458,7 +2458,7 @@ void RewriteInstance::disassembleFunctions() {
       continue;
     }
 
-    auto FunctionData = BC->getFunctionData(Function);
+    auto FunctionData = Function.getData();
     if (!FunctionData) {
       // When could it happen?
       errs() << "BOLT-ERROR: corresponding section is non-executable or "
@@ -2477,7 +2477,7 @@ void RewriteInstance::disassembleFunctions() {
       reinterpret_cast<const uint8_t*>(InputFile->getData().data());
     Function.setFileOffset(FunctionData->begin() - FileBegin);
 
-    Function.disassemble(*FunctionData);
+    Function.disassemble();
 
     if (!Function.isSimple() && BC->HasRelocations) {
       BC->exitWithBugReport("function cannot be properly disassembled. "