Skip to content

Commit

Permalink
[PERF2BOLT/BOLT] Improve support for .so
Browse files Browse the repository at this point in the history
Summary:
Avoid asserting on inputs that are shared libraries with
R_X86_64_64 static relocs and RELATIVE dynamic relocations matching
those. Our relocation checking mechanism would expect the result of
the static relocation to be encoded in the binary, but the linker
instead puts it as an addend in the RELATIVE dyn reloc.

Also fix aggregation for .so if the executable segment is not the
first one in the binary.

(cherry picked from FBD18651868)
  • Loading branch information
rafaelauler authored and maksfb committed Nov 15, 2019
1 parent 4bcc53a commit 28f9187
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
11 changes: 10 additions & 1 deletion bolt/src/DataAggregator.cpp
Expand Up @@ -1795,6 +1795,14 @@ DataAggregator::parseMMapEvent() {
return make_error_code(llvm::errc::io_error);
}

const auto OffsetStr =
Line.split('@').second.ltrim().split(FieldSeparator).first;
if (OffsetStr.getAsInteger(0, ParsedInfo.Offset)) {
reportError("expected mmaped page-aligned offset");
Diag << "Found: " << OffsetStr << "in '" << Line << "'\n";
return make_error_code(llvm::errc::io_error);
}

consumeRestOfLine();

return std::make_pair(FileName, ParsedInfo);
Expand Down Expand Up @@ -1835,7 +1843,8 @@ std::error_code DataAggregator::parseMMapEvents() {
for (const auto &Pair : GlobalMMapInfo) {
dbgs() << " " << Pair.first << " : " << Pair.second.PID << " [0x"
<< Twine::utohexstr(Pair.second.BaseAddress) << ", "
<< Twine::utohexstr(Pair.second.Size) << "]\n";
<< Twine::utohexstr(Pair.second.Size) << " @ "
<< Twine::utohexstr(Pair.second.Offset) << "]\n";
}
);

Expand Down
7 changes: 6 additions & 1 deletion bolt/src/DataAggregator.h
Expand Up @@ -161,6 +161,7 @@ class DataAggregator : public DataReader {
pid_t PID{-1LL};
uint64_t BaseAddress;
uint64_t Size;
uint64_t Offset;
bool Forked{false};
uint64_t Time{0ULL}; // time in micro seconds
};
Expand Down Expand Up @@ -380,7 +381,11 @@ class DataAggregator : public DataReader {
/// conflicts.
void adjustAddress(uint64_t &Address, const MMapInfo &MMI) const {
if (Address >= MMI.BaseAddress && Address < MMI.BaseAddress + MMI.Size) {
Address -= MMI.BaseAddress;
// NOTE: Assumptions about the binary segment load table (PH for ELF)
// Segment file offset equals virtual address (which is true for .so)
// There aren't multiple executable segments loaded because MMapInfo
// doesn't support them.
Address -= MMI.BaseAddress - MMI.Offset;
} else if (Address < MMI.Size) {
// Make sure the address is not treated as belonging to the binary.
Address = (-1ULL);
Expand Down
11 changes: 10 additions & 1 deletion bolt/src/RewriteInstance.cpp
Expand Up @@ -1943,7 +1943,7 @@ void RewriteInstance::adjustCommandLineOptions() {

if (BC->HasRelocations && opts::AggregateOnly &&
!opts::StrictMode.getNumOccurrences()) {
outs() << "BOLT-INFO: enabling strict relocation mode for aggregtion "
outs() << "BOLT-INFO: enabling strict relocation mode for aggregation "
"purposes\n";
opts::StrictMode = true;
}
Expand Down Expand Up @@ -2056,6 +2056,15 @@ bool RewriteInstance::analyzeRelocation(const RelocationRef &Rel,
// Section symbols are marked as ST_Debug.
IsSectionRelocation = (cantFail(Symbol.getType()) == SymbolRef::ST_Debug);
}
// For PIE or dynamic libs, the linker may choose not to put the relocation
// result at the address if it is a X86_64_64 one because it will emit a
// dynamic relocation (X86_RELATIVE) for the dynamic linker and loader to
// resolve it at run time. The static relocation result goes as the addend
// of the dynamic relocation in this case. We can't verify these cases.
// FIXME: perhaps we can try to find if it really emitted a corresponding
// RELATIVE relocation at this offset with the correct value as the addend.
if (!BC->HasFixedLoadAddress && RelSize == 8)
SkipVerification = true;

if (IsSectionRelocation && !IsAArch64) {
auto Section = BC->getSectionForAddress(SymbolAddress);
Expand Down

0 comments on commit 28f9187

Please sign in to comment.