Skip to content

Commit e4c19e0

Browse files
mszabo-wikiaGitHub Enterprise
authored andcommitted
Fix PIE builds (facebook#117)
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 c8ba1d7 . 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 4783fdf commit e4c19e0

11 files changed

Lines changed: 239 additions & 135 deletions

File tree

hphp/runtime/base/program-functions.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2021,13 +2021,15 @@ static int execute_program_impl(int argc, char** argv) {
20212021
}
20222022

20232023
#if USE_JEMALLOC
2024-
if (Cfg::Server::Mode) {
2025-
purge_all();
2026-
setup_auto_arenas({Cfg::Eval::Num1GPagesForA0, Cfg::Eval::Num2MPagesForA0});
2027-
}
2028-
if (Cfg::Eval::FileBackedColdArena) {
2029-
set_cold_file_dir(Cfg::Eval::ColdArenaFileDir.c_str());
2030-
enable_high_cold_file();
2024+
if constexpr (use_position_dependent_jemalloc_arenas) {
2025+
if (Cfg::Server::Mode) {
2026+
purge_all();
2027+
setup_auto_arenas({Cfg::Eval::Num1GPagesForA0, Cfg::Eval::Num2MPagesForA0});
2028+
}
2029+
if (Cfg::Eval::FileBackedColdArena) {
2030+
set_cold_file_dir(Cfg::Eval::ColdArenaFileDir.c_str());
2031+
enable_high_cold_file();
2032+
}
20312033
}
20322034
#endif
20332035

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: 24 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,31 @@ 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 we have custom arenas, TC must fit below lowArenaMinAddr(). If it
123+
// doesn't, we shrink things to make it so.
124+
auto const lowArenaStart = lowArenaMinAddr();
125+
if (Cfg::Server::Mode) {
126+
Logger::Info("lowArenaMinAddr(): 0x%lx", lowArenaStart);
127+
}
128+
always_assert_flog(
129+
usedBase + (32u << 20) <= lowArenaStart,
130+
"brk is too big for LOWPTR build (usedBase = {}, lowArenaStart = {})",
131+
usedBase, lowArenaStart
132+
);
131133

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

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+
emitLdPackedPtr<uint8_t>(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+
emitLdPackedPtr<uint8_t>(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
@@ -300,7 +300,7 @@ TCA emitFuncPrologueRedispatch(CodeBlock& cb, DataBlock& data, const char* name)
300300
ifThen(v, CC_LE, sf, [&] (Vout& v) {
301301
// Fast path (numArgs <= numNonVariadicParams). Call the numArgs prologue.
302302
auto const dest = v.makeReg();
303-
v << loadzlq{callee[numArgs * ptrSize + pTabOff], dest};
303+
emitLdPackedPtr<uint8_t>(v, callee[numArgs * ptrSize + pTabOff], dest);
304304
v << jmpr{dest, func_prologue_regs(true)};
305305
});
306306

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

365365
// Call the numNonVariadicParams + 1 prologue.
366366
auto const dest = v.makeReg();
367-
v << loadzlq{Vreg(r_func_prologue_callee())[numNewArgs * ptrSize + pTabOff], dest};
367+
emitLdPackedPtr<uint8_t>(
368+
v,
369+
Vreg(r_func_prologue_callee())[numNewArgs * ptrSize + pTabOff],
370+
dest
371+
);
368372
v << tailcallstubr{dest, func_prologue_regs(true)};
369373
}, name);
370374
}
@@ -422,7 +426,7 @@ TCA emitFuncPrologueRedispatchUnpack(CodeBlock& main, CodeBlock& cold,
422426
auto const pTabOff = safe_cast<int32_t>(Func::prologueTableOff());
423427
auto const ptrSize = safe_cast<int32_t>(sizeof(LowTCA));
424428
auto const dest = v.makeReg();
425-
v << loadzlq{callee[numNewArgs * ptrSize + pTabOff], dest};
429+
emitLdPackedPtr<uint8_t>(v, callee[numNewArgs * ptrSize + pTabOff], dest);
426430
v << tailcallstubr{dest, func_prologue_regs(true)};
427431
}, name);
428432

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)