tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit e00c7523b56d675f68ebc5cf34c85efd13f530bb
parent f48b35fc556822b979932915b36d28452b745bec
Author: Ben Visness <bvisness@mozilla.com>
Date:   Mon, 24 Nov 2025 21:41:05 +0000

Bug 1949860: Skip null checks in casts using the signal handler. r=rhunt

When doing a ref.cast, we can skip the initial null check in some cases
by using the signal handler. This reduces code size yet further!

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

Diffstat:
Mjs/src/jit-test/tests/wasm/gc/cast-abstract.js | 3---
Mjs/src/jit/CodeGenerator.cpp | 48+++++++++++++++++++++++++++++++-----------------
Mjs/src/jit/MacroAssembler.cpp | 106++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
Mjs/src/jit/MacroAssembler.h | 28++++++++++++++++++----------
Mjs/src/wasm/WasmBaselineCompile.cpp | 24+++++++++++++++---------
Mjs/src/wasm/WasmSignalHandlers.cpp | 2++
6 files changed, 150 insertions(+), 61 deletions(-)

diff --git a/js/src/jit-test/tests/wasm/gc/cast-abstract.js b/js/src/jit-test/tests/wasm/gc/cast-abstract.js @@ -1,6 +1,3 @@ -// |jit-test| skip-if: getPrefValue("wasm_lazy_tiering") -// TODO: skip lazy tiering temporarily to avoid a timeout - load(libdir + "wasm-binary.js"); // Replace this with a string like 'non-null (ref $s1) -> (ref any) -> (ref any)' to test exactly one specific case. Makes debugging easier. diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp @@ -8318,7 +8318,8 @@ void CodeGenerator::emitWasmAnyrefResultChecks(LInstruction* lir, Label ok; masm.branchWasmRefIsSubtype(output, wasm::MaybeRefType(), destType.value(), - &ok, true, temp1, temp2, temp3); + &ok, /*onSuccess=*/true, + /*signalNullChecks=*/false, temp1, temp2, temp3); masm.breakpoint(); masm.bind(&ok); @@ -20944,8 +20945,9 @@ void CodeGenerator::visitWasmRefTestAbstract(LWasmRefTestAbstract* ins) { Label onFail; Label join; masm.branchWasmRefIsSubtype(ref, mir->ref()->wasmRefType(), mir->destType(), - &onSuccess, /*onSuccess=*/true, superSTV, - scratch1, scratch2); + &onSuccess, + /*onSuccess=*/true, /*signalNullChecks=*/false, + superSTV, scratch1, scratch2); masm.bind(&onFail); masm.xor32(result, result); masm.jump(&join); @@ -20968,8 +20970,9 @@ void CodeGenerator::visitWasmRefTestConcrete(LWasmRefTestConcrete* ins) { Label onSuccess; Label join; masm.branchWasmRefIsSubtype(ref, mir->ref()->wasmRefType(), mir->destType(), - &onSuccess, /*onSuccess=*/true, superSTV, - scratch1, scratch2); + &onSuccess, + /*onSuccess=*/true, /*signalNullChecks=*/false, + superSTV, scratch1, scratch2); masm.move32(Imm32(0), result); masm.jump(&join); masm.bind(&onSuccess); @@ -20984,9 +20987,10 @@ void CodeGenerator::visitWasmRefTestAbstractAndBranch( Register scratch1 = ToTempRegisterOrInvalid(ins->temp0()); Label* onSuccess = getJumpLabelForBranch(ins->ifTrue()); Label* onFail = getJumpLabelForBranch(ins->ifFalse()); - masm.branchWasmRefIsSubtype( - ref, ins->sourceType(), ins->destType(), onSuccess, /*onSuccess=*/true, - Register::Invalid(), scratch1, Register::Invalid()); + masm.branchWasmRefIsSubtype(ref, ins->sourceType(), ins->destType(), + onSuccess, /*onSuccess=*/true, + /*signalNullChecks=*/false, Register::Invalid(), + scratch1, Register::Invalid()); masm.jump(onFail); } @@ -20999,9 +21003,9 @@ void CodeGenerator::visitWasmRefTestConcreteAndBranch( Register scratch2 = ToTempRegisterOrInvalid(ins->temp1()); Label* onSuccess = getJumpLabelForBranch(ins->ifTrue()); Label* onFail = getJumpLabelForBranch(ins->ifFalse()); - masm.branchWasmRefIsSubtype(ref, ins->sourceType(), ins->destType(), - onSuccess, /*onSuccess=*/true, superSTV, scratch1, - scratch2); + masm.branchWasmRefIsSubtype( + ref, ins->sourceType(), ins->destType(), onSuccess, /*onSuccess=*/true, + /*signalNullChecks=*/false, superSTV, scratch1, scratch2); masm.jump(onFail); } @@ -21020,9 +21024,14 @@ void CodeGenerator::visitWasmRefCastAbstract(LWasmRefCastAbstract* ins) { masm.wasmTrap(wasm::Trap::BadCast, mir->trapSiteDesc()); }); addOutOfLineCode(ool, ins->mir()); - masm.branchWasmRefIsSubtype(ref, mir->ref()->wasmRefType(), mir->destType(), - ool->entry(), /*onSuccess=*/false, superSTV, - scratch1, scratch2); + FaultingCodeOffset fco = masm.branchWasmRefIsSubtype( + ref, mir->ref()->wasmRefType(), mir->destType(), ool->entry(), + /*onSuccess=*/false, /*signalNullChecks=*/true, superSTV, scratch1, + scratch2); + if (fco.isValid()) { + masm.append(wasm::Trap::BadCast, wasm::TrapMachineInsnForLoadWord(), + fco.get(), mir->trapSiteDesc()); + } } void CodeGenerator::visitWasmRefCastConcrete(LWasmRefCastConcrete* ins) { @@ -21040,9 +21049,14 @@ void CodeGenerator::visitWasmRefCastConcrete(LWasmRefCastConcrete* ins) { masm.wasmTrap(wasm::Trap::BadCast, mir->trapSiteDesc()); }); addOutOfLineCode(ool, ins->mir()); - masm.branchWasmRefIsSubtype(ref, mir->ref()->wasmRefType(), mir->destType(), - ool->entry(), /*onSuccess=*/false, superSTV, - scratch1, scratch2); + FaultingCodeOffset fco = masm.branchWasmRefIsSubtype( + ref, mir->ref()->wasmRefType(), mir->destType(), ool->entry(), + /*onSuccess=*/false, /*signalNullChecks=*/true, superSTV, scratch1, + scratch2); + if (fco.isValid()) { + masm.append(wasm::Trap::BadCast, wasm::TrapMachineInsnForLoadWord(), + fco.get(), mir->trapSiteDesc()); + } } void CodeGenerator::callWasmStructAllocFun( diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp @@ -6924,15 +6924,16 @@ BranchWasmRefIsSubtypeRegisters MacroAssembler::regsForBranchWasmRefIsSubtype( } } -void MacroAssembler::branchWasmRefIsSubtype( +FaultingCodeOffset MacroAssembler::branchWasmRefIsSubtype( Register ref, wasm::MaybeRefType sourceType, wasm::RefType destType, - Label* label, bool onSuccess, Register superSTV, Register scratch1, - Register scratch2) { + Label* label, bool onSuccess, bool signalNullChecks, Register superSTV, + Register scratch1, Register scratch2) { + FaultingCodeOffset result = FaultingCodeOffset(); switch (destType.hierarchy()) { case wasm::RefTypeHierarchy::Any: { - branchWasmRefIsSubtypeAny(ref, sourceType.valueOr(wasm::RefType::any()), - destType, label, onSuccess, superSTV, scratch1, - scratch2); + result = branchWasmRefIsSubtypeAny( + ref, sourceType.valueOr(wasm::RefType::any()), destType, label, + onSuccess, signalNullChecks, superSTV, scratch1, scratch2); } break; case wasm::RefTypeHierarchy::Func: { branchWasmRefIsSubtypeFunc(ref, sourceType.valueOr(wasm::RefType::func()), @@ -6951,12 +6952,14 @@ void MacroAssembler::branchWasmRefIsSubtype( default: MOZ_CRASH("unknown type hierarchy for wasm cast"); } + MOZ_ASSERT_IF(!signalNullChecks, !result.isValid()); + return result; } -void MacroAssembler::branchWasmRefIsSubtypeAny( +FaultingCodeOffset MacroAssembler::branchWasmRefIsSubtypeAny( Register ref, wasm::RefType sourceType, wasm::RefType destType, - Label* label, bool onSuccess, Register superSTV, Register scratch1, - Register scratch2) { + Label* label, bool onSuccess, bool signalNullChecks, Register superSTV, + Register scratch1, Register scratch2) { MOZ_ASSERT(sourceType.isValid()); MOZ_ASSERT(destType.isValid()); MOZ_ASSERT(sourceType.isAnyHierarchy()); @@ -6986,8 +6989,53 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( bind(&fallthrough); }; + // We can omit the null check, and catch nulls in signal handling, if the dest + // type is non-nullable and the cast process will attempt to load from the + // (null) ref. This logic must be kept in sync with the flow of checks below; + // this will be checked by fcoLogicCheck. + bool willLoadShape = + // Dest type is a GC object + (wasm::RefType::isSubTypeOf(destType, wasm::RefType::eq()) && + !destType.isI31() && !destType.isNone()) && + // Source type is not already known to be a GC object + !(wasm::RefType::isSubTypeOf(sourceType, wasm::RefType::struct_()) || + wasm::RefType::isSubTypeOf(sourceType, wasm::RefType::array())); + bool willLoadSTV = + // Dest type is struct or array or concrete type + (wasm::RefType::isSubTypeOf(destType, wasm::RefType::struct_()) || + wasm::RefType::isSubTypeOf(destType, wasm::RefType::array())) && + !destType.isNone(); + bool canOmitNullCheck = signalNullChecks && !destType.isNullable() && + (willLoadShape || willLoadSTV); + + FaultingCodeOffset fco = FaultingCodeOffset(); + auto trackFCO = [&](FaultingCodeOffset newFco) { + if (signalNullChecks && !destType.isNullable() && !fco.isValid()) { + fco = newFco; + } + }; + auto fcoLogicCheck = mozilla::DebugOnly(mozilla::MakeScopeExit([&]() { + // When we think we can omit the null check, we should get a valid FCO, and + // vice versa. These are asserted to match because: + // - If we think we cannot omit the null check, but get a valid FCO, then + // we wasted an optimization opportunity. + // - If we think we *can* omit the null check, but do not get a valid FCO, + // then we will segfault. + // We could ignore the former check, but better to be precise and ensure + // that we are getting the optimizations we expect. + MOZ_ASSERT_IF(signalNullChecks && fco.isValid(), canOmitNullCheck); + MOZ_ASSERT_IF(signalNullChecks && !fco.isValid(), !canOmitNullCheck); + + // We should never get a valid FCO if the caller doesn't expect signal + // handling. This simplifies life for the caller. + MOZ_ASSERT_IF(!signalNullChecks, !fco.isValid()); + })); + + // ----------------------------------- + // Actual test/cast logic begins here. + // Check for null. - if (sourceType.isNullable()) { + if (sourceType.isNullable() && !canOmitNullCheck) { branchWasmAnyRefIsNull(true, ref, nullLabel); } @@ -6995,13 +7043,15 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( // not-null. if (destType.isNone()) { finishFail(); - return; + MOZ_ASSERT(!willLoadShape && !willLoadSTV); + return fco; } if (destType.isAny()) { // No further checks for 'any' finishSuccess(); - return; + MOZ_ASSERT(!willLoadShape && !willLoadSTV); + return fco; } // 'type' is now 'eq' or lower, which currently will either be a gc object or @@ -7015,7 +7065,8 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( if (destType.isI31()) { // No further checks for 'i31' finishFail(); - return; + MOZ_ASSERT(!willLoadShape && !willLoadSTV); + return fco; } } @@ -7024,13 +7075,18 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( if (!wasm::RefType::isSubTypeOf(sourceType, wasm::RefType::struct_()) && !wasm::RefType::isSubTypeOf(sourceType, wasm::RefType::array())) { branchWasmAnyRefIsObjectOrNull(false, ref, failLabel); - branchObjectIsWasmGcObject(false, ref, scratch1, failLabel); + + MOZ_ASSERT(willLoadShape); + trackFCO(branchObjectIsWasmGcObject(false, ref, scratch1, failLabel)); + } else { + MOZ_ASSERT(!willLoadShape); } if (destType.isEq()) { // No further checks for 'eq' finishSuccess(); - return; + MOZ_ASSERT(!willLoadSTV); + return fco; } // 'type' is now 'struct', 'array', or a concrete type. (Bottom types and i31 @@ -7041,14 +7097,16 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( // requires loading the object's superTypeVector->typeDef->kind, and checking // that it is correct. - loadPtr(Address(ref, int32_t(WasmGcObject::offsetOfSuperTypeVector())), - scratch1); + MOZ_ASSERT(willLoadSTV); + trackFCO( + loadPtr(Address(ref, int32_t(WasmGcObject::offsetOfSuperTypeVector())), + scratch1)); if (destType.isTypeRef()) { // Concrete type, do superTypeVector check. branchWasmSTVIsSubtype(scratch1, superSTV, scratch2, destType.typeDef(), label, onSuccess); bind(&fallthrough); - return; + return fco; } // Abstract type, do kind check @@ -7060,6 +7118,7 @@ void MacroAssembler::branchWasmRefIsSubtypeAny( branch32(onSuccess ? Assembler::Equal : Assembler::NotEqual, scratch1, Imm32(int32_t(destType.typeDefKind())), label); bind(&fallthrough); + return fco; } void MacroAssembler::branchWasmRefIsSubtypeFunc( @@ -7491,19 +7550,22 @@ void MacroAssembler::convertStringToWasmAnyRef(Register src, Register dest) { orPtr(Imm32(int32_t(wasm::AnyRefTag::String)), src, dest); } -void MacroAssembler::branchObjectIsWasmGcObject(bool isGcObject, Register src, - Register scratch, - Label* label) { +FaultingCodeOffset MacroAssembler::branchObjectIsWasmGcObject(bool isGcObject, + Register src, + Register scratch, + Label* label) { constexpr uint32_t ShiftedMask = (Shape::kindMask() << Shape::kindShift()); constexpr uint32_t ShiftedKind = (uint32_t(Shape::Kind::WasmGC) << Shape::kindShift()); MOZ_ASSERT(src != scratch); - loadPtr(Address(src, JSObject::offsetOfShape()), scratch); + FaultingCodeOffset fco = + loadPtr(Address(src, JSObject::offsetOfShape()), scratch); load32(Address(scratch, Shape::offsetOfImmutableFlags()), scratch); and32(Imm32(ShiftedMask), scratch); branch32(isGcObject ? Assembler::Equal : Assembler::NotEqual, scratch, Imm32(ShiftedKind), label); + return fco; } void MacroAssembler::wasmNewStructObject(Register instance, Register result, diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h @@ -3985,10 +3985,14 @@ class MacroAssembler : public MacroAssemblerSpecific { // Will select one of the other branchWasmRefIsSubtype* functions depending on // destType. See each function for the register allocation requirements, as // well as which registers will be preserved. - void branchWasmRefIsSubtype(Register ref, wasm::MaybeRefType sourceType, - wasm::RefType destType, Label* label, - bool onSuccess, Register superSTV, - Register scratch1, Register scratch2); + // + // If this function returns a valid FaultingCodeOffset, then you must emit a + // trap site to catch the bad cast. It will never return a valid + // FaultingCodeOffset when signalNullChecks is false. + FaultingCodeOffset branchWasmRefIsSubtype( + Register ref, wasm::MaybeRefType sourceType, wasm::RefType destType, + Label* label, bool onSuccess, bool signalNullChecks, Register superSTV, + Register scratch1, Register scratch2); // Perform a subtype check that `ref` is a subtype of `type`, branching to // `label` depending on `onSuccess`. `type` must be in the `any` hierarchy. @@ -4001,10 +4005,14 @@ class MacroAssembler : public MacroAssemblerSpecific { // // `ref` and `superSTV` are preserved. Scratch registers are // clobbered. - void branchWasmRefIsSubtypeAny(Register ref, wasm::RefType sourceType, - wasm::RefType destType, Label* label, - bool onSuccess, Register superSTV, - Register scratch1, Register scratch2); + // + // If this function returns a valid FaultingCodeOffset, then you must emit a + // trap site to catch the bad cast. It will never return a valid + // FaultingCodeOffset when signalNullChecks is false. + FaultingCodeOffset branchWasmRefIsSubtypeAny( + Register ref, wasm::RefType sourceType, wasm::RefType destType, + Label* label, bool onSuccess, bool signalNullChecks, Register superSTV, + Register scratch1, Register scratch2); // Perform a subtype check that `ref` is a subtype of `type`, branching to // `label` depending on `onSuccess`. `type` must be in the `func` hierarchy. @@ -4109,8 +4117,8 @@ class MacroAssembler : public MacroAssemblerSpecific { const Address& dst, Register scratch); // Branch if the object `src` is or is not a WasmGcObject. - void branchObjectIsWasmGcObject(bool isGcObject, Register src, - Register scratch, Label* label); + FaultingCodeOffset branchObjectIsWasmGcObject(bool isGcObject, Register src, + Register scratch, Label* label); // `typeDefData` will be preserved. `instance` and `result` may be the same // register, in which case `instance` will be clobbered. diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp @@ -3749,8 +3749,9 @@ bool BaseCompiler::jumpConditionalWithResults(BranchState* b, RegRef object, masm.branchWasmRefIsSubtype( object, sourceType, destType, &notTaken, - /*onSuccess=*/b->invertBranch ? onSuccess : !onSuccess, regs.superSTV, - regs.scratch1, regs.scratch2); + /*onSuccess=*/b->invertBranch ? onSuccess : !onSuccess, + /*signalNullChecks=*/false, regs.superSTV, regs.scratch1, + regs.scratch2); freeRegistersForBranchIfRefSubtype(regs); // Shuffle stack args. @@ -3764,8 +3765,8 @@ bool BaseCompiler::jumpConditionalWithResults(BranchState* b, RegRef object, masm.branchWasmRefIsSubtype( object, sourceType, destType, b->label, - /*onSuccess=*/b->invertBranch ? !onSuccess : onSuccess, regs.superSTV, - regs.scratch1, regs.scratch2); + /*onSuccess=*/b->invertBranch ? !onSuccess : onSuccess, + /*signalNullChecks=*/false, regs.superSTV, regs.scratch1, regs.scratch2); freeRegistersForBranchIfRefSubtype(regs); return true; } @@ -8943,8 +8944,8 @@ bool BaseCompiler::emitRefTest(bool nullable) { BranchIfRefSubtypeRegisters regs = allocRegistersForBranchIfRefSubtype(destType); masm.branchWasmRefIsSubtype(ref, MaybeRefType(sourceType), destType, &success, - /*onSuccess=*/true, regs.superSTV, regs.scratch1, - regs.scratch2); + /*onSuccess=*/true, /*signalNullChecks=*/false, + regs.superSTV, regs.scratch1, regs.scratch2); freeRegistersForBranchIfRefSubtype(regs); masm.xor32(result, result); @@ -8976,9 +8977,14 @@ bool BaseCompiler::emitRefCast(bool nullable) { Label success; BranchIfRefSubtypeRegisters regs = allocRegistersForBranchIfRefSubtype(destType); - masm.branchWasmRefIsSubtype(ref, MaybeRefType(sourceType), destType, &success, - /*onSuccess=*/true, regs.superSTV, regs.scratch1, - regs.scratch2); + FaultingCodeOffset fco = masm.branchWasmRefIsSubtype( + ref, MaybeRefType(sourceType), destType, &success, + /*onSuccess=*/true, /*signalNullChecks=*/true, regs.superSTV, + regs.scratch1, regs.scratch2); + if (fco.isValid()) { + masm.append(wasm::Trap::BadCast, wasm::TrapMachineInsnForLoadWord(), + fco.get(), trapSiteDesc()); + } freeRegistersForBranchIfRefSubtype(regs); trap(Trap::BadCast); diff --git a/js/src/wasm/WasmSignalHandlers.cpp b/js/src/wasm/WasmSignalHandlers.cpp @@ -999,6 +999,7 @@ bool wasm::MemoryAccessTraps(const RegisterState& regs, uint8_t* addr, case Trap::OutOfBounds: break; case Trap::NullPointerDereference: + case Trap::BadCast: break; # ifdef WASM_HAS_HEAPREG case Trap::IndirectCallToNull: @@ -1021,6 +1022,7 @@ bool wasm::MemoryAccessTraps(const RegisterState& regs, uint8_t* addr, } break; case Trap::NullPointerDereference: + case Trap::BadCast: if ((uintptr_t)addr >= NullPtrGuardSize) { return false; }