Skip to content

Commit e860a01

Browse files
committed
Fix PIE builds
When built with jemalloc, HHVM defines several custom arenas that allocate from well-known address ranges. This historically was gated behind the Meta-specific `USE_JEMALLOC_EXTENT_HOOKS` define, which was removed in early September by upstream. PIE builds of HHVM are broken as a result since the entire arrangement appears to rely on at least the translation cache being in a predefined memory location, which PIE ipso facto precludes. So, have the `ENABLE_PIE` build flag define an `ENABLE_PIE` define and a `use_position_dependent_jemalloc_arenas` constexpr to disable the problematic behavior. Reinstate and use the previous limited arena setup for PIE builds where only the low arena exists and is backed by sbrk(2) rather than mmap, as it was prior to D78567435. PIE also means we can't use 32-bit SmallPtrs for things like function pointers, so define `FULLPTR_FOR_BUILTINS` in PIE builds and use `emitLdPackedPtr` when referring to them in the JIT rather than 32-bit-only instructions.
1 parent 7f53f5b commit e860a01

12 files changed

Lines changed: 232 additions & 138 deletions

File tree

hphp/runtime/base/program-functions.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,13 +2055,15 @@ static int execute_program_impl(int argc, char** argv) {
20552055
}
20562056

20572057
#if USE_JEMALLOC
2058-
if (Cfg::Server::Mode) {
2059-
purge_all();
2060-
setup_auto_arenas({Cfg::Eval::Num1GPagesForA0, Cfg::Eval::Num2MPagesForA0});
2061-
}
2062-
if (Cfg::Eval::FileBackedColdArena) {
2063-
set_cold_file_dir(Cfg::Eval::ColdArenaFileDir.c_str());
2064-
enable_high_cold_file();
2058+
if constexpr (use_position_dependent_jemalloc_arenas) {
2059+
if (Cfg::Server::Mode) {
2060+
purge_all();
2061+
setup_auto_arenas({Cfg::Eval::Num1GPagesForA0, Cfg::Eval::Num2MPagesForA0});
2062+
}
2063+
if (Cfg::Eval::FileBackedColdArena) {
2064+
set_cold_file_dir(Cfg::Eval::ColdArenaFileDir.c_str());
2065+
enable_high_cold_file();
2066+
}
20652067
}
20662068
#endif
20672069

@@ -2991,7 +2993,7 @@ static bool hphp_warmup(ExecutionContext *context,
29912993

29922994
void hphp_session_init(Treadmill::SessionKind session_kind,
29932995
Transport* transport,
2994-
RequestId id,
2996+
RequestId id,
29952997
RequestId root_req_id) {
29962998
if (id.unallocated()) id = RequestId::allocate();
29972999
assertx(!*s_sessionInitialized);

hphp/runtime/vm/func.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1854,8 +1854,10 @@ struct Func final {
18541854
// should not be inherited from.
18551855
jit::AtomicLowTCA m_prologueTable[1];
18561856
};
1857-
static constexpr size_t kFuncSize = debug ? (use_lowptr ? 72 : 96)
1858-
: (use_lowptr ? 64 : 88);
1857+
1858+
static constexpr size_t kTCAPtrDiff = sizeof(jit::AtomicLowTCA) == 8 ? 16 : 0;
1859+
static constexpr size_t kFuncSize = kTCAPtrDiff + (debug ? (use_lowptr ? 72 : 96)
1860+
: (use_lowptr ? 64 : 88));
18591861
static_assert(CheckSize<Func, kFuncSize>(), "");
18601862

18611863
///////////////////////////////////////////////////////////////////////////////

hphp/runtime/vm/jit/code-cache.cpp

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "hphp/util/hugetlb.h"
2828
#include "hphp/util/numa.h"
2929
#include "hphp/util/trace.h"
30+
#include "util/alloc-defs.h"
3031

3132
namespace HPHP::jit {
3233

@@ -117,29 +118,32 @@ CodeCache::CodeCache() {
117118
}
118119

119120
#if USE_JEMALLOC
120-
// When we have a low arena, TC must fit below lowArenaMinAddr(). If it
121-
// doesn't, we shrink things to make it so.
122-
auto const lowArenaStart = lowArenaMinAddr();
123-
if (Cfg::Server::Mode) {
124-
Logger::Info("lowArenaMinAddr(): 0x%lx", lowArenaStart);
125-
}
126-
always_assert_flog(
127-
usedBase + (32u << 20) <= lowArenaStart,
128-
"brk is too big for LOWPTR build (usedBase = {}, lowArenaStart = {})",
129-
usedBase, lowArenaStart
130-
);
121+
if constexpr (use_position_dependent_jemalloc_arenas) {
122+
// When using position-dependent custom arenas,
123+
// TC must fit below lowArenaMinAddr().
124+
// If it doesn't, we shrink things to make it so.
125+
auto const lowArenaStart = lowArenaMinAddr();
126+
if (Cfg::Server::Mode) {
127+
Logger::Info("lowArenaMinAddr(): 0x%lx", lowArenaStart);
128+
}
129+
always_assert_flog(
130+
usedBase + (32u << 20) <= lowArenaStart,
131+
"brk is too big for LOWPTR build (usedBase = {}, lowArenaStart = {})",
132+
usedBase, lowArenaStart
133+
);
131134

132-
if (usedBase + m_totalSize > lowArenaStart) {
133-
cutTCSizeTo(lowArenaStart - usedBase - thread_local_size);
134-
new (this) CodeCache;
135-
return;
135+
if (usedBase + m_totalSize > lowArenaStart) {
136+
cutTCSizeTo(lowArenaStart - usedBase - thread_local_size);
137+
new (this) CodeCache;
138+
return;
139+
}
140+
always_assert_flog(
141+
usedBase + m_totalSize <= lowArenaStart,
142+
"computed allocationSize ({}) is too large to fit within "
143+
"lowArenaStart ({}), usedBase = {}\n",
144+
m_totalSize, lowArenaStart, usedBase
145+
);
136146
}
137-
always_assert_flog(
138-
usedBase + m_totalSize <= lowArenaStart,
139-
"computed allocationSize ({}) is too large to fit within "
140-
"lowArenaStart ({}), usedBase = {}\n",
141-
m_totalSize, lowArenaStart, usedBase
142-
);
143147
#endif
144148
// Use MAP_FIXED_NOREPLACE instead of MAP_FIXED so we actually get
145149
// an error if we overlap with an existing mapping.

hphp/runtime/vm/jit/code-gen-helpers.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ void emitLdPackedPtr(Vout& v, Vptr mem, Vreg reg) {
6363
ldLowPtrImpl(v, mem, reg, PackedPtr<T>::bits);
6464
}
6565

66+
inline void emitLdTCAPtr(Vout& v, Vptr mem, Vreg reg) {
67+
ldLowPtrImpl(v, mem, reg, LowTCA::bits);
68+
}
69+
6670
/*
6771
* Store the LowPtr<T> in `reg' into `mem', with storage size `size'.
6872
*/

hphp/runtime/vm/jit/irlower-call.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void cgCall(IRLS& env, const IRInstruction* inst) {
156156
auto const pTabOff = safe_cast<int32_t>(Func::prologueTableOff());
157157
auto const ptrSize = safe_cast<int32_t>(sizeof(LowTCA));
158158
auto const dest = v.makeReg();
159-
v << loadzlq{r_func_prologue_callee()[numArgsInclUnpack * ptrSize + pTabOff], dest};
159+
emitLdTCAPtr(v, r_func_prologue_callee()[numArgsInclUnpack * ptrSize + pTabOff], dest);
160160
v << callphpr{dest, func_prologue_regs(withCtx)};
161161
} else {
162162
// It was not statically determined that the arguments are passed in a way
@@ -236,7 +236,7 @@ void cgCallFuncEntry(IRLS& env, const IRInstruction* inst) {
236236
// Load the FuncEntry address dynamically from the function.
237237
auto dest = v.makeReg();
238238
auto const funcEntryOff = safe_cast<int32_t>(Func::funcEntryOff());
239-
v << loadzlq{callee[funcEntryOff], dest};
239+
emitLdTCAPtr(v, callee[funcEntryOff], dest);
240240
// We have to use an ifdef instead of `if (use_lowptr)` here due to
241241
// funcIdOffset only being defined in non-lowptr mode.
242242
#ifdef USE_LOWPTR

hphp/runtime/vm/jit/types.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ using CTCA = const unsigned char*;
4040

4141
using TcaRange = folly::Range<TCA>;
4242

43+
#ifndef HHVM_PIE
4344
using LowTCA = SmallPtr<uint8_t>;
4445
using AtomicLowTCA = AtomicSmallPtr<uint8_t>;
46+
#else
47+
using LowTCA = FullPtr<uint8_t>;
48+
using AtomicLowTCA = AtomicFullPtr<uint8_t>;
49+
#endif
4550

4651
struct ctca_identity_hash {
4752
size_t operator()(CTCA val) const {

hphp/runtime/vm/jit/unique-stubs.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ TCA emitFuncPrologueRedispatch(CodeBlock& cb, DataBlock& data, const char* name)
297297
ifThen(v, CC_LE, sf, [&] (Vout& v) {
298298
// Fast path (numArgs <= numNonVariadicParams). Call the numArgs prologue.
299299
auto const dest = v.makeReg();
300-
v << loadzlq{callee[numArgs * ptrSize + pTabOff], dest};
300+
emitLdTCAPtr(v, callee[numArgs * ptrSize + pTabOff], dest);
301301
v << jmpr{dest, func_prologue_regs(true)};
302302
});
303303

@@ -361,7 +361,11 @@ TCA emitFuncPrologueRedispatch(CodeBlock& cb, DataBlock& data, const char* name)
361361

362362
// Call the numNonVariadicParams + 1 prologue.
363363
auto const dest = v.makeReg();
364-
v << loadzlq{Vreg(r_func_prologue_callee())[numNewArgs * ptrSize + pTabOff], dest};
364+
emitLdTCAPtr(
365+
v,
366+
Vreg(r_func_prologue_callee())[numNewArgs * ptrSize + pTabOff],
367+
dest
368+
);
365369
v << tailcallstubr{dest, func_prologue_regs(true)};
366370
}, name);
367371
}
@@ -419,7 +423,7 @@ TCA emitFuncPrologueRedispatchUnpack(CodeBlock& main, CodeBlock& cold,
419423
auto const pTabOff = safe_cast<int32_t>(Func::prologueTableOff());
420424
auto const ptrSize = safe_cast<int32_t>(sizeof(LowTCA));
421425
auto const dest = v.makeReg();
422-
v << loadzlq{callee[numNewArgs * ptrSize + pTabOff], dest};
426+
emitLdTCAPtr(v, callee[numNewArgs * ptrSize + pTabOff], dest);
423427
v << tailcallstubr{dest, func_prologue_regs(true)};
424428
}, name);
425429

hphp/util/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ HHVM_RENDER_CONFIG_SPECIFICATION(
6060
auto_source_group("hphp_util" "${CMAKE_CURRENT_SOURCE_DIR}"
6161
${ASM_SOURCES} ${CXX_SOURCES} ${HEADER_SOURCES})
6262

63+
if (ENABLE_PIE)
64+
target_compile_definitions(hphp_util PUBLIC HHVM_PIE FULLPTR_FOR_BUILTINS)
65+
endif()
66+
6367
target_link_libraries(hphp_util brotli folly zstd)
6468
if (LIBNUMA_LIBRARIES)
6569
target_link_libraries(hphp_util ${LIBNUMA_LIBRARIES})

hphp/util/alloc-defs.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,22 @@ constexpr bool use_jemalloc =
4848
#endif
4949
;
5050

51+
// Whether to use custom jemalloc arenas that allocate
52+
// from well-defined address ranges.
53+
// This is incompatible with OSS PIE builds.
54+
constexpr bool use_position_dependent_jemalloc_arenas =
55+
#if USE_JEMALLOC && !defined(HHVM_PIE)
56+
true
57+
#else
58+
false
59+
#endif
60+
;
61+
5162
// When we have control over the virtual address space for the heap, all
5263
// static/uncounted strings/arrays have addresses lower than kUncountedMaxAddr,
5364
// and all counted HeapObjects have higher addresses.
5465
constexpr bool addr_encodes_persistency =
55-
#if USE_JEMALLOC && defined(__x86_64__) && defined(__linux__)
66+
#if USE_JEMALLOC && defined(__x86_64__) && defined(__linux__) && !defined(HHVM_PIE)
5667
true
5768
#else
5869
false

0 commit comments

Comments
 (0)