1478982 - Support immutable fields in TypedObjects (to support Wasm)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Lars T Hansen [:lth]]

At the moment, all TO fields are mutable, even in opaque objects.  For Wasm, we want immutable fields for two purposes:

- to ensure that wasm objects that are exposed to JS are not compromised
  by JS setting their immutable fields

- to serve as a stopgap for pointer fields while we evolve TO and wasm together;
  namely, we make all pointer fields of wasm objects immutable to JS so that
  we do not have to worry about type constraints yet.

(TO does not appear to support type constraints at all, indeed it has no "typed reference" type except for the built-in object/any/string.  In the TO syntax, a field x:T where T is some user-defined struct type creates an in-line field of type T, not reference to T.

To fully support Wasm, TO needs a notion of typed reference and it must handle circularly referring types.  For now, the wasm interface to TO uses the Object type.)

There are two obvious ways to add support for immutable fields:

- introduce new primitive types that are "immutable", eg, in addition to the
  current TypedObject.int32 there might be TypedObject.const_int32, say.

- introduce immutability on the side, hidden within the implementation, so
  that wasm gets to specify immutable fields but JS does not.

The latter has fewer ramifications so I've gone with that.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1478982-to-immutable-fields.patch

Probably complete enough.

Comment 2

[Lars T Hansen [:lth]]

Comment on attachment 8995502 [details] [diff] [review]
bug1478982-to-immutable-fields.patch

Till, what do you think of the approach here?  See comment 0 about alternatives.  In particular, if we think TO will evolve to incorporate immutable fields we could do something slightly cleaner, eg introducing TypedObject.const_int32 types.  I would have to redo this patch but I don't think it would be very difficult.

Comment 3

[Till Schneidereit [:till]]

Comment on attachment 8995502 [details] [diff] [review]
bug1478982-to-immutable-fields.patch

Review of attachment 8995502 [details] [diff] [review]:
-----------------------------------------------------------------

TOs will definitely need to gain support for both typed references and immutable fields. I think it's ok to land this as-is for now though: it'll be a bit until we have the TO spec stuff in place, and there's no need to block on that given that you have the patch written.

As for how to express immutability in TOs, I'd vastly prefer not to do it as a second set of types, but as something composing with types. An int32 read from a slot won't retain its immutability just because the slot was immutable, right? (That's obviously not relevant for this patch, note.)

Also, I clearly should've reviewed this and the patch in bug 1479718 in the right order :) I'm still ok with landing things as they are now, given that they'll have to change in more fundamental ways anyway. So again, r=me with or without nit addressed, just in case :)

::: js/src/builtin/TypedObject.cpp
@@ +1107,5 @@
> +StructTypeDescr::fieldIsMutable(size_t index) const
> +{
> +    ArrayObject& fieldMuts = fieldInfoObject(JS_DESCR_SLOT_STRUCT_FIELD_MUTS);
> +    MOZ_ASSERT(index < fieldMuts.getDenseInitializedLength());
> +    return AssertedCast<bool>(fieldMuts.getDenseElement(index).toBoolean());

Isn't this cast guaranteed to be a nop because toBoolean already returns a bool?

::: js/src/builtin/TypedObjectConstants.h
@@ +64,5 @@
>  // Slots on struct type objects
> +#define JS_DESCR_SLOT_STRUCT_FIELD_NAMES 9
> +#define JS_DESCR_SLOT_STRUCT_FIELD_TYPES 10
> +#define JS_DESCR_SLOT_STRUCT_FIELD_OFFSETS 11
> +#define JS_DESCR_SLOT_STRUCT_FIELD_MUTS  12

Ideally we should be able to combine some of these into a bitset, but that can happen later.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e4eec1a2fee
Allow TypedObject fields to be flagged immutable. r=till

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3e4eec1a2fee