[BOLT] Fix .eh_frame update with ICF in non-relocation mode

Summary:
In a rare case, we may fold a function and fail to emit it in
non-relocation mode due to a function size increase. At the same time,
the function that the original function was folded into could have been
successfully emitted, e.g. because it was split in the presence of a
profile information.

Later, because the function was not emitted, we have to use its original
.eh_frame entry in the preserved .eh_frame section. However, that entry
is no longer referencing the original function, but the function that
the original was folded into. This happens since the original symbol gets
emitted at the other function location. As a result, .eh_frame entry for
the folded function is missing.

To prevent incorrect update of the original .eh_frame, create
relocations against absolute values. This guarantees preservation of the
section contents while updating pc-relative references.

(cherry picked from FBD21061130)
This commit is contained in:
Maksim Panchenko 2020-04-16 00:02:35 -07:00
parent 1be7a82540
commit 606532bdf1
6 changed files with 37 additions and 29 deletions

View File

@ -409,6 +409,9 @@ void BinaryFunction::print(raw_ostream &OS, std::string Annotation,
if (IsFragment) {
OS << "\n IsFragment : true";
}
if (isFolded()) {
OS << "\n FoldedInto : " << *getFoldedIntoFunction();
}
if (ParentFunction) {
OS << "\n Parent : " << *ParentFunction;
}

View File

@ -96,8 +96,9 @@ void BinarySection::emitAsData(MCStreamer &Streamer, StringRef NewName) const {
SectionOffset = Relocation.Offset;
}
DEBUG(dbgs() << "BOLT-DEBUG: emitting relocation for symbol "
<< Relocation.Symbol->getName() << " at offset 0x"
<< Twine::utohexstr(Relocation.Offset)
<< (Relocation.Symbol ? Relocation.Symbol->getName()
: StringRef("<none>"))
<< " at offset 0x" << Twine::utohexstr(Relocation.Offset)
<< " with size "
<< Relocation::getSizeForType(Relocation.Type) << '\n');
auto RelocationSize = Relocation.emit(&Streamer);

View File

@ -53,7 +53,7 @@ class BinarySection {
RelocationSetType Relocations;
// Pending relocations for this section. For the moment, just used by
// the .debug_info section. TODO: it would be nice to get rid of this.
// the .debug_info section.
RelocationSetType PendingRelocations;
// Output info
@ -336,7 +336,8 @@ public:
if (!Pending) {
Relocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value});
} else {
PendingRelocations.emplace(Relocation{Offset, Symbol, Type, Addend, Value});
PendingRelocations.emplace(
Relocation{Offset, Symbol, Type, Addend, Value});
}
}

View File

@ -421,9 +421,8 @@ void DWARFRewriter::updateLineTableOffsets() {
auto DbgInfoSection = BC.getUniqueSectionByName(".debug_info");
assert(DbgInfoSection && ".debug_info section must exist");
auto *Zero = BC.registerNameAtAddress("Zero", 0, 0, 0);
DbgInfoSection->addRelocation(LTOffset,
Zero,
nullptr,
ELF::R_X86_64_32,
Offset,
0,

View File

@ -281,24 +281,36 @@ size_t Relocation::emit(MCStreamer *Streamer) const {
if (isPCRelative(Type)) {
auto *TempLabel = Ctx.createTempSymbol();
Streamer->EmitLabel(TempLabel);
auto Value =
MCBinaryExpr::createSub(MCSymbolRefExpr::create(Symbol, Ctx),
MCSymbolRefExpr::create(TempLabel, Ctx),
Ctx);
if (Addend) {
Value = MCBinaryExpr::createAdd(Value,
MCConstantExpr::create(Addend, Ctx),
Ctx);
const MCExpr *Value{nullptr};
if (Symbol) {
Value = MCSymbolRefExpr::create(Symbol, Ctx);
if (Addend) {
Value = MCBinaryExpr::createAdd(Value,
MCConstantExpr::create(Addend, Ctx),
Ctx);
}
} else {
Value = MCConstantExpr::create(Addend, Ctx);
}
Value = MCBinaryExpr::createSub(Value,
MCSymbolRefExpr::create(TempLabel, Ctx),
Ctx);
Streamer->EmitValue(Value, Size);
} else if (Addend) {
return Size;
}
if (Symbol && Addend) {
auto Value = MCBinaryExpr::createAdd(MCSymbolRefExpr::create(Symbol, Ctx),
MCConstantExpr::create(Addend, Ctx),
Ctx);
Streamer->EmitValue(Value, Size);
} else {
} else if (Symbol) {
Streamer->EmitSymbolValue(Symbol, Size);
} else {
Streamer->EmitIntValue(Addend, Size);
}
return Size;
}

View File

@ -1479,6 +1479,7 @@ void RewriteInstance::relocateEHFrameSection() {
if (DwarfType == dwarf::DW_EH_PE_omit)
return;
// Only fix references that are relative to other locations.
if (!(DwarfType & dwarf::DW_EH_PE_pcrel) &&
!(DwarfType & dwarf::DW_EH_PE_textrel) &&
!(DwarfType & dwarf::DW_EH_PE_funcrel) &&
@ -1505,19 +1506,10 @@ void RewriteInstance::relocateEHFrameSection() {
break;
}
auto *BD = BC->getBinaryDataContainingAddress(Value);
auto *Symbol = BD ? BD->getSymbol() : nullptr;
auto Addend = BD ? Value - BD->getAddress() : 0;
if (!Symbol) {
DEBUG(dbgs() << "BOLT-DEBUG: creating symbol for DWARF reference at 0x"
<< Twine::utohexstr(Value) << '\n');
Symbol = BC->getOrCreateGlobalSymbol(Value, "FUNCat");
}
DEBUG(dbgs() << "BOLT-DEBUG: adding DWARF reference against symbol "
<< Symbol->getName() << '\n');
EHFrameSection->addRelocation(Offset, Symbol, RelType, Addend);
// Create a relocation against an absolute value since the goal is to
// preserve the contents of the section independent of the new values
// of referenced symbols.
EHFrameSection->addRelocation(Offset, nullptr, RelType, Value);
};
EHFrame.parse(DE, createReloc);