1472178 - Allow Ref in Globals

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

Same work as bug 1450261, but for any Ref in addition to AnyRef (!).

Some attention will be required when setting value (to take type system into account). (Also please be mindful that the postbarrier code checking if the set location is tenured has never been tested and could be awfully wrong)

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1472178-ref-in-wasm-globals.patch

I think this should actually be good enough, given my existing patches for struct types.  The verifier is already doing the type checking as part of its normal operation with push/pop, so all we need to do is clean up a few places that handle AnyRef but not Ref.  I'll look around for more places and I'll write some more interesting test cases, but I don't expect to find any more problems really.  Input is very welcome.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8994565 [details] [diff] [review]
bug1472178-ref-in-wasm-globals.patch

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

Sweet. So there can't be pointer attributes within a user-defined struct type yet, right? (otherwise i'd expect changes in codegen)
With more tests (including ones for pre- and post-barriers) this would be good to land.

::: js/src/wasm/WasmValidate.cpp
@@ +1763,1 @@
>              return d.fail("expected anyref as type for ref.null");

nit: update error message

Comment 3

[Lars T Hansen [:lth]]

Attachment: bug1472178-ref-in-wasm-globals-v2.patch

With more test cases for import/export and with update to the error message.

Comment 4

[Lars T Hansen [:lth]]

Attachment: bug1472178-ref-in-wasm-globals-v3.patch

The test cases are a little lame because we don't have struct.new et al yet, only null pointers, but they did uncover a bug so they're not worthless.  I've preserved the better test cases from the WIP patches and will land them later with the struct code.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8995227 [details] [diff] [review]
bug1472178-ref-in-wasm-globals-v3.patch

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

Nice.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c47ad4f17e19
Ref types for wasm globals. r=bbouvier

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/c47ad4f17e19