Skip to content

Commit

Permalink
Check function data in symbol table against data in .eh_frame.
Browse files Browse the repository at this point in the history
Summary:
At the moment we rely solely on the symbol table information to discover
function boundaries. However, similar information is contained in
.eh_frame. Verify that the information from these two sources is
consistent, and if it's not, then skip processing the functions with
conflicting information.

(cherry picked from FBD3043800)
  • Loading branch information
maksfb committed Mar 11, 2016
1 parent f2df1a8 commit e7e9e15
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 14 deletions.
13 changes: 7 additions & 6 deletions bolt/BinaryFunction.h
Expand Up @@ -109,10 +109,6 @@ class BinaryFunction {
/// Alignment requirements for the function.
uint64_t Alignment{1};

/// False if the function is too complex to reconstruct its control
/// flow graph and re-assemble.
bool IsSimple{true};

/// True if this function needs to be emitted in two separate parts, one for
/// the hot basic blocks and another for the cold basic blocks.
bool IsSplit{false};
Expand All @@ -125,6 +121,10 @@ class BinaryFunction {

BinaryContext &BC;

/// False if the function is too complex to reconstruct its control
/// flow graph and re-assemble.
bool IsSimple{true};

/// The address for the code for this function in codegen memory.
uint64_t ImageAddress{0};

Expand Down Expand Up @@ -326,9 +326,10 @@ class BinaryFunction {


BinaryFunction(std::string Name, SymbolRef Symbol, SectionRef Section,
uint64_t Address, uint64_t Size, BinaryContext &BC) :
uint64_t Address, uint64_t Size, BinaryContext &BC,
bool IsSimple = true) :
Name(Name), Symbol(Symbol), Section(Section), Address(Address),
Size(Size), BC(BC), CodeSectionName(".text." + Name),
Size(Size), BC(BC), IsSimple(IsSimple), CodeSectionName(".text." + Name),
FunctionNumber(++Count)
{}

Expand Down
14 changes: 12 additions & 2 deletions bolt/Exceptions.h
Expand Up @@ -42,15 +42,25 @@ class CFIReaderWriter {
}
}

using FDEsMap = std::map<uint64_t, const dwarf::FDE *>;

bool fillCFIInfoFor(BinaryFunction &Function) const;

// Include a new EHFrame, updating the .eh_frame_hdr
void rewriteHeaderFor(StringRef EHFrame, uint64_t EHFrameAddress,
uint64_t NewFrameHdrAddress,
ArrayRef<uint64_t> FailedAddresses);

using FDEsMap = std::map<uint64_t, const dwarf::FDE *>;
using fde_iterator = FDEsMap::const_iterator;

/// Get all FDEs discovered by this reader.
iterator_range<fde_iterator> fdes() const {
return iterator_range<fde_iterator>(FDEs.begin(), FDEs.end());
}

const FDEsMap &getFDEs() const {
return FDEs;
}

private:
const DWARFFrame &EHFrame;
uint64_t FrameHdrAddress;
Expand Down
41 changes: 38 additions & 3 deletions bolt/RewriteInstance.cpp
Expand Up @@ -540,9 +540,10 @@ void RewriteInstance::run() {
return;
}

// Main "loop".
discoverStorage();
readSymbolTable();
readSpecialSections();
discoverFileObjects();
disassembleFunctions();
runOptimizationPasses();
emitFunctions();
Expand All @@ -558,7 +559,7 @@ void RewriteInstance::run() {
rewriteFile();
}

void RewriteInstance::readSymbolTable() {
void RewriteInstance::discoverFileObjects() {
std::string FileSymbolName;

FileSymRefs.clear();
Expand Down Expand Up @@ -654,11 +655,45 @@ void RewriteInstance::readSymbolTable() {
continue;
}

// Checkout for conflicts with function data from FDEs.
bool IsSimple = true;
auto FDEI = CFIRdWrt->getFDEs().lower_bound(Address);
if (FDEI != CFIRdWrt->getFDEs().end()) {
auto &FDE = *FDEI->second;
if (FDEI->first != Address) {
// There's no matching starting address in FDE. Make sure the previous
// FDE does not contain this address.
if (FDEI != CFIRdWrt->getFDEs().begin()) {
--FDEI;
auto &PrevFDE = *FDEI->second;
auto PrevStart = PrevFDE.getInitialLocation();
auto PrevLength = PrevFDE.getAddressRange();
if (Address > PrevStart && Address < PrevStart + PrevLength) {
errs() << "BOLT-WARNING: function " << UniqueName
<< " is in conflict with FDE ["
<< Twine::utohexstr(PrevStart) << ", "
<< Twine::utohexstr(PrevStart + PrevLength)
<< "). Skipping.\n";
IsSimple = false;
}
}
} else if (FDE.getAddressRange() != SymbolSize) {
// Function addresses match but sizes differ.
errs() << "BOLT-WARNING: sizes differ for function " << UniqueName
<< ". FDE : " << FDE.getAddressRange()
<< "; symbol table : " << SymbolSize << ". Skipping.\n";

// Create maximum size non-simple function.
IsSimple = false;
SymbolSize = std::max(SymbolSize, FDE.getAddressRange());
}
}

// Create the function and add to the map.
BinaryFunctions.emplace(
Address,
BinaryFunction(UniqueName, Symbol, *Section, Address,
SymbolSize, *BC)
SymbolSize, *BC, IsSimple)
);
}
}
Expand Down
6 changes: 3 additions & 3 deletions bolt/RewriteInstance.h
Expand Up @@ -125,9 +125,9 @@ class RewriteInstance {
/// Run all the necessary steps to read, optimize and rewrite the binary.
void run();

/// Populate array of binary functions and file symbols from file symbol
/// table.
void readSymbolTable();
/// Populate array of binary functions and other objects of interest
/// from meta data in the file.
void discoverFileObjects();

/// Read .eh_frame, .eh_frame_hdr and .gcc_except_table sections for exception
/// and stack unwinding information.
Expand Down

0 comments on commit e7e9e15

Please sign in to comment.