Skip to content

Commit aa9cc72

Browse files
peterwaller-armpeterwaller-arm
authored
Reapply "[BOLT] Add --pad-funcs-before=func:n (#117924)" (#121918)
- **Reapply "[BOLT] Add --pad-funcs-before=func:n (#117924)"** - **[BOLT] Fix --pad-funcs{,-before} state misinteraction** When --pad-funcs-before was introduced, it introduced a bug whereby the first one to get parsed could influence the other. Ensure that each has its own state and test that they don't interact in this manner by testing how the `_subsequent` symbol moves when both arguments are supplied with different padding values. Fixed by having a function (and static state) for each of before/after.
1 parent 0d9cf26 commit aa9cc72

File tree

3 files changed

+104
-15
lines changed

3 files changed

+104
-15
lines changed

‎bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ BreakFunctionNames("break-funcs",
4747
cl::cat(BoltCategory));
4848

4949
static cl::list<std::string>
50-
FunctionPadSpec("pad-funcs",
51-
cl::CommaSeparated,
52-
cl::desc("list of functions to pad with amount of bytes"),
53-
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
54-
cl::Hidden,
55-
cl::cat(BoltCategory));
50+
FunctionPadSpec("pad-funcs", cl::CommaSeparated,
51+
cl::desc("list of functions to pad with amount of bytes"),
52+
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."),
53+
cl::Hidden, cl::cat(BoltCategory));
54+
55+
static cl::list<std::string> FunctionPadBeforeSpec(
56+
"pad-funcs-before", cl::CommaSeparated,
57+
cl::desc("list of functions to pad with amount of bytes"),
58+
cl::value_desc("func1:pad1,func2:pad2,func3:pad3,..."), cl::Hidden,
59+
cl::cat(BoltCategory));
5660

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

73-
size_t padFunction(const BinaryFunction &Function) {
74-
static std::map<std::string, size_t> FunctionPadding;
75-
76-
if (FunctionPadding.empty() && !FunctionPadSpec.empty()) {
77-
for (std::string &Spec : FunctionPadSpec) {
77+
size_t padFunction(std::map<std::string, size_t> &FunctionPadding,
78+
const cl::list<std::string> &Spec,
79+
const BinaryFunction &Function) {
80+
if (FunctionPadding.empty() && !Spec.empty()) {
81+
for (const std::string &Spec : Spec) {
7882
size_t N = Spec.find(':');
7983
if (N == std::string::npos)
8084
continue;
@@ -94,6 +98,15 @@ size_t padFunction(const BinaryFunction &Function) {
9498
return 0;
9599
}
96100

101+
size_t padFunctionBefore(const BinaryFunction &Function) {
102+
static std::map<std::string, size_t> CacheFunctionPadding;
103+
return padFunction(CacheFunctionPadding, FunctionPadBeforeSpec, Function);
104+
}
105+
size_t padFunctionAfter(const BinaryFunction &Function) {
106+
static std::map<std::string, size_t> CacheFunctionPadding;
107+
return padFunction(CacheFunctionPadding, FunctionPadSpec, Function);
108+
}
109+
97110
} // namespace opts
98111

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

335+
if (size_t Padding = opts::padFunctionBefore(Function)) {
336+
// Handle padFuncsBefore after the above alignment logic but before
337+
// symbol addresses are decided.
338+
if (!BC.HasRelocations) {
339+
BC.errs() << "BOLT-ERROR: -pad-before-funcs is not supported in "
340+
<< "non-relocation mode\n";
341+
exit(1);
342+
}
343+
344+
// Preserve Function.getMinAlign().
345+
if (!isAligned(Function.getMinAlign(), Padding)) {
346+
BC.errs() << "BOLT-ERROR: user-requested " << Padding
347+
<< " padding bytes before function " << Function
348+
<< " is not a multiple of the minimum function alignment ("
349+
<< Function.getMinAlign().value() << ").\n";
350+
exit(1);
351+
}
352+
353+
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding before function " << Function
354+
<< " with " << Padding << " bytes\n");
355+
356+
// Since the padding is not executed, it can be null bytes.
357+
Streamer.emitFill(Padding, 0);
358+
}
359+
322360
MCContext &Context = Streamer.getContext();
323361
const MCAsmInfo *MAI = Context.getAsmInfo();
324362

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

375413
// Emit padding if requested.
376-
if (size_t Padding = opts::padFunction(Function)) {
414+
if (size_t Padding = opts::padFunctionAfter(Function)) {
377415
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: padding function " << Function << " with "
378416
<< Padding << " bytes\n");
379417
Streamer.emitFill(Padding, MAI->getTextAlignFillValue());

‎bolt/lib/Passes/ReorderFunctions.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ extern cl::OptionCategory BoltOptCategory;
2828
extern cl::opt<unsigned> Verbosity;
2929
extern cl::opt<uint32_t> RandomSeed;
3030

31-
extern size_t padFunction(const bolt::BinaryFunction &Function);
31+
extern size_t padFunctionBefore(const bolt::BinaryFunction &Function);
32+
extern size_t padFunctionAfter(const bolt::BinaryFunction &Function);
3233

3334
extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
3435
cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions(
@@ -304,8 +305,10 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
304305
return false;
305306
if (B->isIgnored())
306307
return true;
307-
const size_t PadA = opts::padFunction(*A);
308-
const size_t PadB = opts::padFunction(*B);
308+
const size_t PadA = opts::padFunctionBefore(*A) +
309+
opts::padFunctionAfter(*A);
310+
const size_t PadB = opts::padFunctionBefore(*B) +
311+
opts::padFunctionAfter(*B);
309312
if (!PadA || !PadB) {
310313
if (PadA)
311314
return true;

‎bolt/test/AArch64/pad-before-funcs.s

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Test checks that --pad-before-funcs is working as expected.
2+
# It should be able to introduce a configurable offset for the _start symbol.
3+
# It should reject requests which don't obey the code alignment requirement.
4+
5+
# Tests check inserting padding before _start; and additionally a test where
6+
# padding is inserted after start. In each case, check that the following
7+
# symbol ends up in the expected place as well.
8+
9+
10+
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
11+
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -Wl,--section-start=.text=0x4000
12+
# RUN: llvm-bolt %t.exe -o %t.bolt.0 --pad-funcs-before=_start:0
13+
# RUN: llvm-bolt %t.exe -o %t.bolt.4 --pad-funcs-before=_start:4
14+
# RUN: llvm-bolt %t.exe -o %t.bolt.8 --pad-funcs-before=_start:8
15+
# RUN: llvm-bolt %t.exe -o %t.bolt.4.4 --pad-funcs-before=_start:4 --pad-funcs=_start:4
16+
# RUN: llvm-bolt %t.exe -o %t.bolt.4.8 --pad-funcs-before=_start:4 --pad-funcs=_start:8
17+
18+
# RUN: not llvm-bolt %t.exe -o %t.bolt.8 --pad-funcs-before=_start:1 2>&1 | FileCheck --check-prefix=CHECK-BAD-ALIGN %s
19+
20+
# CHECK-BAD-ALIGN: user-requested 1 padding bytes before function _start(*2) is not a multiple of the minimum function alignment (4).
21+
22+
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.0 | FileCheck --check-prefix=CHECK-0 %s
23+
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4 | FileCheck --check-prefix=CHECK-4 %s
24+
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.8 | FileCheck --check-prefix=CHECK-8 %s
25+
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4.4 | FileCheck --check-prefix=CHECK-4-4 %s
26+
# RUN: llvm-objdump --section=.text --disassemble %t.bolt.4.8 | FileCheck --check-prefix=CHECK-4-8 %s
27+
28+
# Trigger relocation mode in bolt.
29+
.reloc 0, R_AARCH64_NONE
30+
31+
.section .text
32+
33+
# CHECK-0: 0000000000400000 <_start>
34+
# CHECK-4: 0000000000400004 <_start>
35+
# CHECK-4-4: 0000000000400004 <_start>
36+
# CHECK-8: 0000000000400008 <_start>
37+
.globl _start
38+
_start:
39+
ret
40+
41+
# CHECK-0: 0000000000400004 <_subsequent>
42+
# CHECK-4: 0000000000400008 <_subsequent>
43+
# CHECK-4-4: 000000000040000c <_subsequent>
44+
# CHECK-4-8: 0000000000400010 <_subsequent>
45+
# CHECK-8: 000000000040000c <_subsequent>
46+
.globl _subsequent
47+
_subsequent:
48+
ret

0 commit comments

Comments
 (0)