Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing WebAssembly memory using TypedArrays is incorrect on Big Endian systems #25569

Closed
majaha opened this issue Mar 23, 2023 · 9 comments · Fixed by #33960
Closed

Accessing WebAssembly memory using TypedArrays is incorrect on Big Endian systems #25569

majaha opened this issue Mar 23, 2023 · 9 comments · Fixed by #33960
Labels
Content:JS JavaScript docs Content:wasm WebAssembly docs

Comments

@majaha
Copy link
Contributor

majaha commented Mar 23, 2023

MDN URL

https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/Memory

What specific section or headline is this issue about?

Examples

What information was incorrect, unhelpful, or incomplete?

The examples access values from WebAssembly using new Uint32Array:

  const summands = new Uint32Array(memory.buffer);
...
  const values = new Uint32Array(obj.instance.exports.memory.buffer);

However in the Uint32Array docs it says:

The Uint32Array typed array represents an array of 32-bit unsigned integers in the platform byte order. If control over byte order is needed, use DataView instead.

(This is backed up by the ECMAScript spec: Typed Arrays are integer-indexed exotic objects, which have the [[Get]] internal method, which calls IntegerIndexedElementGet, which calls GetValueFromBuffer, which uses the platforms endianness in steps 8 and 9.)

However the WebAssembly docs say:

All values are read and written in little endian byte order.

So it appears that the example will return incorrect results on big-endian systems.

What did you expect to see?

An example using a DataView.setInt32(byteOffset, value, littleEndian) instead.

Exposing endian-ness like this has worrying implications for web page portability.
If this is a real portability pitfall, there should be warnings about it on the WebAssembly.memory and TypedArray pages.

Do you have any supporting links, references, or citations?

I've already seen this pattern proliferating in several other places, such as:

So the amount of code already using this buggy pattern is already probably quite large.

Do you have anything more you want to share?

Perhaps javascript could use a LittleEndianTypedArray or similar. And maybe new Uint32Array(wasmmemory.buffer) should throw a type error, only allowing Uint8Array and Int8Array.

MDN metadata

Page report details
@majaha majaha added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Mar 23, 2023
@github-actions github-actions bot added Content:JS JavaScript docs Content:wasm WebAssembly docs labels Mar 23, 2023
@Josh-Cena
Copy link
Member

This needs edits on both JS and WASM pages, but I'm not sure who we should ask for advice...

@hamishwillee
Copy link
Collaborator

@MendyBerger Something you could advise on?

@MendyBerger
Copy link
Contributor

This sounds like an interesting problem, I'll try to make a bit more research on this.

@Josh-Cena Josh-Cena added needs info Needs more information to review or act on. and removed needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Apr 5, 2023
@MendyBerger
Copy link
Contributor

@sunfishcode would you be willing to advise here?
I'm assuming that the Uint32Array constructor is converting the data into the platforms endianness, but I can't find any documentation to back that up.

@majaha majaha changed the title Accessing WebAssembly memory using TypedArrays is incorrect on Big Endian systems? Accessing WebAssembly memory using TypedArrays is incorrect on Big Endian systems Apr 15, 2023
@Josh-Cena
Copy link
Member

Based on the outcome of https://es.discourse.group/t/is-accessing-webassembly-memory-using-typedarrays-incorrect-on-big-endian-systems/1696/4, we shouldn't be using anything other than 8-bit typed arrays and DataView to view memory. I will try to update the content.

@Josh-Cena Josh-Cena removed the needs info Needs more information to review or act on. label Apr 16, 2023
@michaelficarra
Copy link
Contributor

@Josh-Cena Many people will not care about portability of their JS to big-endian systems, though. This will be most people. For them, using multi-byte typed arrays is fine. You would be doing a disservice to them if you told them in the docs that the only safe option is a DataView.

@Josh-Cena
Copy link
Member

@michaelficarra It is the docs' duty to point out potential portability issues, and the more we inform, the better—the ultimate weighing of impact will still be on the readers, but at least they know. This is the same reason why we discourage Annex B (like substr(), not more confusing things like non-strict block-scoped functions)—the reason given is that "this may not work everywhere" (although in practice the chance is precisely 0), not "because the committee regrets them". In addition, as mhofman said on Discourse, using typed arrays imposes additional assumptions on the underlying memory's data type and may not make sense. Thanks for pointing out though, I may not use words as strong as "the only safe option".

@majaha
Copy link
Contributor Author

majaha commented Apr 17, 2023

@michaelficarra I'm afraid I have to disagree here. The whole point of WebAssembly is portable performant code. The main reason this hasn't been complained about before is that there are no mainstream big-endian WebAssembly-in-JS implementations yet, as far as I know. (The main candidates are firefox and node.js. I tried to get them running on PowerPC64 in Qemu to test this, but didn't have any success in running modern enough versions of them.)

But when/if the first one does appear, it would be a shame for them to discover that, after all that work, no Wasm web pages support them anyway. As long as ECMAScript provides for Big Endian, we should be advocating for supporting it.

For most uses, the DataView version should be just fine. A real pain point is when you want to use Wasm with other platform APIs like WebGL that want to take TypedArrays. I'm not sure what can be done about that without effecting performance. Perhaps Wasm needs to be given a way to write platform-endianness buffers / TypedArrays.

But anyway, let's not crush Wasm-on-BE before it's even begun!

@Josh-Cena
Copy link
Member

I've sent #33960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs Content:wasm WebAssembly docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants