Skip to content

Commit 4783fdf

Browse files
mszabo-wikiaGitHub Enterprise
authored andcommitted
Fix Arm JIT for PIE builds (facebook#119)
The Arm JIT ends up segfaulting on PIE HHVM builds. Running a debug build pointed to `makeTarget32` receiving a 64-bit pointer instead of the expected 32-bit pointer, so rework affected usages to operate on 64-bit pointers and registers instead. x86 is not affected by this issue because it presumably makes no equivalent assumption about pointer sizing.
1 parent 62a1c2e commit 4783fdf

2 files changed

Lines changed: 16 additions & 19 deletions

File tree

hphp/runtime/vm/jit/smashable-instr-arm.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ TCA emitSmashableCall(CodeBlock& cb, CGMeta& meta, TCA target) {
6969
auto const the_start = cb.frontier();
7070
meta.smashableLocations.insert(the_start);
7171

72-
assertx((makeTarget32(target) & 3) == 0);
7372
addVeneer(meta, the_start, target);
7473
vixl::Label veneer_addr;
7574
a.bind(&veneer_addr);
@@ -87,7 +86,6 @@ TCA emitSmashableJmp(CodeBlock& cb, CGMeta& meta, TCA target) {
8786
auto const the_start = cb.frontier();
8887
meta.smashableLocations.insert(the_start);
8988

90-
assertx((makeTarget32(target) & 3) == 0);
9189
addVeneer(meta, the_start, target);
9290
vixl::Label veneer_addr;
9391
a.bind(&veneer_addr);
@@ -106,7 +104,6 @@ TCA emitSmashableJcc(CodeBlock& cb, CGMeta& meta, TCA target,
106104
auto const the_start = cb.frontier();
107105
meta.smashableLocations.insert(the_start);
108106

109-
assertx((makeTarget32(target) & 3) == 0);
110107
addVeneer(meta, the_start, target);
111108
vixl::Label veneer_addr;
112109
a.bind(&veneer_addr);
@@ -148,7 +145,7 @@ bool isVeneer(vixl::Instruction* ldr) {
148145
auto const rd = ldr->Rd();
149146

150147
return (ldr->IsLoadLiteral() &&
151-
ldr->Mask(LoadLiteralMask) == LDR_w_lit &&
148+
ldr->Mask(LoadLiteralMask) == LDR_x_lit &&
152149
br->Mask(UnconditionalBranchToRegisterMask) == BR &&
153150
br->Rn() == rd);
154151
}
@@ -206,7 +203,7 @@ void smashCall(TCA inst, TCA target) {
206203

207204
auto const bl = Instruction::Cast(inst);
208205
auto const ldr = bl->ImmPCOffsetTarget();
209-
patchTarget32(ldr->LiteralAddress(), target);
206+
patchTarget64(ldr->LiteralAddress(), target);
210207

211208
// If the target can be reached through a direct call, then patch the original
212209
// call. Notice that this optimization prevents a debugger guard from being
@@ -247,7 +244,7 @@ void smashJmp(TCA inst, TCA target) {
247244

248245
auto const b = Instruction::Cast(inst);
249246
auto const ldr = b->ImmPCOffsetTarget();
250-
patchTarget32(ldr->LiteralAddress(), target);
247+
patchTarget64(ldr->LiteralAddress(), target);
251248

252249
// If the target can be reached through a direct jump, then patch the original
253250
// jump. Notice that this optimization prevents a debugger guard from being
@@ -286,7 +283,7 @@ void smashJcc(TCA inst, TCA target) {
286283

287284
auto const b = Instruction::Cast(inst);
288285
auto const ldr = b->ImmPCOffsetTarget();
289-
patchTarget32(ldr->LiteralAddress(), target);
286+
patchTarget64(ldr->LiteralAddress(), target);
290287

291288
// If the target can be reached through a direct branch, then patch the
292289
// original branch. Notice that this optimization prevents a debugger guard

hphp/runtime/vm/jit/vasm-arm.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,9 @@ void Vgen::emitVeneers(Venv& env) {
510510
MacroAssembler av{*cb};
511511
vixl::Label target_data;
512512
meta.addressImmediates.insert(vaddr);
513-
poolLiteral(*cb, meta, (uint64_t)makeTarget32(veneer.target), 32, true);
513+
poolLiteral(*cb, meta, (uint64_t)veneer.target, 64, true);
514514
av.bind(&target_data);
515-
av.Ldr(rAsm_w, &target_data);
515+
av.Ldr(rAsm, &target_data);
516516
av.Br(rAsm);
517517

518518
// Update the veneer source instruction to jump/call the veneer.
@@ -663,17 +663,17 @@ void Vgen::processVveneers(Venv& env) {
663663

664664
void Vgen::patch(Venv& env) {
665665
// Patch the 32 bit target of the LDR
666-
auto patch = [&env](TCA instr, TCA target) {
666+
auto patch = [](TCA instr, TCA target) {
667667
// The LDR loading the address to branch to.
668668
auto ldr = Instruction::Cast(instr);
669669
auto const DEBUG_ONLY br = ldr->NextInstruction();
670-
assertx(ldr->Mask(LoadLiteralMask) == LDR_w_lit &&
670+
assertx(ldr->Mask(LoadLiteralMask) == LDR_x_lit &&
671671
br->Mask(UnconditionalBranchToRegisterMask) == BR &&
672672
ldr->Rd() == br->Rn());
673673
// The address the LDR loads.
674674
auto targetAddr = ldr->LiteralAddress();
675675
// Patch the 32 bit target following the LDR and BR
676-
patchTarget32(targetAddr, target);
676+
patchTarget64(targetAddr, target);
677677
};
678678

679679
for (auto const& p : env.jmps) {
@@ -1047,10 +1047,10 @@ void Vgen::emit(const jcc& i) {
10471047
recordAddressImmediate();
10481048
a->B(&skip, vixl::InvertCondition(C(i.cc)));
10491049
recordAddressImmediate();
1050-
poolLiteral(*env.cb, env.meta, (uint64_t)makeTarget32(a->frontier()),
1051-
32, false);
1050+
poolLiteral(*env.cb, env.meta, (uint64_t)a->frontier(),
1051+
64, false);
10521052
a->bind(&data); // This will be remmaped during the handleLiterals phase.
1053-
a->Ldr(rAsm_w, &data);
1053+
a->Ldr(rAsm, &data);
10541054
a->Br(rAsm);
10551055
a->bind(&skip);
10561056
}
@@ -1075,9 +1075,9 @@ void Vgen::emit(const jmp& i) {
10751075
// Emit a "far JMP" sequence for easy patching later. Static relocation
10761076
// might be able to simplify this (see optimizeFarJmp()).
10771077
recordAddressImmediate();
1078-
poolLiteral(*env.cb, env.meta, (uint64_t)a->frontier(), 32, false);
1078+
poolLiteral(*env.cb, env.meta, (uint64_t)a->frontier(), 64, false);
10791079
a->bind(&data); // This will be remapped during the handleLiterals phase.
1080-
a->Ldr(rAsm_w, &data);
1080+
a->Ldr(rAsm, &data);
10811081
a->Br(rAsm);
10821082
}
10831083

@@ -1087,9 +1087,9 @@ void Vgen::emit(const jmpi& i) {
10871087
// Cannot use simple a->Mov() since such a sequence cannot be
10881088
// adjusted while live following a relocation.
10891089
recordAddressImmediate();
1090-
poolLiteral(*env.cb, env.meta, (uint64_t)i.target, 32, false);
1090+
poolLiteral(*env.cb, env.meta, (uint64_t)i.target, 64, false);
10911091
a->bind(&data); // This will be remapped during the handleLiterals phase.
1092-
a->Ldr(rAsm_w, &data);
1092+
a->Ldr(rAsm, &data);
10931093
a->Br(rAsm);
10941094
}
10951095

0 commit comments

Comments
 (0)