MWasmDerivedPointer denotes a base-plus-constant-offset value. In the
presence of wasm-gc types, this is dangerous when the base value is reftyped.
This is because MWasmDerivedPointer makes it possible to construct values that
we have an obligation to modify at GC time, yet it does not give us any
mechanism to do so, since they are not valid object pointers from the GC's
point of view. MWasmDerivedIndexPointer is similarly dangerous.
Hence if these values are live across a GC, they will be invalid after the GC,
and hence cause a crash if used after the GC. This has been observed to
happen.
A reasonable fix seems to be to restrict their base types to be non-ref. For
cases where the base type is ref, instead pass around a pair of the base and
the (constant) offset, to the place where the access actually happens. In
effect what this does is to make it impossible to create these "sort-of but
not really a ref" values.
The idea is simple in theory but gives rise to quite a large patch. Main
changes are:
* MWasmDerivedPointer::New, MWasmDerivedIndexPointer::New: assert the base
type to exclude reftypes, and add comments.
* change from a single `address` to a `base + constant offset` formulation, in:
- wasm::EmitWasmPreBarrierGuard
- wasm::EmitWasmPreBarrierCall
In places where we have a single `address` but know it is "safe", because
either it's non-reftyped, or isn't live across any potential GC event, but
we need to use a `base + constant offset` formulation, zero is passed for
the offset.
* add a `base + constant offset` formulation for the following, but leave the
existing `address`-only formulation in place:
- Instance::postBarrierPreciseWithOffset
* remove FunctionCompiler::writeGcValueAtAddress and route all such traffic
through FunctionCompiler::writeGcValueAtBasePlusOffset
* For constructors of MWasmStoreRef, MWasmLoadField, MWasmLoadFieldKA,
MWasmStoreFieldKA, MWasmStoreFieldRefKA, accept an offset of type size_t,
but restrict it to 0 .. 2^31-1 (a pre-existing limitation from
MWasmDerivedPointer) so as to ensure that it is a valid offset to give to a
macroassembler `Address` constructor. However, actually store such values
in the MIR as a uint32_t so as not to waste space.
* [ridealong] add some missing `getExtras` (debug-printing) methods for
MWasmStoreRef MWasmLoadField MWasmLoadFieldKA MWasmStoreFieldKA
MWasmStoreFieldRefKA.
* [ridealong] some minor rearranging of access method orderings for
MWasmLoadField et al to make them more consistent.
Differential Revision: https://phabricator.services.mozilla.com/D166899
An explanation of the Firefox Source Code Directory Structure and links to
project pages with documentation can be found at:
https://firefox-source-docs.mozilla.org/contributing/directory_structure.html
For information on how to build Firefox from the source code and create the patch see:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
If you have a question about developing Firefox, and can't find the solution
on https://firefox-source-docs.mozilla.org/, you can try asking your question on Matrix at chat.mozilla.org in `Introduction` (https://chat.mozilla.org/#/room/#introduction:mozilla.org) channel.
Nightly development builds can be downloaded from:
https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/
- or -
https://www.mozilla.org/firefox/channel/desktop/#nightly
Keep in mind that nightly builds, which are used by Firefox developers for
testing, may be buggy.