1571230 - WasmTextToBinary: does not accept a infinity value

Status: VERIFIED FIXED

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

Description

[Volker Berlin]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36

Steps to reproduce:

I want to use the follow text syntax with the function WasmTextToBinary to push an infinity value on the stack:

f64.const inf

This syntax is working with the Wabt tool.

Actual results:

I receive the error message:
SyntaxError: wasm text error: parsing wasm text at 693:17

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks for filing a bug: the correct token is "infinity", not "inf". It's trivial to accept it too, so I'll make a patch.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1571230: Accept "inf" for infinity in wasmTextToBinary; r?jseward

Comment 3

[Volker Berlin]

WABT does not accept "infinity". This is the value that I had expect first.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Ah, that's right. Let's all keep "inf" then, and close this bug; unless WABT does otherwise any time soon.

Comment 5

[Volker Berlin]

The fix does not work for me. I have test https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/jsshell-win64.zip from 08-Aug-2019 12:08.

I have also try the suggested "infinity" which is incompatible with WABT and it work. Look like your fix has no effect.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Oops, I think there was some miscommunication here: I've just not landed the fix, since you said that WABT didn't support it, and we try to be aligned with wabt in general.

So, just to be clear, are you saying that:

• using inf doesn't work for you? (Can you use it, or is there a particular reason why you can't?)
• or you tried to use infinity, but since we never landed the patch (for the reason that WABT doesn't implement it), it had not effect and didn't work?

Answering this will help us figure out if this is needed.

Comment 7

[Volker Berlin]

Sorry for my bad communication. What I have means:

• First I have try infinity with WABT but it does not work. This seems the natural syntax for me. That I can understand that SpiderMonkey support it.
• After asking the community I have try inf and this work with WABT without any problems. But it does not work with SpiderMonkey.

If both (WABT and SM) should be compatible then inf is the correct syntax. You can try it self in the online demo at https://webassembly.github.io/wabt/demo/wat2wasm/

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Duh, right, I was thinking backwards, sorry (I thought inf was implemented but not infinity, while the reverse is true in Spidermonkey). Well, we can just reuse my previous patch, at least :)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a2d23bd5015
Accept "inf" for infinity in wasmTextToBinary; r=jseward

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4a2d23bd5015

Comment 11

[Volker Berlin]

It look like that you have only fix the syntax "+inf" and "-inf" (line 909) but not the simple syntax "inf" without sign in (line 2092).

Comment 12

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1571230: Allow inf in wasmTextToBinary without a sign prefix; r?jseward

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2841c9e7397
Allow inf in wasmTextToBinary without a sign prefix; r=jseward

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 15

[Volker Berlin]

Works for me now. Thanks.