Skip to content

[BOLT] Fix --pad-funcs{,-before} state misinteraction #121918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 7, 2025
Merged

[BOLT] Fix --pad-funcs{,-before} state misinteraction #121918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebe8796
Reapply "[BOLT] Add --pad-funcs-before=func:n (#117924)"
peterwaller-arm Jan 7, 2025
3b5d626
[BOLT] Fix --pad-funcs{,-before} state misinteraction
peterwaller-arm Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 50 additions & 12 deletions bolt/lib/Core/BinaryEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,16 @@ BreakFunctionNames("break-funcs",
cl::cat(BoltCategory));

static cl::list<std::string>
FunctionPadSpec("pad-funcs",
cl::CommaSeparated,
cl::desc("list of functions to pad with amount of bytes"),
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
cl::Hidden,
cl::cat(BoltCategory));
FunctionPadSpec("pad-funcs", cl::CommaSeparated,
cl::desc("list of functions to pad with amount of bytes"),
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
cl::Hidden, cl::cat(BoltCategory));

static cl::list<std::string> FunctionPadBeforeSpec(
"pad-funcs-before", cl::CommaSeparated,
cl::desc("list of functions to pad with amount of bytes"),
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."), cl::Hidden,
cl::cat(BoltCategory));

static cl::opt<bool> MarkFuncs(
"mark-funcs",
Expand All @@ -70,11 +74,11 @@ X86AlignBranchBoundaryHotOnly("x86-align-branch-boundary-hot-only",
cl::init(true),
cl::cat(BoltOptCategory));

size_t padFunction(const BinaryFunction &Function) {
static std::map<std::string, size_t> FunctionPadding;

if (FunctionPadding.empty() && !FunctionPadSpec.empty()) {
for (std::string &Spec : FunctionPadSpec) {
size_t padFunction(std::map<std::string, size_t> &FunctionPadding,
const cl::list<std::string> &Spec,
const BinaryFunction &Function) {
if (FunctionPadding.empty() && !Spec.empty()) {
for (const std::string &Spec : Spec) {
size_t N = Spec.find(':');
if (N == std::string::npos)
continue;
Expand All @@ -94,6 +98,15 @@ size_t padFunction(const BinaryFunction &Function) {
return 0;
}

size_t padFunctionBefore(const BinaryFunction &Function) {
static std::map<std::string, size_t> CacheFunctionPadding;
return padFunction(CacheFunctionPadding, FunctionPadBeforeSpec, Function);
}
size_t padFunctionAfter(const BinaryFunction &Function) {
static std::map<std::string, size_t> CacheFunctionPadding;
return padFunction(CacheFunctionPadding, FunctionPadSpec, Function);
}

} // namespace opts

namespace {
Expand Down Expand Up @@ -319,6 +332,31 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
Streamer.emitCodeAlignment(Function.getAlign(), &*BC.STI);
}

if (size_t Padding = opts::padFunctionBefore(Function)) {
// Handle padFuncsBefore after the above alignment logic but before
// symbol addresses are decided.
if (!BC.HasRelocations) {
BC.errs() << "BOLT-ERROR: -pad-before-funcs is not supported in "
<< "non-relocation mode\n";
exit(1);
}

// Preserve Function.getMinAlign().
if (!isAligned(Function.getMinAlign(), Padding)) {
BC.errs() << "BOLT-ERROR: user-requested " << Padding
<< " padding bytes before function " << Function
<< " is not a multiple of the minimum function alignment ("
<< Function.getMinAlign().value() << ").\n";
exit(1);
}

LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding before function " << Function
<< " with " << Padding << " bytes\n");

// Since the padding is not executed, it can be null bytes.
Streamer.emitFill(Padding, 0);
}

MCContext &Context = Streamer.getContext();
const MCAsmInfo *MAI = Context.getAsmInfo();

Expand Down Expand Up @@ -373,7 +411,7 @@ bool BinaryEmitter::emitFunction(BinaryFunction &Function,
emitFunctionBody(Function, FF, /*EmitCodeOnly=*/false);

// Emit padding if requested.
if (size_t Padding = opts::padFunction(Function)) {
if (size_t Padding = opts::padFunctionAfter(Function)) {
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding function " << Function << " with "
<< Padding << " bytes\n");
Streamer.emitFill(Padding, MAI->getTextAlignFillValue());
Expand Down
9 changes: 6 additions & 3 deletions bolt/lib/Passes/ReorderFunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ extern cl::OptionCategory BoltOptCategory;
extern cl::opt<unsigned> Verbosity;
extern cl::opt<uint32_t> RandomSeed;

extern size_t padFunction(const bolt::BinaryFunction &Function);
extern size_t padFunctionBefore(const bolt::BinaryFunction &Function);
extern size_t padFunctionAfter(const bolt::BinaryFunction &Function);

extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions(
Expand Down Expand Up @@ -304,8 +305,10 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
return false;
if (B->isIgnored())
return true;
const size_t PadA = opts::padFunction(*A);
const size_t PadB = opts::padFunction(*B);
const size_t PadA = opts::padFunctionBefore(*A) +
opts::padFunctionAfter(*A);
const size_t PadB = opts::padFunctionBefore(*B) +
opts::padFunctionAfter(*B);
if (!PadA || !PadB) {
if (PadA)
return true;
Expand Down
48 changes: 48 additions & 0 deletions bolt/test/AArch64/pad-before-funcs.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Test checks that --pad-before-funcs is working as expected.
# It should be able to introduce a configurable offset for the _start symbol.
# It should reject requests which don't obey the code alignment requirement.

# Tests check inserting padding before _start; and additionally a test where
# padding is inserted after start. In each case, check that the following
# symbol ends up in the expected place as well.


# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -Wl,--section-start=.text=0x4000
# RUN: llvm-bolt %t.exe -o %t.bolt.0 --pad-funcs-before=_start:0
# RUN: llvm-bolt %t.exe -o %t.bolt.4 --pad-funcs-before=_start:4
# RUN: llvm-bolt %t.exe -o %t.bolt.8 --pad-funcs-before=_start:8
# RUN: llvm-bolt %t.exe -o %t.bolt.4.4 --pad-funcs-before=_start:4 --pad-funcs=_start:4
# RUN: llvm-bolt %t.exe -o %t.bolt.4.8 --pad-funcs-before=_start:4 --pad-funcs=_start:8

# RUN: not llvm-bolt %t.exe -o %t.bolt.8 --pad-funcs-before=_start:1 2>&1 | FileCheck --check-prefix=CHECK-BAD-ALIGN %s

# CHECK-BAD-ALIGN: user-requested 1 padding bytes before function _start(*2) is not a multiple of the minimum function alignment (4).

# RUN: llvm-objdump --section=.text --disassemble %t.bolt.0 | FileCheck --check-prefix=CHECK-0 %s
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4 | FileCheck --check-prefix=CHECK-4 %s
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.8 | FileCheck --check-prefix=CHECK-8 %s
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4.4 | FileCheck --check-prefix=CHECK-4-4 %s
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4.8 | FileCheck --check-prefix=CHECK-4-8 %s

# Trigger relocation mode in bolt.
.reloc 0, R_AARCH64_NONE

.section .text

# CHECK-0: 0000000000400000 <_start>
# CHECK-4: 0000000000400004 <_start>
# CHECK-4-4: 0000000000400004 <_start>
# CHECK-8: 0000000000400008 <_start>
.globl _start
_start:
ret

# CHECK-0: 0000000000400004 <_subsequent>
# CHECK-4: 0000000000400008 <_subsequent>
# CHECK-4-4: 000000000040000c <_subsequent>
# CHECK-4-8: 0000000000400010 <_subsequent>
# CHECK-8: 000000000040000c <_subsequent>
.globl _subsequent
_subsequent:
ret
Loading