tor-browser

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

commit 55dcc8c0cf229f3958e575ab3041a98e93f46a14
parent 0437106b4fa1f47c8562971c72128a755327bb68
Author: Julian Seward <jseward@acm.org>
Date:   Wed, 17 Dec 2025 10:27:23 +0000

Bug 1995106 - Extend wasm stackmap machinery to handle struct- OOL pointers.  r=rhunt,bvisness.

This patch extends wasm's stackmap machinery to track pointers to structure
OOL data areas.  It also removes the current overloading of
LSafepoint::slotsOrElements{Regs_,Slots_} for wasm arrays, and instead handles
array OOL pointers using machinery equivalent to what has been added to handle
struct OOL pointers.

The updated stackmaps are not actually used for anything yet.  The place
where they should in future get used is Instance::updateFrameForMovingGC.

The changes are mostly a straightforward extension of what already exists.

Changes:

* MIR

  - add MIRType::WasmStructData

  - MWasmLoadField constructor: add more assertions

  - FunctionCompiler::{readValueFrom,writeValueTo}StructField: make the OOL
    pointer have type MIRType::WasmStructData

* LIR

  - add LDefinition::Type::WASM_STRUCT_DATA and ::WASM_ARRAY_DATA and generally
    stop using ::SLOTS for array data pointers

  - add LSafepoint::wasm{Struct,Array}Data{Regs_,Slots_}

  - Add StackMap::Kind::StructDataPointer

  - Translate MIRType::Wasm{Struct,Array}Data to
    LDefinition::Type::WASM_{STRUCT,ARRAY}_DATA

  - LSafepoint::addGCAllocation and helper routines: create entries in
    LSafepoint::wasm{Struct,Array}Data{Regs_,Slots_} as required

  - CreateStackMapFromLSafepoint: assert that safepoint.slotsOrElements* is
    empty (because this is wasm-only) and instead work with
    LSafepoint::wasm{AnyRef,Struct,Array}Data{Regs_,Slots_} to create stackmaps

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

Diffstat:
Mjs/src/jit/CodeGenerator.cpp | 64++++++++++++++++++++++++++++++++++++++++++++++++++++------------
Mjs/src/jit/IonAnalysis.cpp | 1+
Mjs/src/jit/IonTypes.h | 19+++++++++++--------
Mjs/src/jit/LIR.cpp | 8++++++++
Mjs/src/jit/LIR.h | 87++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
Mjs/src/jit/MIR-wasm.h | 10++++++++++
Mjs/src/wasm/WasmGC.h | 12+++++++++---
Mjs/src/wasm/WasmIonCompile.cpp | 4++--
8 files changed, 171 insertions(+), 34 deletions(-)

diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp @@ -4166,6 +4166,8 @@ void CodeGenerator::visitMoveGroup(LMoveGroup* group) { case LDefinition::OBJECT: case LDefinition::SLOTS: case LDefinition::WASM_ANYREF: + case LDefinition::WASM_STRUCT_DATA: + case LDefinition::WASM_ARRAY_DATA: #ifdef JS_NUNBOX32 case LDefinition::TYPE: case LDefinition::PAYLOAD: @@ -11028,6 +11030,7 @@ void CodeGenerator::emitWasmValueLoad(InstructionWithMaybeTrapSite* ins, break; case MIRType::Pointer: case MIRType::WasmAnyRef: + case MIRType::WasmStructData: case MIRType::WasmArrayData: MOZ_ASSERT(wideningOp == MWideningOp::None); fco = masm.loadPtr(addr, dst.gpr()); @@ -16978,11 +16981,18 @@ static bool CreateStackMapFromLSafepoint(LSafepoint& safepoint, // This is the total number of bytes covered by the map. const size_t nTotalBytes = nNonRegisterBytes + nRegisterBytes; -#ifndef DEBUG - bool needStackMap = !(safepoint.wasmAnyRefRegs().empty() && - safepoint.wasmAnyRefSlots().empty() && - safepoint.slotsOrElementsSlots().empty()); + // This stackmap/safepoint is for a wasm frame, so there should be no + // slotsOrElements-style roots. + MOZ_RELEASE_ASSERT(safepoint.slotsOrElementsSlots().empty()); + MOZ_RELEASE_ASSERT(safepoint.slotsOrElementsRegs().empty()); +#ifndef DEBUG + bool needStackMap = !safepoint.wasmAnyRefRegs().empty() || + !safepoint.wasmAnyRefSlots().empty() || + !safepoint.wasmStructDataRegs().empty() || + !safepoint.wasmStructDataSlots().empty() || + !safepoint.wasmArrayDataRegs().empty() || + !safepoint.wasmArrayDataSlots().empty(); // There are no references, and this is a non-debug build, so don't bother // building the stackmap. if (!needStackMap) { @@ -17001,10 +17011,26 @@ static bool CreateStackMapFromLSafepoint(LSafepoint& safepoint, // REG DUMP AREA, if any. size_t regDumpWords = 0; const LiveGeneralRegisterSet wasmAnyRefRegs = safepoint.wasmAnyRefRegs(); - const LiveGeneralRegisterSet slotsOrElementsRegs = - safepoint.slotsOrElementsRegs(); + const LiveGeneralRegisterSet wasmStructDataRegs = + safepoint.wasmStructDataRegs(); + const LiveGeneralRegisterSet wasmArrayDataRegs = + safepoint.wasmArrayDataRegs(); + + // These three sets should be disjoint. + MOZ_ASSERT(GeneralRegisterSet::Intersect(wasmAnyRefRegs.set(), + wasmStructDataRegs.set()) + .empty()); + MOZ_ASSERT(GeneralRegisterSet::Intersect(wasmStructDataRegs.set(), + wasmArrayDataRegs.set()) + .empty()); + MOZ_ASSERT(GeneralRegisterSet::Intersect(wasmArrayDataRegs.set(), + wasmAnyRefRegs.set()) + .empty()); const LiveGeneralRegisterSet refRegs(GeneralRegisterSet::Union( - wasmAnyRefRegs.set(), slotsOrElementsRegs.set())); + wasmAnyRefRegs.set(), + GeneralRegisterSet::Union(wasmStructDataRegs.set(), + wasmArrayDataRegs.set()))); + GeneralRegisterForwardIterator refRegsIter(refRegs); switch (safepoint.wasmSafepointKind()) { case WasmSafepointKind::LirCall: @@ -17024,8 +17050,10 @@ static bool CreateStackMapFromLSafepoint(LSafepoint& safepoint, if (wasmAnyRefRegs.has(reg)) { stackMap->set(index, wasm::StackMap::AnyRef); + } else if (wasmStructDataRegs.has(reg)) { + stackMap->set(index, wasm::StackMap::StructDataPointer); } else { - MOZ_ASSERT(slotsOrElementsRegs.has(reg)); + MOZ_ASSERT(wasmArrayDataRegs.has(reg)); stackMap->set(index, wasm::StackMap::ArrayDataPointer); } } @@ -17053,8 +17081,10 @@ static bool CreateStackMapFromLSafepoint(LSafepoint& safepoint, if (wasmAnyRefRegs.has(reg)) { stackMap->set(offsetFromBottom, wasm::StackMap::AnyRef); + } else if (wasmStructDataRegs.has(reg)) { + stackMap->set(offsetFromBottom, wasm::StackMap::StructDataPointer); } else { - MOZ_ASSERT(slotsOrElementsRegs.has(reg)); + MOZ_ASSERT(wasmArrayDataRegs.has(reg)); stackMap->set(offsetFromBottom, wasm::StackMap::ArrayDataPointer); } } @@ -17095,11 +17125,21 @@ static bool CreateStackMapFromLSafepoint(LSafepoint& safepoint, } } - // Track array data pointers on the stack - const LSafepoint::SlotList& slots = safepoint.slotsOrElementsSlots(); - for (SafepointSlotEntry slot : slots) { + // Track struct data pointers on the stack + for (SafepointSlotEntry slot : safepoint.wasmStructDataSlots()) { MOZ_ASSERT(slot.stack); + // It's a slot in the body allocation, so .slot is interpreted + // as an index downwards from the Frame* + MOZ_ASSERT(slot.slot <= nBodyBytes); + uint32_t offsetInBytes = nBodyBytes - slot.slot; + MOZ_ASSERT(offsetInBytes % sizeof(void*) == 0); + stackMap->set(regDumpWords + offsetInBytes / sizeof(void*), + wasm::StackMap::Kind::StructDataPointer); + } + // Track array data pointers on the stack + for (SafepointSlotEntry slot : safepoint.wasmArrayDataSlots()) { + MOZ_ASSERT(slot.stack); // It's a slot in the body allocation, so .slot is interpreted // as an index downwards from the Frame* MOZ_ASSERT(slot.slot <= nBodyBytes); diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp @@ -3456,6 +3456,7 @@ static bool IsResumableMIRType(MIRType type) { case MIRType::Elements: case MIRType::Pointer: case MIRType::WasmAnyRef: + case MIRType::WasmStructData: case MIRType::WasmArrayData: case MIRType::StackResults: return false; diff --git a/js/src/jit/IonTypes.h b/js/src/jit/IonTypes.h @@ -511,14 +511,15 @@ enum class MIRType : uint8_t { MagicUninitializedLexical, // JS_UNINITIALIZED_LEXICAL magic value. // Types above are specialized. Value, - None, // Invalid, used as a placeholder. - Slots, // A slots vector - Elements, // An elements vector - Pointer, // An opaque pointer that receives no special treatment - WasmAnyRef, // Wasm Ref/AnyRef/NullRef: a raw JSObject* or a raw (void*)0 - WasmArrayData, // A WasmArrayObject data pointer - StackResults, // Wasm multi-value stack result area, which may contain refs - Shape, // A Shape pointer. + None, // Invalid, used as a placeholder. + Slots, // A slots vector + Elements, // An elements vector + Pointer, // An opaque pointer that receives no special treatment + WasmAnyRef, // Wasm Ref/AnyRef/NullRef: a raw JSObject* or a raw (void*)0 + WasmStructData, // A WasmStructObject data pointer (to OOL storage only) + WasmArrayData, // A WasmArrayObject data pointer (to IL or OOL storage) + StackResults, // Wasm multi-value stack result area, which may contain refs + Shape, // A Shape pointer. Last = Shape }; @@ -661,6 +662,8 @@ static inline const char* StringFromMIRType(MIRType type) { return "Pointer"; case MIRType::WasmAnyRef: return "WasmAnyRef"; + case MIRType::WasmStructData: + return "WasmStructData"; case MIRType::WasmArrayData: return "WasmArrayData"; case MIRType::StackResults: diff --git a/js/src/jit/LIR.cpp b/js/src/jit/LIR.cpp @@ -391,6 +391,10 @@ static const char* DefTypeName(LDefinition::Type type) { return "s"; case LDefinition::WASM_ANYREF: return "wr"; + case LDefinition::WASM_STRUCT_DATA: + return "wsd"; + case LDefinition::WASM_ARRAY_DATA: + return "wad"; case LDefinition::FLOAT32: return "f"; case LDefinition::DOUBLE: @@ -725,6 +729,10 @@ bool LSafepoint::addGCAllocation(uint32_t vregId, LDefinition* def, case LDefinition::WASM_ANYREF: return addWasmAnyRef(a); + case LDefinition::WASM_STRUCT_DATA: + return addWasmStructDataPointer(a); + case LDefinition::WASM_ARRAY_DATA: + return addWasmArrayDataPointer(a); #ifdef JS_NUNBOX32 case LDefinition::TYPE: diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h @@ -557,12 +557,15 @@ class LDefinition { GENERAL, // Generic, integer or pointer-width data (GPR). INT32, // int32 data (GPR). OBJECT, // Pointer that may be collected as garbage (GPR). - SLOTS, // Slots/elements/wasm array data pointer that may be moved by minor - // GCs (GPR). - WASM_ANYREF, // Tagged pointer that may be collected as garbage (GPR). - FLOAT32, // 32-bit floating-point value (FPU). - DOUBLE, // 64-bit floating-point value (FPU). - SIMD128, // 128-bit SIMD vector (FPU). + SLOTS, // Slots/elements pointer that may be moved by minor GCs (GPR). + WASM_ANYREF, // Tagged pointer that may be collected as garbage (GPR). + WASM_STRUCT_DATA, // Pointer to wasm struct OOL storage that may be moved + // by minor GCs (GPR). + WASM_ARRAY_DATA, // Pointer to wasm array IL or OOL storage; in the OOL + // case it may be moved by minor GCs (GPR). + FLOAT32, // 32-bit floating-point value (FPU). + DOUBLE, // 64-bit floating-point value (FPU). + SIMD128, // 128-bit SIMD vector (FPU). STACKRESULTS, // A variable-size stack allocation that may contain objects. #ifdef JS_NUNBOX32 // A type virtual register must be followed by a payload virtual @@ -700,10 +703,13 @@ class LDefinition { #endif case MIRType::Slots: case MIRType::Elements: - case MIRType::WasmArrayData: return LDefinition::SLOTS; case MIRType::WasmAnyRef: return LDefinition::WASM_ANYREF; + case MIRType::WasmStructData: + return LDefinition::WASM_STRUCT_DATA; + case MIRType::WasmArrayData: + return LDefinition::WASM_ARRAY_DATA; case MIRType::Pointer: case MIRType::IntPtr: return LDefinition::GENERAL; @@ -751,6 +757,8 @@ inline LStackSlot::Width LStackSlot::width(LDefinition::Type type) { case LDefinition::OBJECT: case LDefinition::SLOTS: case LDefinition::WASM_ANYREF: + case LDefinition::WASM_STRUCT_DATA: + case LDefinition::WASM_ARRAY_DATA: #endif #ifdef JS_NUNBOX32 case LDefinition::TYPE: @@ -764,6 +772,8 @@ inline LStackSlot::Width LStackSlot::width(LDefinition::Type type) { case LDefinition::OBJECT: case LDefinition::SLOTS: case LDefinition::WASM_ANYREF: + case LDefinition::WASM_STRUCT_DATA: + case LDefinition::WASM_ARRAY_DATA: #endif #ifdef JS_PUNBOX64 case LDefinition::BOX: @@ -1641,7 +1651,6 @@ class LSafepoint : public TempObject { // The subset of liveRegs which contains pointers to slots/elements. LiveGeneralRegisterSet slotsOrElementsRegs_; - // List of slots which have slots/elements pointers. SlotList slotsOrElementsSlots_; @@ -1650,6 +1659,16 @@ class LSafepoint : public TempObject { // List of slots which have wasm::AnyRef's. SlotList wasmAnyRefSlots_; + // The subset of liveRegs which contains wasm struct data (OOL) pointers. + LiveGeneralRegisterSet wasmStructDataRegs_; + // List of slots which have have wasm struct data (OOL) pointers. + SlotList wasmStructDataSlots_; + + // The subset of liveRegs which contains wasm array data (IL or OOL) pointers. + LiveGeneralRegisterSet wasmArrayDataRegs_; + // List of slots which have have wasm array data (IL or OOL) pointers. + SlotList wasmArrayDataSlots_; + // Wasm only: with what kind of instruction is this LSafepoint associated? WasmSafepointKind wasmSafepointKind_; @@ -1670,12 +1689,15 @@ class LSafepoint : public TempObject { public: void assertInvariants() { - // Every register in valueRegs and gcRegs should also be in liveRegs. + // Every register in valueRegs, gcRegs, wasmAnyRefRegs, wasmStructDataRegs + // and wasmArrayDataRegs should also be in liveRegs. #ifndef JS_NUNBOX32 MOZ_ASSERT((valueRegs().bits() & ~liveRegs().gprs().bits()) == 0); #endif MOZ_ASSERT((gcRegs().bits() & ~liveRegs().gprs().bits()) == 0); MOZ_ASSERT((wasmAnyRefRegs().bits() & ~liveRegs().gprs().bits()) == 0); + MOZ_ASSERT((wasmStructDataRegs().bits() & ~liveRegs().gprs().bits()) == 0); + MOZ_ASSERT((wasmArrayDataRegs().bits() & ~liveRegs().gprs().bits()) == 0); } explicit LSafepoint(TempAllocator& alloc) @@ -1689,6 +1711,8 @@ class LSafepoint : public TempObject { #endif slotsOrElementsSlots_(alloc), wasmAnyRefSlots_(alloc), + wasmStructDataSlots_(alloc), + wasmArrayDataSlots_(alloc), wasmSafepointKind_(WasmSafepointKind::LirCall), framePushedAtStackMapBase_(0) { assertInvariants(); @@ -1735,6 +1759,49 @@ class LSafepoint : public TempObject { assertInvariants(); return true; } + + SlotList& wasmStructDataSlots() { return wasmStructDataSlots_; } + LiveGeneralRegisterSet wasmStructDataRegs() const { + return wasmStructDataRegs_; + } + [[nodiscard]] bool addWasmStructDataSlot(bool stack, uint32_t slot) { + bool result = wasmStructDataSlots_.append(SlotEntry(stack, slot)); + if (result) { + assertInvariants(); + } + return result; + } + [[nodiscard]] bool addWasmStructDataPointer(LAllocation alloc) { + if (alloc.isMemory()) { + return addWasmStructDataSlot(alloc.isStackSlot(), alloc.memorySlot()); + } + MOZ_ASSERT(alloc.isGeneralReg()); + wasmStructDataRegs_.addUnchecked(alloc.toGeneralReg()->reg()); + assertInvariants(); + return true; + } + + SlotList& wasmArrayDataSlots() { return wasmArrayDataSlots_; } + LiveGeneralRegisterSet wasmArrayDataRegs() const { + return wasmArrayDataRegs_; + } + [[nodiscard]] bool addWasmArrayDataSlot(bool stack, uint32_t slot) { + bool result = wasmArrayDataSlots_.append(SlotEntry(stack, slot)); + if (result) { + assertInvariants(); + } + return result; + } + [[nodiscard]] bool addWasmArrayDataPointer(LAllocation alloc) { + if (alloc.isMemory()) { + return addWasmArrayDataSlot(alloc.isStackSlot(), alloc.memorySlot()); + } + MOZ_ASSERT(alloc.isGeneralReg()); + wasmArrayDataRegs_.addUnchecked(alloc.toGeneralReg()->reg()); + assertInvariants(); + return true; + } + bool hasSlotsOrElementsPointer(LAllocation alloc) const { if (alloc.isGeneralReg()) { return slotsOrElementsRegs().has(alloc.toGeneralReg()->reg()); @@ -2030,6 +2097,8 @@ bool LDefinition::isSafepointGCType(LNode* ins) const { case LDefinition::OBJECT: case LDefinition::SLOTS: case LDefinition::WASM_ANYREF: + case LDefinition::WASM_STRUCT_DATA: + case LDefinition::WASM_ARRAY_DATA: #ifdef JS_NUNBOX32 case LDefinition::TYPE: case LDefinition::PAYLOAD: diff --git a/js/src/jit/MIR-wasm.h b/js/src/jit/MIR-wasm.h @@ -2554,6 +2554,7 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data { // "if you want to widen the value when it is loaded, the destination type // must be Int32". MOZ_ASSERT_IF(wideningOp != MWideningOp::None, type == MIRType::Int32); + // Check the alias set is one of the expected kinds. MOZ_ASSERT( aliases.flags() == AliasSet::Load(AliasSet::WasmStructOutlineDataPointer).flags() || @@ -2568,6 +2569,14 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data { aliases.flags() == AliasSet::Load(AliasSet::WasmArrayDataArea).flags() || aliases.flags() == AliasSet::Load(AliasSet::Any).flags()); + // Check the alias set is consistent with the result type. + MOZ_ASSERT( + (aliases.flags() == + AliasSet::Load(AliasSet::WasmStructOutlineDataPointer).flags()) == + (type == MIRType::WasmStructData)); + MOZ_ASSERT((aliases.flags() == + AliasSet::Load(AliasSet::WasmArrayDataPointer).flags()) == + (type == MIRType::WasmArrayData)); setResultType(type); if (maybeTrap_) { // This is safe, but see bug 1992059 for associated details. @@ -2787,6 +2796,7 @@ class MWasmStoreFieldRef : public MAryInstruction<4>, MOZ_ASSERT(base->type() == TargetWordMIRType() || base->type() == MIRType::Pointer || base->type() == MIRType::WasmAnyRef || + base->type() == MIRType::WasmStructData || base->type() == MIRType::WasmArrayData); MOZ_ASSERT(offset <= INT32_MAX); MOZ_ASSERT(value->type() == MIRType::WasmAnyRef); diff --git a/js/src/wasm/WasmGC.h b/js/src/wasm/WasmGC.h @@ -152,9 +152,13 @@ struct StackMap final { POD = 0, AnyRef = 1, - // The data pointer for a WasmArrayObject, which may be an interior pointer - // to the object itself. See WasmArrayObject::inlineStorage. - ArrayDataPointer = 2, + // The data pointer for a WasmStructObject that requires OOL storage. + StructDataPointer = 2, + + // The data pointer for a WasmArrayObject, which is either an interior + // pointer to the object itself, or a pointer to OOL storage managed by + // BufferAllocator. See WasmArrayObject::data_/inlineStorage. + ArrayDataPointer = 3, Limit, }; @@ -210,6 +214,8 @@ struct StackMap final { inline void set(uint32_t index, Kind kind) { MOZ_ASSERT(index < header.numMappedWords); MOZ_ASSERT(kind < Kind::Limit); + // Because we don't zero out the field before writing it .. + MOZ_ASSERT(get(index) == (Kind)0); uint32_t wordIndex = index / mappedWordsPerBitmapElem; uint32_t wordOffset = index % mappedWordsPerBitmapElem * bitsPerMappedWord; bitmap[wordIndex] |= (kind << wordOffset); diff --git a/js/src/wasm/WasmIonCompile.cpp b/js/src/wasm/WasmIonCompile.cpp @@ -5138,7 +5138,7 @@ class FunctionCompiler { auto* loadDataPointer = MWasmLoadField::New( alloc(), structObject, nullptr, WasmStructObject::offsetOfOutlineData(), mozilla::Nothing(), - MIRType::Pointer, MWideningOp::None, + MIRType::WasmStructData, MWideningOp::None, AliasSet::Load(AliasSet::WasmStructOutlineDataPointer), mozilla::Some(trapSiteDesc())); if (!loadDataPointer) { @@ -5190,7 +5190,7 @@ class FunctionCompiler { auto* loadDataPointer = MWasmLoadField::New( alloc(), structObject, nullptr, WasmStructObject::offsetOfOutlineData(), mozilla::Nothing(), - MIRType::Pointer, MWideningOp::None, + MIRType::WasmStructData, MWideningOp::None, AliasSet::Load(AliasSet::WasmStructOutlineDataPointer), mozilla::Some(trapSiteDesc())); if (!loadDataPointer) {