1990202 - Wasm validator disallows name imports longer than 100_000 bytes

Status: RESOLVED FIXED

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

Description

[Martin Kustermann]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36

Steps to reproduce:

We have a dart2wasm compiler and noticed on some programs that JSShell results in a validation error. The root cause of it is that it refuses import names longer than 100_000 bytes.

Normally imported names are surely short.

Though the recent js-string-builtin wasm spec enables a magic import mechanism where one can import JS strings via specially recognized imports.

String constants in Dart applications can exceed 100_000 bytes and cause a wasm validation error. The root cause is due to using of the constant in [0] when decoding import names.

[0] https://github.com/mozilla-firefox/firefox/blob/11e21bedcc15be83ce69dfe01159b5d021196b99/js/src/wasm/WasmConstants.h#L1167

Actual results:

Using a import with name larger than 100_000 bytes results in a wasm validation error.

Expected results:

Looking through the core wasm spec and js-wasm spec (https://webassembly.github.io/spec/js-api/index.html#limits) it's unclear where this 100_000 byte limit comes from.

Can it be that this is not specified?

D8/Chrome doesn't have this issue: It loads & runs the module just fine.

Comment 1

[Ryan Hunt [:rhunt]]

Looks like this limit comes from bug 1324008 and probably predates 1.0. I'm guessing this limit wasn't ever agreed upon.

Do you know if Chrome has any limit here? I see there is a MaxStringSize in their limits file, but it doesn't seem to be used anywhere [1]. I might be missing something though.

[1] https://source.chromium.org/chromium/chromium/src/+/main:v8/src/wasm/wasm-limits.h;l=49;drc=d78d6f646dee0f68dcc59eed2d7de0f910e2fe2a

Comment 2

[Martin Kustermann]

Do you know if Chrome has any limit here?

V8 team says there's no specific limit for the names themselves, the only limits being general limits (being the u32 index, and JS wasm spec that limits module size to 1 GB

I filed the same bug with WebKit, as they also seem to have this 100k limit: https://bugs.webkit.org/show_bug.cgi?id=299393

Comment 3

[Ryan Hunt [:rhunt]]

I'm going to open a spec discussion for this. I'm fine with raising the limit, but feel like it'd still be nice to have a limit on name size.

Comment 4

[Ryan Hunt [:rhunt]]

I filed https://github.com/WebAssembly/design/issues/1574.

Comment 5

[Ryan Hunt [:rhunt]]

The consensus in the above discussion is to not have any JS-API limit on name lengths. We should remove ours. Julien can you do this?

It's basically just removing this limit [1] and updating tests.

Comment 6

[Julien Pages [:jpages]]

Attachment: Bug 1990202 - wasm: Remove the size limit for name imports. r=rhunt

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4ffa205cca52
https://hg.mozilla.org/integration/autoland/rev/cdc32b934643
wasm: Remove the size limit for name imports. r=rhunt

Comment 8

[amarc]

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