search

WebAssembly / spec

WebAssembly specification, reference interpreter, and test suite.
https://webassembly.github.io/spec/
Other
3.13k stars 445 forks source link

[test] Import v128 global #1597

Closed rossberg closed 1 year ago

rossberg commented 1 year ago

Adds a simple test for ex/importing a global of type v128.

@ajklein, @jakobkummerow, this makes V8's InstanceBuilder crash, presumably because v128 isn't handled here (tested with node 18.14.0, the CI failure is caused by that as well).

ajklein commented 1 year ago

Thanks, filed https://crbug.com/v8/13732 to track the V8 crash.

rossberg commented 1 year ago

Thanks. @jakobkummerow, do you happen to know the rough turn-around time for a V8 fix to make it downstream into node? I'm hesitant to land this PR right now, since I'd have to disable running JS tests in CI to make it green.

jakobkummerow commented 1 year ago

do you happen to know the rough turn-around time for a V8 fix to make it downstream into node?

I don't. I suspect it could take a while. As a data point: the latest Node releases right now ship V8 10.8.168.25 dated 2022-12-20.

The V8 fix is awaiting review: https://chromium-review.googlesource.com/c/v8/v8/+/4239841, it'll be in the V8 11.2 branch.

As a short-term workaround, you could make the global mutable (with a TODO comment to add a test for an immutable global as well, once the V8 fix has propagated far enough).

rossberg commented 1 year ago

As a short-term workaround, you could make the global mutable (with a TODO comment to add a test for an immutable global as well, once the V8 fix has propagated far enough).

Done.

tomstuart commented 1 year ago

It’s surprising that this appears in a test/core/*.wast test script instead of test/core/simd/*.wast. All other occurrences of v128 appear in test/core/simd/*.wast.

rossberg commented 1 year ago

Good point, will fix.