[BOLT] Fix symbol table entries for secondary entries

Summary:
Commit "Support full instrumentation" changed the map
SymbolToFunction in BinaryContext to map secondary entries of functions
too. This introduced unexpected behavior in our symbol table rewriting
logic, which caused it to mistakenly write them with the address of the
original function. Fix the behavior of getBinaryFunctionAtAddress to
correct this. Also fix other users of SymbolToFunction to ensure they
are not accidentally using secondary entries when they shouldn't.

(cherry picked from FBD19168319)
This commit is contained in:
Rafael Auler 2019-12-18 12:14:42 -08:00 committed by Maksim Panchenko
parent 9aa276d349
commit de284bc510
6 changed files with 24 additions and 4 deletions

View File

@ -1910,7 +1910,8 @@ BinaryContext::getBinaryFunctionContainingAddress(uint64_t Address,
BinaryFunction *
BinaryContext::getBinaryFunctionAtAddress(uint64_t Address, bool Shallow) {
if (const auto *BD = getBinaryDataAtAddress(Address)) {
if (auto *BF = getFunctionForSymbol(BD->getSymbol())) {
auto *BF = getFunctionForSymbol(BD->getSymbol());
if (BF && BF->getAddress() == Address) {
while (BF->getParentFunction() && !Shallow) {
BF = BF->getParentFunction();
}

View File

@ -3391,7 +3391,7 @@ uint64_t BinaryFunction::getEntryForSymbol(const MCSymbol *EntrySymbol) const {
++NumEntries;
}
llvm_unreachable("no entry for symbol");
return std::numeric_limits<uint64_t>::max();
}
BinaryFunction::BasicBlockOrderType BinaryFunction::dfs() const {

View File

@ -1061,9 +1061,19 @@ public:
/// functions.
const MCSymbol *getSymbolForEntry(uint64_t EntryNum) const;
/// Return an entry ID corresponding to a symbol.
/// Return an entry ID corresponding to a symbol. Return
/// numeric_limits<T>::max() if entry does not exist.
uint64_t getEntryForSymbol(const MCSymbol *EntrySymbol) const;
/// Return true if \p is a valid secondary entry point in this function, false
/// otherwise.
bool isSecondaryEntryPoint(const MCSymbol *Label) const {
uint64_t EntryNo = getEntryForSymbol(Label);
if (EntryNo == std::numeric_limits<uint64_t>::max())
return false;
return EntryNo != 0;
}
MCSymbol *getColdSymbol() {
if (ColdSymbol)
return ColdSymbol;

View File

@ -225,6 +225,10 @@ bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
// Compare symbols as functions.
const auto *FunctionA = BC.getFunctionForSymbol(SymbolA);
const auto *FunctionB = BC.getFunctionForSymbol(SymbolB);
if (FunctionA && FunctionA->isSecondaryEntryPoint(SymbolA))
FunctionA = nullptr;
if (FunctionB && FunctionB->isSecondaryEntryPoint(SymbolB))
FunctionB = nullptr;
if (FunctionA && FunctionB) {
// Self-referencing functions and recursive calls.
if (FunctionA == &A && FunctionB == &B)

View File

@ -461,6 +461,10 @@ bool Inliner::inlineCallsInFunction(BinaryFunction &Function) {
continue;
}
// Don't inline calls to a secondary entry point in a target function
if (TargetFunction->isSecondaryEntryPoint(TargetSymbol))
continue;
// Don't do recursive inlining.
if (TargetFunction == &Function) {
++InstIt;

View File

@ -424,7 +424,8 @@ uint64_t LongJmpPass::getSymbolAddress(const BinaryContext &BC,
}
auto *TargetFunc = BC.getFunctionForSymbol(Target);
auto Iter = HotAddresses.find(TargetFunc);
if (Iter == HotAddresses.end()) {
if (Iter == HotAddresses.end() ||
(TargetFunc && TargetFunc->isSecondaryEntryPoint(Target))) {
// Look at BinaryContext's resolution for this symbol - this is a symbol not
// mapped to a BinaryFunction
auto ValueOrError = BC.getSymbolValue(*Target);