Issue 11863 in v8: Wasm memory growth causes heap corruption

[conn… via monorail]

Status: Untriaged
Owner: ----
Type: Bug

New issue 11863 by conn...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863

Version: Chrome 91.0.4472.88 (Official Build) (64-bit) (but also affects 32-bit builds)
OS: Android 11
Architecture: arm64

What steps will reproduce the problem?

1. Either compile the attached minimal test case with Emscripten (we've verified with latest and some previous releases), or visit https://clv.zappar.io/1492122390623570121/1.0.0/
Note the issue only appears if DevTools is not attached. Everything seems to work fine when DevTools is attached.

2. The test case allocates memory twice, in order to trigger a heap grow between the allocations. If you launch the page (without DevTools attached) and wait a few seconds for it to load and run, then attach DevTools you can see that there's been a crash 'RuntimeError: memory access out of bounds'.

What is the expected output?
That the heap remains intact between grow operations

What do you see instead?
RuntimeError: memory access out of bounds and sometimes other errors (in other applications where we see this issue)

Let me know if this is not clear or if I can provide any additional information!

Attachments:
main.cpp 1.0 KB
build.sh 203 bytes

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[aza… via monorail]

Updates:
Components: WebAssembly

Comment #1 on issue 11863 by aza...@google.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c1

That this does not happen with devtools open may indicate it is a bug in TurboFan?

(Note: I could not verify the bug as I don't have an Android device to test on. But it does not happen on desktop, so it may indeed be arm or arm64 specific.)

[aza… via monorail]

Comment #2 on issue 11863 by aza...@google.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c2

Based on discussion on the original issue, this may already be fixed on canary?

https://github.com/emscripten-core/emscripten/issues/14391#issuecomment-856792331

[conn… via monorail]

Comment #3 on issue 11863 by conn...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c3

Indeed I'm unable to replicate the issue on Chrome Beta (92.0.4515.40) although I'm not sure if that means it's been fixed or not :-)

[si… via monorail]

Comment #4 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c4

I'm able to reproduce with the attached file, compiled using latest emscripten 2.0.23 with:
emcc -O2 -s INITIAL_MEMORY=8mb -s ALLOW_MEMORY_GROWTH=1 main.cpp -o index.html

Hosted version available here: https://tango-bravo.net/v8-bug-11863/index.html

It's just a single malloc that requires the heap to be enlarged, followed by setting just a few of the array elements. The issue seems to occur with this example when setting the byte 1703 bytes after the original 8mb heap size - after that the log output shows random bytes of memory, suggesting heap corruption has occurred somewhere.

I think it is TurboFan specific - it's 100% reproducible for me on a Pixel 4A (Chrome 91.0.4472.88) when setting chrome://flags "WebAssembly Baseline Compiler" to Disabled. Conversely, it always works as expected when setting "WebAssembly Baseline Compiler" to Enabled and "WebAssembly Tiering" to Disabled.

Some other findings:
- The use of iostream and -O2 appears to be needed to reproduce the bug. I think this is likely a red herring, and just the fact that it probably inlines a decent chunk of the upstream stuff into main and makes that function sufficiently complicated to trigger some TurboFan bug. In our real codebase we have seen this in code that doesn't use iostreams too.
- Still reproduces with other emscripten builds (we were previously using 1.38.18 from 2018), but the final byte offset that shows the correct log is different in different builds, it was reproducibly around 5400 bytes after the previous heap size in a build from that toolchain.

I note the chrome://flags mentioned above are no longer available in Chrome Canary, so perhaps the tiering decisions have been changed and it's just not using TurboFan there. Will take a look at beta to see if there's a way to force TurboFan.

Attachments:
main.cpp 679 bytes

[si… via monorail]

Comment #5 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c5

The flags are still available in Chrome beta (92.0.4515.40), and I can't reproduce with either of these test cases there even when "WebAssembly Baseline Compiler" is Disabled. So that indicates the bug has maybe been fixed.

Would be good to 100% confirm though, especially as the apparent heap corruption has the potential to be a security issue.

Our workaround for now is to allocate a large WebAssembly.Memory up-front when UA sniffing detects Chrome 91, which avoids the need for memory growth and runtime and appears to be stable.

[si… via monorail]

Comment #6 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c6

The issue is still reproducible on the 91.0.4472.101 patch update that has just rolled out to my Pixel 4A.

[gdee… via monorail]

Updates:
Owner: gde...@chromium.org
Status: Assigned

Comment #7 on issue 11863 by gde...@google.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c7

Heap corruption sounds bad, assigning to myself to pinpoint what might have changed between versions. May take me some time to obtain an Arm64 device though, I can't reproduce this on desktop. In the mean time, would it be possible to attach the compiled files in addition to the .c file attached that reproduce this issue? For example, compiled main.js, main.wasm files would be helpful to avoid different wasm binaries being generated between different emscripten versions.

[si… via monorail]

Comment #8 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c8

I've attached the output files from my test case in comment 4.

This seems to reproduce on both 32-bit and 64-bit Android builds, so any Android device will likely do. The emscripten issue referenced in comment 2 was originally noticed on linux desktop, so it is potentially cross-platform still.

Attachments:
index.html 91.5 KB
index.js 79.7 KB
index.wasm 122 KB

[donal… via monorail]

Comment #9 on issue 11863 by donal...@gmail.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c9

As mentioned in the previous comment, it should be possible to reproduce this issue (or at least this is what it seems like) by using
- Chrome 90.0.4430.212 (Official Build) (64-bit) or
- Chrome 89.0.4389.90 (Official Build) (64-bit)
on Linux by visiting this example deployment:
https://emscripten-dynamic-library-error-donalffons.vercel.app/demos/polygon

[lore… via monorail]

Comment #10 on issue 11863 by lore...@scandit.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c10

Any news about this issue?
We have reports (via Sentry) of this happening to more than 100 different Chrome 91.0.4472 users on our library demo page using WebAssembly, with no logic change on our side: https://websdk.scandit.com/
Probably related to this, the following kinds of errors are triggered:
- WebAssembly.Table.get(): invalid index [number] into function table
- RuntimeError: table index is out of bounds
- wasmTable.get(...) is not a function
- RuntimeError: function signature mismatch

[cleme… via monorail]

Updates:
Labels: Priority-1

Comment #11 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c11

(No comment was entered for this change.)

[fgall… via monorail]

Comment #12 on issue 11863 by fgal...@google.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c12

I think this is affecting our users as well.
https://github.com/emscripten-core/emscripten/issues/14538

[steph… via monorail]

Comment #13 on issue 11863 by steph...@gmail.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c13

I think that this is affecting me as well, when using Rust-based WASM to do some image decoding in the gaming context:
```
Stack:

RuntimeError: memory access out of bounds

at wee_alloc::alloc_first_fit::h54871a339ecedadf (https://cand.li/921d46bab2092445787f.module.wasm:wasm-function[45]:0x18786)
at png::decoder::zlib::ZlibStream::new::h19ba9524b04e662c (https://cand.li/921d46bab2092445787f.module.wasm:wasm-function[50]:0x1944b)
at wasmhelpers::png::decode_png_callback::h4631c5d81b50fa00 (https://cand.li/921d46bab2092445787f.module.wasm:wasm-function[15]:0xc6f3)
at decode_png_callback (https://cand.li/921d46bab2092445787f.module.wasm:wasm-function[134]:0x1efce)
...
```

This can be "experienced" by going to https://cand.li, skipping the onboarding, then clicking on the first example game (the pink Candivaders). I can reproduce this with Chrome 91 on all Android devices I've tested so far (half a dozen), but not on Linux desktop. Chrome beta 92 does not exhibit the problem. On the devices experiencing the problem, if I click a second time on the game, it works, so it does seem linked to memory growth (that is why I think I am experiencing this issue).

[gdee… via monorail]

Comment #14 on issue 11863 by gde...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c14

Experimenting with an Android device on 91.0.4472.88, I can reproduce the issues with hosted demos on:

https://clv.zappar.io/1492122390623570121/1.0.0/
https://tango-bravo.net/v8-bug-11863/index.html
https://websdk.scandit.com/

This does not reproduce on my android device for https://cand.li.

The hosted demo here https://emscripten-dynamic-library-error-donalffons.vercel.app/demos/polygon seems to work fine on my linux desktop, do I need to do anything specific to trigger the issue?

[steph… via monorail]

Comment #15 on issue 11863 by steph...@gmail.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c15

Sorry I forgot to update this issue: we deployed a work-around on https://cand.li, if Chrome version 91 is detected, the server serves an alternative set of files where the WASM is replaced by pure Javascript code generated through wasm2js. The cost of that hack is a 2-3 time slowdown, but it un-breaks our product for our users.

You can easily reproduce the issue by mocking the user agent to something that does not resolve to Chrome version 91, so that the WASM is served normally.

[donal… via monorail]

Comment #16 on issue 11863 by donal...@gmail.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c16

> The hosted demo here https://emscripten-dynamic-library-error-donalffons.vercel.app/demos/polygon seems to work fine on my linux desktop, do I need to do anything specific to trigger the issue?

No, not really. This example works fine in the latest Google Chrome version (Version 91.0.4472.114), but did cause errors with Google Chrome 90.0.4430.212 and 89.0.4389.90 on two different systems.

You might be able to reproduce an error with Appimages from here (although the error seems to be somewhat different different with these versions): https://ungoogled-software.github.io/ungoogled-chromium-binaries/releases/appimage/64bit/

For me, the results are:

ungoogled-chromium_91.0.4472.114-1.1: works fine
ungoogled-chromium_90.0.4430.212-1.1: causes Error code: SIGSEGV (with and without devtools open)
ungoogled_chromium_90.0.4430.85-1.1: causes Error code: SIGSEGV (with and without devtools open)

As mentioned on Github before: Maybe this means that this issue is solved with in the newest version of Chrome for this particular case - but maybe the underlying issue is still hiding somewhere...

[v… via monorail]

Comment #17 on issue 11863 by v...@hauzd.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c17

good to see someone is on this... big issue for us!
we can repro close to 100% with any of our projects such as:

https://hauzd.com/sample

(ignore webGL warnings they are fixed in our next release)
happens on practically any Android device on image processing worker during jpeg decompression, does not happen on desktops.
Also, if dev tools are connected then problem does NOT occur.

The memory is reported as allocated "ok", but heap is definitely corrupted.

[si… via monorail]

Comment #18 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c18

I've got a feeling the culprit here might be different from the one responsible for the bug on linux that is mentioned in comment 9 and comment 16, even though the symptoms appear similar.

The android issue only appeared for us in Chrome 91 (current stable release) and is 100% reproducible with all the test cases given in earlier comments. So I'd suggest focussing on those repeatable android cases as the more pressing concern right now for projects in-the-wild, rather than trying to reproduce the issue from comments 9 and 16 on linux builds.

Once the android one has been tracked down it might be obvious whether it would also be responsible for the behaviour on linux reported in comments 9 and 16 too.

In terms of the workaround we use, I have described that in more detail here: https://github.com/emscripten-core/emscripten/issues/14538#issuecomment-873605760

[gdee… via monorail]

Comment #19 on issue 11863 by gde...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c19

Following up with an update:

This is reproducible all the way up to 91.0.4472.161 (the last branch at the time I was testing), but does not occur 92.0.4500.0 onwards. With the files in comment #8, I've tried reproducing this standalone in the engine, but that has not worked.

I'm out for the most part of the next couple of weeks, ecmziegler@ could this be reassigned to someone on the runtime team who may be aware of changes on Arm64 that may have fixed this for the 92 branch?

[ecmzi… via monorail]

Updates:
Owner: ah...@chromium.org

Comment #20 on issue 11863 by ecmzi...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c20

(No comment was entered for this change.)

[sheriffbot via monorail]

Comment #21 on issue 11863 by sheriffbot: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c21

Dear owner, please check the status of the issue and update it according to our guideline: http://go/v8-issue-guidelines

Thanks for your time! To disable nags, add the Disable-Nags label.

[sheriffbot via monorail]

Comment #22 on issue 11863 by sheriffbot: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c22

[ah… via monorail]

Updates:
Status: Fixed

Comment #23 on issue 11863 by ah...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c23

I could not figure out what fixed this issue, and I don't plan to spend more time on this issue. As the issue seems to be fixed, I'm closing it now. Please re-open this issue if there is still a problem.

[si… via monorail]

Comment #24 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c24

I think our sites currently will still have the workaround - as the cause is still a bit of a mystery we'll need to do more testing to verify this is indeed fixed and not just harder to reproduce in the current builds.

I appreciate the hours spent attempting to find the root of this one, and of course understand the investigation having lower priority as it doesn't seem to affect current builds. I was hoping the resolution would be a bit more clear-cut, along with additional asserts / regression tests added to ensure the same issue won't affect future versions. I guess we've always had pretty high confidence in Chrome's rolling releases, but we perhaps need to take a more pro-active stance of running some automated testing against the Canary and Beta streams to get ahead of this kind of thing in future.

This bug rendered many of our sites (and plenty of others) broken for an entire Chrome major version window for billions of devices. Thankfully the workaround of pre-allocating a large WebAssembly.Memory to avoid memory growth at runtime enabled us to hot-fix the affected pages, so our business exposure to it was limited, but the prospect of WebAssembly just breaking overnight for a large portion of our users is now permanently added to my long list of things to worry about...

[benma… via monorail]

Comment #25 on issue 11863 by benma...@gmail.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c25

Our users still having this problem even on Chrome Android 97, please see this issue for more details and reproducable codesandbox: https://github.com/google/draco/issues/726
It can be consistently reproduced on many devices: Note FE, S21, S8, Xiaomi K20

[cleme… via monorail]

Comment #26 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c26

It looks like this was never confirmed to be a V8 bug actually. (Not saying it's not, but we should aim to get a better understanding of the issue.)
Can someone provide a minimized example (a stand-alone Wasm module) that fails when executed in V8?

[si… via monorail]

Comment #27 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c27

I can't allocate much time to this personally, but it would be good to get to the bottom of it - we're still avoiding memory growth on our sites by allocating a large chunk up-front, just in case.

I think the easiest way to track down / repro / bisect will be with android builds and the simple test cases that were 100% reproducible in Android Chrome 91. It's possible the other larger test cases are suffering a similar bug, but they seem much harder to reproduce.

So following the instructions at https://www.chromium.org/getting-involved/download-chromium/#downloading-old-builds-of-chrome-chromium I looked up Chrome version 91.0.4472.88 and got the following info:

Commit: 7786c26efaeaed9406b32952d029a1c47fb3fa48
Branch Base Commit: 3d60439cfb36485e76a1c5bb7f513d3721b20da1
Branch Base Position: 870763
V8 Commit: fd856cebb3515699d942d0f2517f6658a0cf720b
V8 Version: 9.1.269.28
V8 Position: 56
Skia Commit: 71fcb16fc206466727cb49cbf318b765f8a9d5a7

870756 was the preceding build available at the continuous build archive, which I found here:
https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Android/870756/

I have verified that the simple test case still reproduces the bug in that build of Chromium (using ChromePublic.apk). I've also rebuilt the test with the latest emscripten (3.1.3) and it still reproduces there. The exact offset that causes the corruption is a bit different again with this build (somewhere around 1730 bytes after the initially-allocated 8MB) so I've adjusted main.cpp to be less targeted at one particular offset.

As discovered above, TurboFan is required. Setting "Disable baseline compiler" in chrome://flags will ensure TurboFan is used, even on the first load.

Is there a similar continuous build archive just for V8? I've built from source in the past but don't have a linux build environment set up right now.

I think the test should be able to run standalone via d8; my first attempt would be something like this that I've used for running benchmarking code in the past:
./d8 --no-wasm-tier-up --no-liftoff --predictable test.js

Attached the main.cpp test case, the compiled test.js and test.wasm, and a more minimal html file that just hooks up the console output to a <textarea>.
Compilation command line: emcc -O2 -s INITIAL_MEMORY=8mb -s ALLOW_MEMORY_GROWTH=1 main.cpp -o test.js

Hosted version of minimal page: https://tangobravo-public.s3.eu-west-1.amazonaws.com/v8-bug-11863/index.wasm

Over to someone on the V8 team to try to reproduce with the d8 commit shown above, or to bisect based on the Chromium continuous builds archive. I feel like I've done my part on this one now...

Attachments:
main.cpp 709 bytes
minimal-shell.html 1.2 KB
test.js 75.9 KB
test.wasm 122 KB

[si… via monorail]

Comment #28 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c28

Apologies for the error in the hosted URL, that should be:
https://tango-bravo.net/v8-bug-11863/minimal-shell.html

[cleme… via monorail]

Updates:
Cc: ah...@chromium.org
Owner: clem...@chromium.org
Status: Assigned

Comment #29 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c29

I can confirm that running test.js with minimal changes (replace "readBinary" by "readbuffer") runs successfully with --liftoff-only, but produces weird output with --no-liftoff on 9.1.269.28:
[...]
3126400 1336
3126500 1436
3126600 1536
3126700 1636
ąƇȉʋ̍ΏБғ֗ڛܝޟ㤥槨骫쭮ﰱ

It works correctly on ToT though. I'll try to bisect what fixed this.

[cleme… via monorail]

Comment #30 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c30

Aha, bisection points to 1955ef64b32b5fed7d214b6153233e521973c8f1 (Disable untrusted code mitigations on Android).

[cleme… via monorail]

Updates:
Labels: Needs-Feedback

Comment #31 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c31

As there seems to be indication that some variant of this problem still exists in current versions, I am again asking for a reproducer we can run in d8 or chrome, in order to get to the bottom of this.

[si… via monorail]

Comment #32 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c32

Thanks for bisecting the commit that fixed it; perhaps worth bisecting the one where it was introduced too in case that gives any more hints about whether it is likely scoped to the untrusted mitigations?

The reproducer in comment 25 has this direct link:
https://2ydjz.csb.app/

I do see a RuntimeError in the console on my 4A with that in current Chrome 97, but that is loading a large 2.5 million poly file so might just be running out of memory (with Chrome still using 32 bit I guess virtual address space congestion might limit how big wasm memory can get?)

[si… via monorail]

Comment #33 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c33

To me, that does seem likely to be a different error though - the initial report was around table index / memory access out of bounds, which doesn't seem to be the case with that current reproducer test case.

[cleme… via monorail]

Updates:
Cc: mano...@chromium.org

Comment #34 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c34

Good idea for another bisect to see what introduced this. It was
f6ee9ed045395ed364bed0a629089ea5bf5d8d19 ([turbofan] Introduce LoadImmutable, use it in wasm compiler).

Manos, are you aware of bugs in that CL which were subsequently fixed? Otherwise we should maybe investigate what was causing the problem, to check if the problem still exists.

I'll try to minimize the reproducer in the meantime.

[cleme… via monorail]

Comment #35 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c35

Ok, I got a minimized reproducer (attached). I didn't investigate the exact bug yet.

Attachments:
test.js 2.4 KB

[Git Watcher via monorail]

Comment #36 on issue 11863 by Git Watcher: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c36

The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/0ad95cf26b5b3580f9f9756b46db3fb797dcced3

commit 0ad95cf26b5b3580f9f9756b46db3fb797dcced3
Author: Clemens Backes <clem...@chromium.org>
Date: Tue Feb 15 09:12:27 2022

[wasm] Fix data segment address emission in module builder

The module builder was outputting the address as an unsigned LEB value
instead of a signed value, leading to wrong results.

R=mano...@chromium.org

Bug: v8:11863
Change-Id: I547ca98defcae0ba15b4004a506b65387534b08a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3463715
Reviewed-by: Manos Koukoutos <mano...@chromium.org>
Commit-Queue: Clemens Backes <clem...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79090}

[modify] https://crrev.com/0ad95cf26b5b3580f9f9756b46db3fb797dcced3/test/mjsunit/wasm/wasm-module-builder.js

[Git Watcher via monorail]

Comment #37 on issue 11863 by Git Watcher: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c37

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/ef55d88c1467480c9502bc3b7c6c9a28c5d322c9

commit ef55d88c1467480c9502bc3b7c6c9a28c5d322c9
Author: Clemens Backes <clem...@chromium.org>
Date: Mon Feb 14 18:04:55 2022

[wasm][fuzzer] Correctly generate function imports

The --wasm-fuzzer-gen-test functionality didn't handle imported
functions correctly.

R=mano...@chromium.org

Bug: v8:11863
Change-Id: I0486e15465bfabead7bc352d1a94326a39439e43
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3460412

Reviewed-by: Manos Koukoutos <mano...@chromium.org>
Commit-Queue: Clemens Backes <clem...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#79091}

[modify] https://crrev.com/ef55d88c1467480c9502bc3b7c6c9a28c5d322c9/test/fuzzer/wasm-fuzzer-common.cc

[manos… via monorail]

Updates:
Status: Fixed

Comment #38 on issue 11863 by mano...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c38

After further investigation, this was caused by masking the memory index returned by explicit bounds checks when untrusted code mitigations was on. This would also explain why it did not manifest in x64, where we have been using trap handlers for memory-OOB errors for a while.
I am not exactly sure what the problem was, but this option has been removed since April 2021, so it is not worth pursuing further.

[cleme… via monorail]

Comment #39 on issue 11863 by clem...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c39

Thanks for investigating, Manos!

[Git Watcher via monorail]

Comment #40 on issue 11863 by Git Watcher: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c40

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/88b931f7cee6321be41551aceda9bb1546420f1f

commit 88b931f7cee6321be41551aceda9bb1546420f1f
Author: Clemens Backes <clem...@chromium.org>
Date: Tue Feb 15 11:08:51 2022

[wasm][fuzzer] Dump data segments when generating tests

Data segments were missing in the output of --wasm-fuzzer-gen-test.

R=mano...@chromium.org

Bug: v8:11863
Change-Id: I40e60ef8626125ca9df6bead688607215d9e5b58
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3461932

Reviewed-by: Manos Koukoutos <mano...@chromium.org>
Commit-Queue: Clemens Backes <clem...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#79106}

[modify] https://crrev.com/88b931f7cee6321be41551aceda9bb1546420f1f/test/fuzzer/wasm-fuzzer-common.cc

[em… via monorail]

Updates:
Cc: em...@chromium.org

Comment #41 on issue 11863 by em...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c41

Hello,
Could someone please share what are the conditions for the bug to happen? We're trying to understand whether we need to hold on the NaCl=>Wasm migration of one Chrome OS components (Smart Card Connector) because of this issue on older versions.

[manos… via monorail]

Comment #42 on issue 11863 by mano...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c42

Hello,
This bug is present up to V8 release 9.2. It manifests in platforms without trap handler support; for 9.2, this would be everything except x64 and Mac on arm64. We do not know the exact conditions that reproduce it, but it is some combination of memory.grow followed by a memory load/store.

[si… via monorail]

Comment #43 on issue 11863 by si...@zappar.com: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c43

Thanks Manos.

So just to be clear, you're now confident this is fixed in versions after 9.2?

ie Comment 25 relates to a distinct bug (probably a bug in that library rather than V8?), and we should be good to re-enable memory growth on our builds?

[manos… via monorail]

Comment #44 on issue 11863 by mano...@chromium.org: Wasm memory growth causes heap corruption
https://bugs.chromium.org/p/v8/issues/detail?id=11863#c44

I can confirm the bug does not reproduce as of V8 version 9.3.345.3/ Chromium version 94.0.4581.0.
The bug of Comment 25 seems distinct, as the error message is different, but I cannot confirm it is not a V8 bug at this moment.