Skip to content

Commit 066dd91

Browse files
authored
[BOLT] Offset LPStart to avoid unnecessary instructions (#116713)
For C++ exception handling, when we write a call site table, we must avoid emitting 0-value offsets for landing pads unless the call site has no landing pad. However, 0 can be a real offset from the start of the FDE if the FDE corresponds to a function fragment that starts with a landing pad. In such cases, we used to emit a trap instruction at the start of the fragment to guarantee non-zero LP offset. To avoid emitting unnecessary trap instructions, we can instead set LPStart to an offset from the FDE. If we emit it as [FDEStart - 1], then all real offsets from LPStart in FDE become non-negative.
1 parent e394fec commit 066dd91

File tree

1 file changed

+37
-33
lines changed

1 file changed

+37
-33
lines changed

bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -416,17 +416,6 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
416416
BF.duplicateConstantIslands();
417417
}
418418

419-
if (!FF.empty() && FF.front()->isLandingPad()) {
420-
assert(!FF.front()->isEntryPoint() &&
421-
"Landing pad cannot be entry point of function");
422-
// If the first block of the fragment is a landing pad, it's offset from the
423-
// start of the area that the corresponding LSDA describes is zero. In this
424-
// case, the call site entries in that LSDA have 0 as offset to the landing
425-
// pad, which the runtime interprets as "no handler". To prevent this,
426-
// insert some padding.
427-
Streamer.emitBytes(BC.MIB->getTrapFillValue());
428-
}
429-
430419
// Track the first emitted instruction with debug info.
431420
bool FirstInstr = true;
432421
for (BinaryBasicBlock *const BB : FF) {
@@ -926,39 +915,54 @@ void BinaryEmitter::emitLSDA(BinaryFunction &BF, const FunctionFragment &FF) {
926915
// Emit the LSDA header.
927916

928917
// If LPStart is omitted, then the start of the FDE is used as a base for
929-
// landing pad displacements. Then if a cold fragment starts with
930-
// a landing pad, this means that the first landing pad offset will be 0.
931-
// As a result, the exception handling runtime will ignore this landing pad
932-
// because zero offset denotes the absence of a landing pad.
933-
// For this reason, when the binary has fixed starting address we emit LPStart
934-
// as 0 and output the absolute value of the landing pad in the table.
918+
// landing pad displacements. Then, if a cold fragment starts with a landing
919+
// pad, this means that the first landing pad offset will be 0. However, C++
920+
// runtime treats 0 as if there is no landing pad present, thus we *must* emit
921+
// non-zero offsets for all valid LPs.
935922
//
936-
// If the base address can change, we cannot use absolute addresses for
937-
// landing pads (at least not without runtime relocations). Hence, we fall
938-
// back to emitting landing pads relative to the FDE start.
939-
// As we are emitting label differences, we have to guarantee both labels are
940-
// defined in the same section and hence cannot place the landing pad into a
941-
// cold fragment when the corresponding call site is in the hot fragment.
942-
// Because of this issue and the previously described issue of possible
943-
// zero-offset landing pad we have to place landing pads in the same section
944-
// as the corresponding invokes for shared objects.
923+
// As a solution, for fixed-address binaries we set LPStart to 0, and for
924+
// position-independent binaries we set LP start to FDE start minus one byte
925+
// for FDEs that start with a landing pad.
926+
const bool NeedsLPAdjustment = !FF.empty() && FF.front()->isLandingPad();
945927
std::function<void(const MCSymbol *)> emitLandingPad;
946928
if (BC.HasFixedLoadAddress) {
947929
Streamer.emitIntValue(dwarf::DW_EH_PE_udata4, 1); // LPStart format
948930
Streamer.emitIntValue(0, 4); // LPStart
949931
emitLandingPad = [&](const MCSymbol *LPSymbol) {
950-
if (!LPSymbol)
951-
Streamer.emitIntValue(0, 4);
952-
else
932+
if (LPSymbol)
953933
Streamer.emitSymbolValue(LPSymbol, 4);
934+
else
935+
Streamer.emitIntValue(0, 4);
954936
};
955937
} else {
956-
Streamer.emitIntValue(dwarf::DW_EH_PE_omit, 1); // LPStart format
938+
if (NeedsLPAdjustment) {
939+
// Use relative LPStart format and emit LPStart as [SymbolStart - 1].
940+
Streamer.emitIntValue(dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4, 1);
941+
MCSymbol *DotSymbol = BC.Ctx->createTempSymbol("LPBase");
942+
Streamer.emitLabel(DotSymbol);
943+
944+
const MCExpr *LPStartExpr = MCBinaryExpr::createSub(
945+
MCSymbolRefExpr::create(StartSymbol, *BC.Ctx),
946+
MCSymbolRefExpr::create(DotSymbol, *BC.Ctx), *BC.Ctx);
947+
LPStartExpr = MCBinaryExpr::createSub(
948+
LPStartExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
949+
Streamer.emitValue(LPStartExpr, 4);
950+
} else {
951+
// DW_EH_PE_omit means FDE start (StartSymbol) will be used as LPStart.
952+
Streamer.emitIntValue(dwarf::DW_EH_PE_omit, 1);
953+
}
957954
emitLandingPad = [&](const MCSymbol *LPSymbol) {
958-
if (!LPSymbol)
955+
if (LPSymbol) {
956+
const MCExpr *LPOffsetExpr = MCBinaryExpr::createSub(
957+
MCSymbolRefExpr::create(LPSymbol, *BC.Ctx),
958+
MCSymbolRefExpr::create(StartSymbol, *BC.Ctx), *BC.Ctx);
959+
if (NeedsLPAdjustment)
960+
LPOffsetExpr = MCBinaryExpr::createAdd(
961+
LPOffsetExpr, MCConstantExpr::create(1, *BC.Ctx), *BC.Ctx);
962+
Streamer.emitValue(LPOffsetExpr, 4);
963+
} else {
959964
Streamer.emitIntValue(0, 4);
960-
else
961-
Streamer.emitAbsoluteSymbolDiff(LPSymbol, StartSymbol, 4);
965+
}
962966
};
963967
}
964968

0 commit comments

Comments
 (0)