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

"Out of bounds memory access" on iOS 11.2.2 #6042

Closed
bhollis opened this Issue Jan 9, 2018 · 39 comments

Comments

Projects
None yet
@bhollis
Copy link

bhollis commented Jan 9, 2018

We're using a modified fork of sql.js built for wasm using an emscripten version from about 7 months ago (can't recall the exact version). With the update to iOS 11.2.2 and the Meltdown/Spectre mitigations, this code now fails at sqlite3_open with "Out of bounds memory access".

screen shot 2018-01-08 at 10 27 02 pm

I suspect this change may have affected something: https://trac.webkit.org/changeset/226461/webkit

I'm rebuilding with the latest emscripten now to see if it helps. I'm not sure if this is an emscripten issue or a sqlite issue.

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 9, 2018

Rebuilding with 1.37.28 (which took some changes to work) still produces wasm that fails.

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 9, 2018

Rebuilding with the latest sqlite3.c (3.21.0) still produces wasm that fails.

@juj

This comment has been minimized.

Copy link
Collaborator

juj commented Jan 9, 2018

You could try doing an asm.js build to see if that works - if it also fails, then it might be an Emscripten issue. (it can also be an Emscripten issue even if it doesn't, but might help at poking the issue from another angle). Also doing an -O0 build so that callstacks are preserved should show the function names instead of the "wasm function" blocks being printed - that might help debug as well.

To be sure, it would probably be best if you're able to run the original build that did work on iOS, and cross-check the same build against the old working iOS and new breaking iOS. If the only variable is the iOS version change, it would then be fair to say that it would be an iOS regression, although whether it's then a wasm execution regression or a consequence of some other web api regressing (and resulting in an eventual wasm trap) is then still a question.

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 9, 2018

The asm.js build works - right now I'm trying to execute wasm, catching the error, and falling back to asm.js. The older build's asm.js didn't work but it may have been broken for unrelated reasons - I hadn't tested it in a bit.

We definitely only have this issue on iOS 11.2.2 - both the original build and the new one don't work there but continue to work on iOS 11.2.1. Weirdly, the patched desktop Safari runs the wasm fine.

@kripken

This comment has been minimized.

Copy link
Member

kripken commented Jan 9, 2018

This sounds more and more like an iOS regression. Another thing I'd check is to run the same workload on Chrome and Firefox on desktop. If, as seems likely, it works on desktop on Safari, Chrome, and Firefox, and just fails on iOS, I'd file a WebKit bug with this.

@seankibler

This comment has been minimized.

Copy link

seankibler commented Jan 9, 2018

Experiencing the exact issue as well. Have tested on multiple Apple devices against the exact same WASM build pre and post 11.2.2 update and all exhibit the same behavior; work fine pre 11.2.2 but not after. Have also confirmed desktop Safari is ok. Will also try an ASM build. Agree with others that this is likely a regression, perhaps from a rushed guard against Meltdown or Spectre, or maybe just coincidence.

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 9, 2018

Yup, desktop Firefox and Chrome work fine. I'm mostly putting this here so people are aware, I'm pretty sure it's an Apple bug, and it's a hard one for wasm users to work around because you can't just feature detect it.

@jfbastien

This comment has been minimized.

Copy link
Contributor

jfbastien commented Jan 9, 2018

This is a known issue (rdar://problem/36236004) and is fixed in tip-of-tree WebKit.

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 10, 2018

Awesome! I don't have radar access but I'm excited for the fix.

@elbalkini

This comment has been minimized.

Copy link

elbalkini commented Jan 14, 2018

anyone has an update regarding this issue?

@bhollis

This comment has been minimized.

Copy link

bhollis commented Jan 14, 2018

I've been told the fix is included in the latest iOS betas.

@elbalkini

This comment has been minimized.

Copy link

elbalkini commented Jan 14, 2018

i am on IOS 11.2.5 Beta and it is still not fixed. Same error message

@lgfaler

This comment has been minimized.

Copy link

lgfaler commented Jan 15, 2018

Also seeing this same issue with our WASM. Falling back to ASM fixed the issue, but the decrease in performance makes that an unusable option for our application. Hoping for a fix soon.

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 18, 2018

11.2.5beta6 doesn't fully resolve it for me in ogv.js; I filed https://bugs.webkit.org/show_bug.cgi?id=181781 in case there's another regression on top of the initial fix.

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 23, 2018

11.2.5 released today, apparently without the fix.

@elbalkini

This comment has been minimized.

Copy link

elbalkini commented Jan 23, 2018

It is totally frustrating how Apple is handeling their technology, Webassembly is broken, getusermedia is only on safari no support in the Webkit webview nor safari controller. everything is either Apple way or 'nil'.

@phoboslab

This comment has been minimized.

Copy link

phoboslab commented Jan 24, 2018

Can confirm. The bug is still present in iOS 11.2.5.

Is there some way to detect the faulty WebAssembly implementation other than just probing the UA with /iPhone OS 11_2_(2|5)/.test(navigator.userAgent)? Preferably without having to load the WASM blob, execute it and catch the error first.

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 24, 2018

@phoboslab you can feature-detect the failure with a minimal wasm module like this:

(You could do this synchronously by loading from an array in the source instead of loading a small .wasm file.)

Base error seems to be that it has trouble loading/storing memory. There may be a cleaner test, but this works for me confirming the fail on 11.2.5 and doesn't fail elsewhere: store a non-zero i32 value at memory index 4, then load it back. If you get 0, the failure is live.

(Note this doesn't throw the out of bounds exception, which looks like a side effect of having wrong values loaded/stored such as pointers.)

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 25, 2018

I can confirm this is resolved for me in iOS 11.3 beta 1.

@elbalkini

This comment has been minimized.

Copy link

elbalkini commented Jan 25, 2018

@brion thanks for the update, this is much appreciated.

ericmandel added a commit to ericmandel/js9 that referenced this issue Jan 25, 2018

wasm broken in ios 11.2.2 and 11.2.5 but fixed in 11.3beta1
therefore, we can restrict which versions of ios need wasm disabled
see: emscripten-core/emscripten#6042
@steipete

This comment has been minimized.

Copy link

steipete commented Jan 25, 2018

We hit the problem in our Web PDF Renderer for iOS 11.2.2 and 11.2.5 and I can confirm that 11.3b1 indeed resolves this issue. Our fallback is to use the asm.js build on these specific versions.

@elbalkini

This comment has been minimized.

Copy link

elbalkini commented Jan 25, 2018

any ideas when IOS 11.3 will be public to all ios users?

@shader13

This comment has been minimized.

Copy link

shader13 commented Jan 25, 2018

Did you mean 11.3b1 is 11.3 development beta1?
I have updated to this version, but I still get the issue using Unity WebGL WASM:
Unhandled Promise Rejection: Error: Out of executable memory in function at index
image

@steipete

This comment has been minimized.

Copy link

steipete commented Jan 25, 2018

Apple rarely gives exact release dates for iOS point releases, but has given a general time frame of "this spring." So far, only the first beta has been released to developers, not the general public. Given the features in this release, we expect at least four or five beta releases, possibly many more. We doubt iOS 11.3 will release before March. (source)

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 25, 2018

Error: Out of executable memory in function at index

That may be a distinct issue from the "out of bounds" failures, but is certainly also worrying if it's a regression. It sounds like that could be an issue with compiling a large amount of JS or wasm code in general... Note some devices supported by iOS 11 have low memory ceilings like the first gen iPad Air, which may also make them more susceptible to that.

If it looks like a regression where it worked prior to 11.2.2 on the same device, then definitely file a bug with bugs.webkit.org or direct in Apple's internal tracker.

@jfbastien

This comment has been minimized.

Copy link
Contributor

jfbastien commented Jan 25, 2018

JSC usable executable memory is set here on a per-processor type basis: https://trac.webkit.org/browser/webkit/trunk/Source/JavaScriptCore/jit/ExecutableAllocator.cpp

@brion

This comment has been minimized.

Copy link
Collaborator

brion commented Jan 29, 2018

I've got a synchronous-friendly test case that can be copy-pasted straight into JS:

https://github.com/brion/min-wasm-fail/blob/master/min-wasm-fail.js

This compiles a tiny wasm module from an array in the JS source, tests for the bug, and reports back. I'm using it now in ogv.js to avoid having to blacklist on user-agent; feel free to copy it!

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Jul 31, 2018

Same problem for me:

12381ms |  exception thrown: Error: Out of bounds memory access
(evaluating 'Module["asm"]["_main"].apply(null,
arguments)'),<?>.wasm-function[__ZN4game18Scene_SplashScreen4initEv]@[wasm
code]
<?>.wasm-function[__ZN4game18Scene_SplashScreen6createEv]@[wasm code]
<?>.wasm-function[__ZN4game5CMain4initEv]@[wasm code]
<?>.wasm-function[__ZN4game11AppDelegate29applicationDidFinishLaunchingEv]@[wasm
code]
<?>.wasm-function[__ZN7cocos2d11Application3runEv]@[wasm code]
<?>.wasm-function[_main]@[wasm code]
wasm-stub@[wasm code]
_main@[native code]

callMain
http://192.168.1.37/loader.js:456:30
fn@http://192.168.1.37/loader.js:465:32

emscripten: 1.38.10 (commit 8e5456b)
iphone: 6, 8, 10
ios 11.4.1

@kripken

This comment has been minimized.

Copy link
Member

kripken commented Jul 31, 2018

cc @jfbastien, see last comment - this seems to still be an issue.

@kmiller68

This comment has been minimized.

Copy link

kmiller68 commented Jul 31, 2018

@caiiiycuk Do you have a repro case? If so, can you open a bug on bugs.webkit.org and CC me (keith_miller)?

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Aug 1, 2018

@kmiller68 I think I have only build in binary form (wasm). I can't provide sources because of NDA, is it ok for you?

@kmiller68

This comment has been minimized.

Copy link

kmiller68 commented Aug 1, 2018

Sure, I can take a look either way.

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Aug 1, 2018

I figured out the reason of this exception. I think it is not webkit bug and not emscripten bug, I use incorrect link settings. I've use -s TOTAL_MEMORY=48Mb -s ALLOW_MEMORY_GROWTH=1 -s WASM_MEM_MAX=128Mb. I thought that means that memory will grow from 48Mb but not bigger then 128Mb, but actually memory will grow as follow: 48Mb, 96Mb, 192Mb... And when code tries to increase memory from 98Mb to 192Mb it fails silently and error "exception thrown: Error: Out of bounds memory access" will occurs after a while.

It is happen in all browsers, I found this on iphone because I think it's slower. My code do some concurrent tasks, and I think they stay alive more time on iphone (so need more memory).

In FF:

exception thrown: RuntimeError: index out of bounds,_free@http://localhost/oom.js:17190:1
_fclose@http://localhost/oom.js:17631:1
_main@http://localhost/oom.js:34018:1

In Chrome:

exception thrown: RuntimeError: memory access out of bounds,RuntimeError: memory access out of bounds
    at _free (wasm-function[45]:1508)
    at _fclose (wasm-function[46]:167)
    at _main (wasm-function[81]:10986)

In Safari:

exception thrown: Error: Out of bounds memory access (evaluating 'Module["asm"]["_main"].apply(null, arguments)'),<?>.wasm-function[_free]@[wasm code]
<?>.wasm-function[_fclose]@[wasm code]
<?>.wasm-function[_main]@[wasm code]
wasm-stub@[wasm code]
_main@[native code

The problem is that this error is not clear (in -O3 --profling), I expect to see "Out of memory" in moment when memory can't grow. But error "exception thrown: Error: Out of bounds memory access" occurs not in moment of failed grow.

Super simple test case to reproduce attached. Compile with:

emcc --preload-file "res_test@" --no-heap-copy -O3 --profiling -s TOTAL_MEMORY=48Mb -s ALLOW_MEMORY_GROWTH=1 -s WASM_MEM_MAX=128Mb --std=c++11 oom.cpp -o oom.html

oom.zip

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Aug 1, 2018

@kripken Should I create issue to improve error message for this case?

@kripken

This comment has been minimized.

Copy link
Member

kripken commented Aug 2, 2018

Thanks @caiiiycuk, I think what's wrong is that we didn't respect the wasm mem max in growth mode. Fix in #6937 - does your app work ok with that? (or, at least show a proper error on malloc failing when reaching the wasm mem max limit)

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Aug 3, 2018

Sorry, I can't verify because I am on vocations now, but when I return (after 15th of August) I will check for sure. Thanks!

kripken added a commit that referenced this issue Aug 10, 2018

Respect the WASM_MEM_MAX limit in memory growth (#6937)
If the user specified a limit, it is set in the wasm binary, and we can't (and shouldn't) grow past it.

Also the docs in settings.js about this were wrong.

See #6042
@kripken

This comment has been minimized.

Copy link
Member

kripken commented Aug 10, 2018

(@caiiiycuk That PR was merged, so when you get back you can just test it on incoming now.)

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

caiiiycuk commented Aug 20, 2018

Just checked, problem is solved 👍

@kripken

This comment has been minimized.

Copy link
Member

kripken commented Aug 20, 2018

Great, thanks @caiiiycuk.

Ok, I think all bugs discussed here are known to be fixed, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment