commit ebe50f018f9bf1db5f31daacb64368795ab6e279
parent cafbc725a928519a19c2e20c26a17a44facebc8e
Author: Julian Seward <jseward@acm.org>
Date: Thu, 2 Oct 2025 11:07:34 +0000
Bug 1986850 - wasm-gc: Improve precision of alias-checking for structure loads/stores. r=bvisness,rhunt.
This improves alias checking for wasm wasm struct loads/stores, and has some
ridealong cleanups for other wasm alias-set markings.
* new method RefType::valuesMightAlias
* new method MWasmLoadField::mightAlias,
with helper StructTypesMightBeRelatedByInheritance,
which adds some better checks for struct aliasing
* ridealongs:
- MWasmLoadField: set movable if there's no trap-site info
- MWasmLoadField::congruentTo: tidy up (no functional change)
* ridealongs: MWasmNeg, MWasmStackArg, WasmRegisterResult,
WasmFloatRegisterResult, WasmBuiltinFloatRegisterResult,
WasmRegister64Result, MWasmRefAsNonNull, WasmRefCastAbstract,
WasmRefCastConcrete: add missing empty-alias-set annotations,
as detected during testing
* ridealong: MIROps.yaml: WasmBoundsCheckRange32: add missing `alias_set` of
none and missing `guard` marking (the latter an outright bug)
Differential Revision: https://phabricator.services.mozilla.com/D264039
Diffstat:
4 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/js/src/jit/MIR-wasm.h b/js/src/jit/MIR-wasm.h
@@ -579,6 +579,7 @@ class MWasmNeg : public MUnaryInstruction, public NoTypePolicy::Data {
public:
INSTRUCTION_HEADER(WasmNeg)
TRIVIAL_NEW_WRAPPERS
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
ALLOW_CLONE(MWasmNeg)
};
@@ -1635,6 +1636,10 @@ class MWasmParameter : public MNullaryInstruction {
public:
INSTRUCTION_HEADER(WasmParameter)
TRIVIAL_NEW_WRAPPERS
+ // MWasmParameter has no getAliasSet routine. Hence it acquires the default
+ // aliases-everything setting. This doesn't matter in practice because these
+ // nodes only appear at the start of the function's entry block, and in any
+ // case they are not marked as movable.
ABIArg abi() const { return abi_; }
};
@@ -1681,6 +1686,9 @@ class MWasmStackArg : public MUnaryInstruction, public NoTypePolicy::Data {
uint32_t spOffset() const { return spOffset_; }
void incrementOffset(uint32_t inc) { spOffset_ += inc; }
+ AliasSet getAliasSet() const override {
+ return AliasSet::Store(AliasSet::Flag::Any);
+ }
ALLOW_CLONE(MWasmStackArg)
};
@@ -1710,6 +1718,7 @@ class MWasmRegisterResult : public MWasmResultBase<Register> {
public:
INSTRUCTION_HEADER(WasmRegisterResult)
TRIVIAL_NEW_WRAPPERS
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
};
class MWasmFloatRegisterResult : public MWasmResultBase<FloatRegister> {
@@ -1719,6 +1728,7 @@ class MWasmFloatRegisterResult : public MWasmResultBase<FloatRegister> {
public:
INSTRUCTION_HEADER(WasmFloatRegisterResult)
TRIVIAL_NEW_WRAPPERS
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
};
class MWasmBuiltinFloatRegisterResult : public MWasmResultBase<FloatRegister> {
@@ -1730,6 +1740,7 @@ class MWasmBuiltinFloatRegisterResult : public MWasmResultBase<FloatRegister> {
public:
INSTRUCTION_HEADER(WasmBuiltinFloatRegisterResult)
TRIVIAL_NEW_WRAPPERS
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
bool hardFP() const { return hardFP_; }
};
@@ -1741,6 +1752,7 @@ class MWasmRegister64Result : public MWasmResultBase<Register64> {
public:
INSTRUCTION_HEADER(WasmRegister64Result)
TRIVIAL_NEW_WRAPPERS
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
};
class MWasmStackResultArea : public MNullaryInstruction {
@@ -2559,7 +2571,10 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data {
aliases.flags() == AliasSet::Load(AliasSet::Any).flags());
setResultType(type);
if (maybeTrap_) {
+ // This is safe, but see bug 1992059 for associated details.
setGuard();
+ } else {
+ setMovable();
}
initWasmRefType(maybeRefType);
}
@@ -2585,14 +2600,15 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data {
return false;
}
const MWasmLoadField* other = ins->toWasmLoadField();
- return ins->isWasmLoadField() && congruentIfOperandsEqual(ins) &&
- offset() == other->offset() &&
+ return congruentIfOperandsEqual(other) && offset() == other->offset() &&
structFieldIndex() == other->structFieldIndex() &&
wideningOp() == other->wideningOp() &&
getAliasSet().flags() == other->getAliasSet().flags() &&
hierarchy() == other->hierarchy();
}
+ virtual AliasType mightAlias(const MDefinition* ins) const override;
+
#ifdef JS_JITSPEW
void getExtras(ExtrasCollector* extras) const override {
char buf[96];
@@ -2954,6 +2970,7 @@ class MWasmRefAsNonNull : public MUnaryInstruction, public NoTypePolicy::Data {
}
MDefinition* foldsTo(TempAllocator& alloc) override;
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
ALLOW_CLONE(MWasmRefAsNonNull)
};
@@ -3061,6 +3078,7 @@ class MWasmRefCastAbstract : public MUnaryInstruction,
INSTRUCTION_HEADER(WasmRefCastAbstract)
TRIVIAL_NEW_WRAPPERS
NAMED_OPERANDS((0, ref))
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
wasm::RefType destType() const { return destType_; };
const wasm::TrapSiteDesc& trapSiteDesc() const { return trapSiteDesc_; }
@@ -3095,6 +3113,7 @@ class MWasmRefCastConcrete : public MBinaryInstruction,
INSTRUCTION_HEADER(WasmRefCastConcrete)
TRIVIAL_NEW_WRAPPERS
NAMED_OPERANDS((0, ref), (1, superSTV))
+ AliasSet getAliasSet() const override { return AliasSet::None(); }
wasm::RefType destType() const { return destType_; };
const wasm::TrapSiteDesc& trapSiteDesc() const { return trapSiteDesc_; }
diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp
@@ -8324,3 +8324,59 @@ bool MInt32ToStringWithBase::congruentTo(const MDefinition* ins) const {
}
return congruentIfOperandsEqual(ins);
}
+
+// Returns `false` if it can be proven that (1) both `mtyA` and `mtyB` are
+// struct types and (2) they are not related by inheritance. Returns `true` in
+// all other cases. `true` is the safe-but-possibly-suboptimal return value.
+static bool StructTypesMightBeRelatedByInheritance(wasm::MaybeRefType mtyA,
+ wasm::MaybeRefType mtyB) {
+ if (!mtyA.isSome() || !mtyB.isSome()) {
+ // The "Track Wasm ref types" pass couldn't establish that both `mtyA` and
+ // `mtyB` are ref types. Give up.
+ return true;
+ }
+
+ wasm::RefType tyA = mtyA.value();
+ wasm::RefType tyB = mtyB.value();
+ if (!tyA.isTypeRef() || !tyA.typeDef()->isStructType() || !tyB.isTypeRef() ||
+ !tyB.typeDef()->isStructType()) {
+ // They aren't both struct types. Give up.
+ return true;
+ }
+
+ // They are both struct types. So they are related by inheritance if one is
+ // a subtype of the other. (Which is also the case if they are the same
+ // type.)
+ return wasm::RefType::valuesMightAlias(tyA, tyB);
+}
+
+MDefinition::AliasType MWasmLoadField::mightAlias(
+ const MDefinition* ins) const {
+ if (!(getAliasSet().flags() & ins->getAliasSet().flags())) {
+ return AliasType::NoAlias;
+ }
+ MOZ_ASSERT(!isEffectful() && ins->isEffectful());
+
+ // Pick off cases where we can easily prove non-aliasing. The idea is that
+ // two struct field accesses can't alias if either they are at different
+ // offsets, or the struct types are unrelated (which implies that the struct
+ // base pointer for one of the accesses could not validly be handed to the
+ // other access).
+ if (ins->isWasmStoreField()) {
+ const MWasmStoreField* store = ins->toWasmStoreField();
+ if (offset() != store->offset() ||
+ !StructTypesMightBeRelatedByInheritance(base()->wasmRefType(),
+ store->base()->wasmRefType())) {
+ return AliasType::NoAlias;
+ }
+ } else if (ins->isWasmStoreFieldRef()) {
+ const MWasmStoreFieldRef* store = ins->toWasmStoreFieldRef();
+ if (offset() != store->offset() ||
+ !StructTypesMightBeRelatedByInheritance(base()->wasmRefType(),
+ store->base()->wasmRefType())) {
+ return AliasType::NoAlias;
+ }
+ }
+
+ return AliasType::MayAlias;
+}
diff --git a/js/src/jit/MIROps.yaml b/js/src/jit/MIROps.yaml
@@ -3957,6 +3957,8 @@
type_policy: none
generate_lir: true
lir_temps: 1
+ alias_set: none
+ guard: true
- name: WasmExtendU32Index
operands:
diff --git a/js/src/wasm/WasmValType.h b/js/src/wasm/WasmValType.h
@@ -444,6 +444,17 @@ class RefType {
static bool isSubTypeOf(RefType subType, RefType superType);
static bool castPossible(RefType sourceType, RefType destType);
+ // If we have two references, one of type `a` and one of type `b`, return
+ // true if there is any possibility that they might point at the same thing.
+ // That can only happen if either they are the same type or if one type is a
+ // subtype of the other. Note, this can only be used for types in the same
+ // hierarchy.
+ static bool valuesMightAlias(RefType a, RefType b) {
+ MOZ_RELEASE_ASSERT(a.hierarchy() == b.hierarchy());
+ // The exact-same-type case is subsumed by `isSubTypeOf`.
+ return RefType::isSubTypeOf(a, b) || RefType::isSubTypeOf(b, a);
+ }
+
// Gets the top of the given type's hierarchy, e.g. Any for structs and
// arrays, and Func for funcs.
RefType topType() const;
