Skip to content

Conversation

ospencer
Copy link
Member

Hey there!

I was messing around with the JS output and noticed an issue with the way memory is copied when lowering lists of bytes. The argument is a typed array, but it copies the entire underlying buffer, which in some cases can be larger than the view of the typed array. This results in an error since we end up trying to write more data than what's expected.

I fixed this by explicitly providing the byte offset and size.

Let me know if there's any other information I can provide or anything I can do to help get this merged.

Cheers!

@alexcrichton
Copy link
Member

Thanks! Would it be possible to add a test for this as well?

@ospencer ospencer force-pushed the oscar/js-buffer-size branch from d190190 to 7a91642 Compare January 26, 2022 21:50
@ospencer ospencer force-pushed the oscar/js-buffer-size branch from 7a91642 to d9cf1f0 Compare January 26, 2022 21:54
@ospencer
Copy link
Member Author

@alexcrichton I added a list-roundtrip to make it a bit easier to test this, and confirmed the test fails with the current implementation and passes with the fix. Let me know if the test is clear enough!

@alexcrichton
Copy link
Member

Looks great to me, thanks! I'll merge when CI is green

@alexcrichton alexcrichton merged commit 48f364d into bytecodealliance:main Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants