diff --git a/js/src/jit-test/tests/wasm/gc/structs.js b/js/src/jit-test/tests/wasm/gc/structs.js index d1d1ffde7921..5b4300eb8768 100644 --- a/js/src/jit-test/tests/wasm/gc/structs.js +++ b/js/src/jit-test/tests/wasm/gc/structs.js @@ -634,3 +634,51 @@ var bad = new Uint8Array([0x00, 0x61, 0x73, 0x6d, assertErrorMessage(() => new WebAssembly.Module(bad), WebAssembly.CompileError, /signature index references non-signature/); + +// Exercise alias-analysis code for struct access +{ + let txt = + `(module + (type $meh (struct)) + (type $hasOOL (struct + ;; In-line storage + (field i64) (field i64) + (field $ILnonref (mut i64)) (field $ILref (mut eqref)) + (field i64) (field i64) (field i64) (field i64) + (field i64) (field i64) (field i64) (field i64) + (field i64) (field i64) (field i64) (field i64) + ;; Out-of-line storage (or maybe it starts earlier, but + ;; definitely not after this point). + (field $OOLnonref (mut i64)) (field $OOLref (mut eqref))) + ) + (func (export "create") (result eqref) + (struct.new $hasOOL + (i64.const 1) (i64.const 2) + (i64.const 9876) (ref.null $meh) + (i64.const 3) (i64.const 4) (i64.const 5) (i64.const 6) + (i64.const 7) (i64.const 8) (i64.const 9) (i64.const 10) + (i64.const 11) (i64.const 12) (i64.const 13) (i64.const 14) + (i64.const 4321) (ref.null $meh)) + ) + ;; Write to an OOL field, then an IL field, then to an OOL field, so + ;; that we can at least check (from inspection of the optimised MIR) + ;; that the GVN+alias analysis causes the OOL block pointer not to be + ;; reloaded for the second OOL write. First for non-ref fields .. + (func (export "threeSetsNonReffy") (param eqref) + (local (ref $hasOOL)) + (local.set 1 (ref.cast $hasOOL (local.get 0))) + (struct.set $hasOOL 16 (local.get 1) (i64.const 1337)) ;; set $OOLnonref + (struct.set $hasOOL 2 (local.get 1) (i64.const 7331)) ;; set $ILnonref + (struct.set $hasOOL 16 (local.get 1) (i64.const 9009)) ;; set $OOLnonref + ) + ;; and the same for ref fields. + (func (export "threeSetsReffy") (param eqref) + (local (ref $hasOOL)) + (local.set 1 (ref.cast $hasOOL (local.get 0))) + (struct.set $hasOOL 17 (local.get 1) (ref.null $meh)) ;; set $OOLref + (struct.set $hasOOL 3 (local.get 1) (ref.null $meh)) ;; set $ILref + (struct.set $hasOOL 17 (local.get 1) (ref.null $meh)) ;; set $OOLref + ) + )`; + let exports = wasmEvalText(txt).exports; +} diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index f67c5a701fa8..0e09f6ed9409 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -361,10 +361,19 @@ class AliasSet { // The fuzzilliHash slot FuzzilliHash = 1 << 19, - Last = FuzzilliHash, + // The WasmStruct::inlineData_[..] storage area + WasmStructInlineDataArea = 1 << 20, + + // The WasmStruct::outlineData_ pointer only + WasmStructOutlineDataPointer = 1 << 21, + + // The malloc'd block that WasmStruct::outlineData_ points at + WasmStructOutlineDataArea = 1 << 22, + + Last = WasmStructOutlineDataArea, Any = Last | (Last - 1), - NumCategories = 20, + NumCategories = 23, // Indicates load or store. Store_ = 1 << 31 @@ -10724,15 +10733,21 @@ enum class MNarrowingOp : uint8_t { None, To16, To8 }; class MWasmLoadField : public MUnaryInstruction, public NoTypePolicy::Data { uint32_t offset_; MWideningOp wideningOp_; + AliasSet aliases_; MWasmLoadField(MDefinition* obj, uint32_t offset, MIRType type, - MWideningOp wideningOp) + MWideningOp wideningOp, AliasSet aliases) : MUnaryInstruction(classOpcode, obj), offset_(offset), - wideningOp_(wideningOp) { + wideningOp_(wideningOp), + aliases_(aliases) { // "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); + MOZ_ASSERT( + aliases.flags() == + AliasSet::Load(AliasSet::WasmStructOutlineDataPointer).flags() || + aliases.flags() == AliasSet::Load(AliasSet::Any).flags()); setResultType(type); } @@ -10743,9 +10758,22 @@ class MWasmLoadField : public MUnaryInstruction, public NoTypePolicy::Data { uint32_t offset() const { return offset_; } MWideningOp wideningOp() const { return wideningOp_; } - - AliasSet getAliasSet() const override { - return AliasSet::Load(AliasSet::Any); + AliasSet getAliasSet() const override { return aliases_; } + bool congruentTo(const MDefinition* ins) const override { + // In the limited case where this insn is used to read + // WasmStructObject::outlineData_ (the field itself, not what it points + // at), we allow commoning up to happen. This is OK because + // WasmStructObject::outlineData_ is readonly for the life of the + // WasmStructObject. + if (!ins->isWasmLoadField()) { + return false; + } + const MWasmLoadField* other = ins->toWasmLoadField(); + return ins->isWasmLoadField() && congruentIfOperandsEqual(ins) && + offset() == other->offset() && wideningOp() == other->wideningOp() && + getAliasSet().flags() == other->getAliasSet().flags() && + getAliasSet().flags() == + AliasSet::Load(AliasSet::WasmStructOutlineDataPointer).flags(); } }; @@ -10762,13 +10790,21 @@ class MWasmLoadField : public MUnaryInstruction, public NoTypePolicy::Data { class MWasmLoadFieldKA : public MBinaryInstruction, public NoTypePolicy::Data { uint32_t offset_; MWideningOp wideningOp_; + AliasSet aliases_; MWasmLoadFieldKA(MDefinition* ka, MDefinition* obj, uint32_t offset, - MIRType type, MWideningOp wideningOp) + MIRType type, MWideningOp wideningOp, AliasSet aliases) : MBinaryInstruction(classOpcode, ka, obj), offset_(offset), - wideningOp_(wideningOp) { + wideningOp_(wideningOp), + aliases_(aliases) { MOZ_ASSERT_IF(wideningOp != MWideningOp::None, type == MIRType::Int32); + MOZ_ASSERT( + aliases.flags() == + AliasSet::Load(AliasSet::WasmStructInlineDataArea).flags() || + aliases.flags() == + AliasSet::Load(AliasSet::WasmStructOutlineDataArea).flags() || + aliases.flags() == AliasSet::Load(AliasSet::Any).flags()); setResultType(type); } @@ -10780,9 +10816,7 @@ class MWasmLoadFieldKA : public MBinaryInstruction, public NoTypePolicy::Data { uint32_t offset() const { return offset_; } MWideningOp wideningOp() const { return wideningOp_; } - AliasSet getAliasSet() const override { - return AliasSet::Load(AliasSet::Any); - } + AliasSet getAliasSet() const override { return aliases_; } }; // Store a value to an object field at a fixed offset from a base pointer. @@ -10795,17 +10829,26 @@ class MWasmStoreFieldKA : public MTernaryInstruction, public NoTypePolicy::Data { uint32_t offset_; MNarrowingOp narrowingOp_; + AliasSet aliases_; MWasmStoreFieldKA(MDefinition* ka, MDefinition* obj, uint32_t offset, - MDefinition* value, MNarrowingOp narrowingOp) + MDefinition* value, MNarrowingOp narrowingOp, + AliasSet aliases) : MTernaryInstruction(classOpcode, ka, obj, value), offset_(offset), - narrowingOp_(narrowingOp) { + narrowingOp_(narrowingOp), + aliases_(aliases) { MOZ_ASSERT(value->type() != MIRType::RefOrNull); // "if you want to narrow the value when it is stored, the source type // must be Int32". MOZ_ASSERT_IF(narrowingOp != MNarrowingOp::None, value->type() == MIRType::Int32); + MOZ_ASSERT( + aliases.flags() == + AliasSet::Store(AliasSet::WasmStructInlineDataArea).flags() || + aliases.flags() == + AliasSet::Store(AliasSet::WasmStructOutlineDataArea).flags() || + aliases.flags() == AliasSet::Store(AliasSet::Any).flags()); } public: @@ -10816,9 +10859,7 @@ class MWasmStoreFieldKA : public MTernaryInstruction, uint32_t offset() const { return offset_; } MNarrowingOp narrowingOp() const { return narrowingOp_; } - AliasSet getAliasSet() const override { - return AliasSet::Store(AliasSet::Any); - } + AliasSet getAliasSet() const override { return aliases_; } }; // Store a reference value to a location which (it is assumed) is within a @@ -10829,11 +10870,20 @@ class MWasmStoreFieldKA : public MTernaryInstruction, // described for MWasmLoadFieldKA above. class MWasmStoreFieldRefKA : public MAryInstruction<4>, public NoTypePolicy::Data { + AliasSet aliases_; + MWasmStoreFieldRefKA(MDefinition* instance, MDefinition* ka, - MDefinition* valueAddr, MDefinition* value) - : MAryInstruction<4>(classOpcode) { + MDefinition* valueAddr, MDefinition* value, + AliasSet aliases) + : MAryInstruction<4>(classOpcode), aliases_(aliases) { MOZ_ASSERT(valueAddr->type() == MIRType::Pointer); MOZ_ASSERT(value->type() == MIRType::RefOrNull); + MOZ_ASSERT( + aliases.flags() == + AliasSet::Store(AliasSet::WasmStructInlineDataArea).flags() || + aliases.flags() == + AliasSet::Store(AliasSet::WasmStructOutlineDataArea).flags() || + aliases.flags() == AliasSet::Store(AliasSet::Any).flags()); initOperand(0, instance); initOperand(1, ka); initOperand(2, valueAddr); @@ -10845,9 +10895,7 @@ class MWasmStoreFieldRefKA : public MAryInstruction<4>, TRIVIAL_NEW_WRAPPERS NAMED_OPERANDS((0, instance), (1, ka), (2, valueAddr), (3, value)) - AliasSet getAliasSet() const override { - return AliasSet::Store(AliasSet::Any); - } + AliasSet getAliasSet() const override { return aliases_; } }; #ifdef FUZZING_JS_FUZZILLI diff --git a/js/src/wasm/WasmGcObject.h b/js/src/wasm/WasmGcObject.h index 9e0f3d53240a..e1e7732ea6df 100644 --- a/js/src/wasm/WasmGcObject.h +++ b/js/src/wasm/WasmGcObject.h @@ -215,7 +215,9 @@ class WasmStructObject : public WasmGcObject { static const JSClass class_; // Owned pointer to a malloc'd block containing out-of-line fields, or - // nullptr if none. + // nullptr if none. Note that MIR alias analysis assumes this is readonly + // for the life of the object; do not change it once the object is created. + // See MWasmLoadObjectField::congruentTo. uint8_t* outlineData_; // The inline (wasm-struct-level) data fields. This must be a multiple of diff --git a/js/src/wasm/WasmIonCompile.cpp b/js/src/wasm/WasmIonCompile.cpp index 6c6931ac09fa..eb4325f8f49e 100644 --- a/js/src/wasm/WasmIonCompile.cpp +++ b/js/src/wasm/WasmIonCompile.cpp @@ -3087,9 +3087,9 @@ class FunctionCompiler { const TagOffsetVector& offsets = tagType->argOffsets_; // Get the data pointer from the exception object - auto* data = MWasmLoadField::New(alloc(), exception, - WasmExceptionObject::offsetOfData(), - MIRType::Pointer, MWideningOp::None); + auto* data = MWasmLoadField::New( + alloc(), exception, WasmExceptionObject::offsetOfData(), + MIRType::Pointer, MWideningOp::None, AliasSet::Load(AliasSet::Any)); if (!data) { return false; } @@ -3102,9 +3102,9 @@ class FunctionCompiler { // Load each value from the data pointer for (size_t i = 0; i < params.length(); i++) { - auto* load = - MWasmLoadFieldKA::New(alloc(), exception, data, offsets[i], - params[i].toMIRType(), MWideningOp::None); + auto* load = MWasmLoadFieldKA::New( + alloc(), exception, data, offsets[i], params[i].toMIRType(), + MWideningOp::None, AliasSet::Load(AliasSet::Any)); if (!load || !values->append(load)) { return false; } @@ -3213,9 +3213,9 @@ class FunctionCompiler { } // Load the data pointer from the object - auto* data = MWasmLoadField::New(alloc(), exception, - WasmExceptionObject::offsetOfData(), - MIRType::Pointer, MWideningOp::None); + auto* data = MWasmLoadField::New( + alloc(), exception, WasmExceptionObject::offsetOfData(), + MIRType::Pointer, MWideningOp::None, AliasSet::Load(AliasSet::Any)); if (!data) { return false; } @@ -3229,7 +3229,8 @@ class FunctionCompiler { if (!type.isRefRepr()) { auto* store = MWasmStoreFieldKA::New(alloc(), exception, data, offset, - argValues[i], MNarrowingOp::None); + argValues[i], MNarrowingOp::None, + AliasSet::Store(AliasSet::Any)); if (!store) { return false; } @@ -3245,9 +3246,9 @@ class FunctionCompiler { curBlock_->add(fieldAddr); // Load the previous value - auto* prevValue = - MWasmLoadFieldKA::New(alloc(), exception, data, offset, - type.toMIRType(), MWideningOp::None); + auto* prevValue = MWasmLoadFieldKA::New( + alloc(), exception, data, offset, type.toMIRType(), MWideningOp::None, + AliasSet::Load(AliasSet::Any)); if (!prevValue) { return false; } @@ -3255,7 +3256,8 @@ class FunctionCompiler { // Store the new value auto* store = MWasmStoreFieldRefKA::New( - alloc(), instancePointer_, exception, fieldAddr, argValues[i]); + alloc(), instancePointer_, exception, fieldAddr, argValues[i], + AliasSet::Store(AliasSet::Any)); if (!store) { return false; } @@ -3433,9 +3435,10 @@ class FunctionCompiler { // accordingly. MDefinition* base; if (areaIsOutline) { - auto* load = MWasmLoadField::New(alloc(), structObject, - WasmStructObject::offsetOfOutlineData(), - MIRType::Pointer, MWideningOp::None); + auto* load = MWasmLoadField::New( + alloc(), structObject, WasmStructObject::offsetOfOutlineData(), + MIRType::Pointer, MWideningOp::None, + AliasSet::Load(AliasSet::WasmStructOutlineDataPointer)); if (!load) { return false; } @@ -3448,9 +3451,16 @@ class FunctionCompiler { // The transaction is to happen at `base + areaOffset`, so to speak. // After this point we must ignore `fieldOffset`. + // The alias set denoting the field's location, although lacking a + // Load-vs-Store indication at this point. + AliasSet::Flag fieldAliasSet = areaIsOutline + ? AliasSet::WasmStructOutlineDataArea + : AliasSet::WasmStructInlineDataArea; + if (!fieldType.isRefRepr()) { auto* store = MWasmStoreFieldKA::New(alloc(), structObject, base, - areaOffset, value, fieldNarrowingOp); + areaOffset, value, fieldNarrowingOp, + AliasSet::Store(fieldAliasSet)); if (!store) { return false; } @@ -3477,17 +3487,18 @@ class FunctionCompiler { // results from struct.new, the old value is always zero. So we should // synthesise a suitable zero constant rather than reading it from the // object. See also bug 1799999. - auto* prevValue = - MWasmLoadFieldKA::New(alloc(), structObject, base, 0, - fieldType.toMIRType(), MWideningOp::None); + auto* prevValue = MWasmLoadFieldKA::New( + alloc(), structObject, base, 0, fieldType.toMIRType(), + MWideningOp::None, AliasSet::Load(fieldAliasSet)); if (!prevValue) { return false; } curBlock_->add(prevValue); // Store the new value - auto* store = MWasmStoreFieldRefKA::New(alloc(), instancePointer_, - structObject, base, value); + auto* store = + MWasmStoreFieldRefKA::New(alloc(), instancePointer_, structObject, base, + value, AliasSet::Store(fieldAliasSet)); if (!store) { return false; } @@ -3518,7 +3529,8 @@ class FunctionCompiler { if (areaIsOutline) { auto* loadOOLptr = MWasmLoadField::New( alloc(), structObject, WasmStructObject::offsetOfOutlineData(), - MIRType::Pointer, MWideningOp::None); + MIRType::Pointer, MWideningOp::None, + AliasSet::Load(AliasSet::WasmStructOutlineDataPointer)); if (!loadOOLptr) { return nullptr; } @@ -3531,11 +3543,18 @@ class FunctionCompiler { // The transaction is to happen at `base + areaOffset`, so to speak. // After this point we must ignore `fieldOffset`. + // The alias set denoting the field's location, although lacking a + // Load-vs-Store indication at this point. + AliasSet::Flag fieldAliasSet = areaIsOutline + ? AliasSet::WasmStructOutlineDataArea + : AliasSet::WasmStructInlineDataArea; + MIRType mirType; MWideningOp mirWideningOp; fieldLoadInfoToMIR(fieldType, wideningOp, &mirType, &mirWideningOp); - auto* load = MWasmLoadFieldKA::New(alloc(), structObject, base, areaOffset, - mirType, mirWideningOp); + auto* load = + MWasmLoadFieldKA::New(alloc(), structObject, base, areaOffset, mirType, + mirWideningOp, AliasSet::Load(fieldAliasSet)); if (!load) { return nullptr; }