Skip to content

Commit

Permalink
Bug 1568564 - Uplift unaligned private value fix behind a config opti…
Browse files Browse the repository at this point in the history
…on r=tcampbell a=jcristau

The full version of this change has landed in Firefox 70 (bug 1505902). It allows embedders to store unaligned pointers in JS::Value. (Previously they could only store aligned pointers.) In this patch, the change is hidden behind an opt-in config option. It is not part of the build for standard Firefox.

Uplift was requested by tjr on torbrowser and ptomato on mozjs.

Differential Revision: https://phabricator.services.mozilla.com/D39214

--HG--
extra : amend_source : f57be09494581109223315acc9c2bc618830b741
  • Loading branch information
iainireland committed Jul 24, 2019
1 parent 9118f4e commit 83ff5d3
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 1 deletion.
12 changes: 12 additions & 0 deletions js/moz.configure
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,15 @@ js_option('--enable-wasm-private-reftypes',

set_config('WASM_PRIVATE_REFTYPES', depends_if('--enable-wasm-private-reftypes')(lambda x: True))
set_define('WASM_PRIVATE_REFTYPES', depends_if('--enable-wasm-private-reftypes')(lambda x: True))

# Support for unaligned private values.
# This is the default in Firefox 70. It is backported to ESR68 under a compile
# option for the use of embedders. It is not part of the standard Firefox build.
# ==============================================================================

js_option('--enable-unaligned-private-values',
default=False,
help='{Enable|disable} unaligned private pointers in JS values')

set_define('JS_UNALIGNED_PRIVATE_VALUES',
depends_if('--enable-unaligned-private-values')(lambda x: True))
13 changes: 12 additions & 1 deletion js/public/Value.h
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,13 @@ union alignas(8) Value {
s_.tag_ = JSValueTag(0);
s_.payload_.ptr_ = ptr;
#elif defined(JS_PUNBOX64)
# if defined(JS_UNALIGNED_PRIVATE_VALUES)
// ptr must be a valid user-mode pointer, with the top 16 bits clear.
MOZ_ASSERT((uintptr_t(ptr) & 0xFFFF000000000000ULL) == 0);
asBits_ = uintptr_t(ptr);
# else
asBits_ = uintptr_t(ptr) >> 1;
# endif
#endif
MOZ_ASSERT(isDouble());
}
Expand All @@ -842,8 +848,13 @@ union alignas(8) Value {
#if defined(JS_NUNBOX32)
return s_.payload_.ptr_;
#elif defined(JS_PUNBOX64)
MOZ_ASSERT((asBits_ & 0x8000000000000000ULL) == 0);
# if defined(JS_UNALIGNED_PRIVATE_VALUES)
// This must be a valid user-mode pointer, with the top 16 bits clear.
MOZ_ASSERT((asBits_ & 0xFFFF000000000000ULL) == 0);
return reinterpret_cast<void*>(asBits_);
# else
return reinterpret_cast<void*>(asBits_ << 1);
# endif
#endif
}

Expand Down
4 changes: 4 additions & 0 deletions js/src/jit/arm64/MacroAssembler-arm64-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,9 @@ void MacroAssembler::branchPtr(Condition cond, const BaseIndex& lhs,

void MacroAssembler::branchPrivatePtr(Condition cond, const Address& lhs,
Register rhs, Label* label) {
#if defined(JS_UNALIGNED_PRIVATE_VALUES)
branchPtr(cond, lhs, rhs, label);
#else
vixl::UseScratchRegisterScope temps(this);
const Register scratch = temps.AcquireX().asUnsized();
if (rhs != scratch) {
Expand All @@ -1013,6 +1016,7 @@ void MacroAssembler::branchPrivatePtr(Condition cond, const Address& lhs,
// Instead of unboxing lhs, box rhs and do direct comparison with lhs.
rshiftPtr(Imm32(1), scratch);
branchPtr(cond, lhs, scratch, label);
#endif
}

void MacroAssembler::branchFloat(DoubleCondition cond, FloatRegister lhs,
Expand Down
2 changes: 2 additions & 0 deletions js/src/jit/arm64/MacroAssembler-arm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ BufferOffset MacroAssemblerCompat::movePatchablePtr(ImmWord ptr,

void MacroAssemblerCompat::loadPrivate(const Address& src, Register dest) {
loadPtr(src, dest);
#if !defined(JS_UNALIGNED_PRIVATE_VALUES)
asMasm().lshiftPtr(Imm32(1), dest);
#endif
}

void MacroAssemblerCompat::handleFailureWithHandlerTail(
Expand Down
4 changes: 4 additions & 0 deletions js/src/jit/arm64/MacroAssembler-arm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,11 @@ class MacroAssemblerCompat : public vixl::MacroAssembler {
}

void unboxPrivate(const ValueOperand& src, Register dest) {
#if defined(JS_UNALIGNED_PRIVATE_VALUES)
Mov(ARMRegister(dest, 64), ARMRegister(src.valueReg(), 64));
#else
Lsl(ARMRegister(dest, 64), ARMRegister(src.valueReg(), 64), 1);
#endif
}

void notBoolean(const ValueOperand& val) {
Expand Down
4 changes: 4 additions & 0 deletions js/src/jit/mips64/MacroAssembler-mips64-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,16 @@ void MacroAssembler::branch64(Condition cond, const Address& lhs,

void MacroAssembler::branchPrivatePtr(Condition cond, const Address& lhs,
Register rhs, Label* label) {
#if defined(JS_UNALIGNED_PRIVATE_VALUES)
branchPtr(cond, lhs, rhs, label);
#else
if (rhs != ScratchRegister) {
movePtr(rhs, ScratchRegister);
}
// Instead of unboxing lhs, box rhs and do direct comparison with lhs.
rshiftPtr(Imm32(1), ScratchRegister);
branchPtr(cond, lhs, ScratchRegister, label);
#endif
}

template <class L>
Expand Down
6 changes: 6 additions & 0 deletions js/src/jit/mips64/MacroAssembler-mips64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,9 @@ void MacroAssemblerMIPS64Compat::loadPtr(wasm::SymbolicAddress address,
void MacroAssemblerMIPS64Compat::loadPrivate(const Address& address,
Register dest) {
loadPtr(address, dest);
#if !defined(JS_UNALIGNED_PRIVATE_VALUES)
ma_dsll(dest, dest, Imm32(1));
#endif
}

void MacroAssemblerMIPS64Compat::loadUnalignedDouble(
Expand Down Expand Up @@ -1312,7 +1314,11 @@ void MacroAssemblerMIPS64Compat::unboxValue(const ValueOperand& src,

void MacroAssemblerMIPS64Compat::unboxPrivate(const ValueOperand& src,
Register dest) {
#if defined(JS_UNALIGNED_PRIVATE_VALUES)
ma_move(dest, src);
#else
ma_dsll(dest, src.valueReg(), Imm32(1));
#endif
}

void MacroAssemblerMIPS64Compat::boxDouble(FloatRegister src,
Expand Down
4 changes: 4 additions & 0 deletions js/src/jit/x64/MacroAssembler-x64-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -566,13 +566,17 @@ void MacroAssembler::branchPtr(Condition cond, wasm::SymbolicAddress lhs,

void MacroAssembler::branchPrivatePtr(Condition cond, const Address& lhs,
Register rhs, Label* label) {
#if defined(JS_UNALIGNED_PRIVATE_VALUES)
branchPtr(cond, lhs, rhs, label);
#else
ScratchRegisterScope scratch(*this);
if (rhs != scratch) {
movePtr(rhs, scratch);
}
// Instead of unboxing lhs, box rhs and do direct comparison with lhs.
rshiftPtr(Imm32(1), scratch);
branchPtr(cond, lhs, scratch, label);
#endif
}

void MacroAssembler::branchTruncateFloat32ToPtr(FloatRegister src,
Expand Down
4 changes: 4 additions & 0 deletions js/src/jit/x64/MacroAssembler-x64.h
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,9 @@ class MacroAssemblerX64 : public MacroAssemblerX86Shared {
}
void loadPrivate(const Address& src, Register dest) {
loadPtr(src, dest);
#if !defined(JS_UNALIGNED_PRIVATE_VALUES)
shlq(Imm32(1), dest);
#endif
}
void load32(AbsoluteAddress address, Register dest) {
if (X86Encoding::IsAddressImmediate(address.addr)) {
Expand Down Expand Up @@ -757,7 +759,9 @@ class MacroAssemblerX64 : public MacroAssemblerX86Shared {
}
void unboxPrivate(const ValueOperand& src, const Register dest) {
movq(src.valueReg(), dest);
#if !defined(JS_UNALIGNED_PRIVATE_VALUES)
shlq(Imm32(1), dest);
#endif
}

void notBoolean(const ValueOperand& val) { xorq(Imm32(1), val.valueReg()); }
Expand Down

0 comments on commit 83ff5d3

Please sign in to comment.