http://people.mozilla.com/~vladimir/misc/webgl-perf/hello-gles.html With Odin enabled on the latest nightly, the gears are corrupted after a few frames (only half are drawn). If asmjs gets disabled, they render fine. My guess is some corruption in the asmjs heap, or maybe in memory overall (because the data is stored in a VBO).

The graphical corruption sounds like what Douglas was reporting earlier (can't find the bug maybe that was in an email thread?). Vlad: are you able to compile a build locally and, if so, does the problem repro if you put "return true" at the head of TryEnablingIon in ion/AsmJS.cpp?

I can repro on a plain FF Nightly build on a Samsung Galaxy SII, btw.

(In reply to Luke Wagner [:luke] from comment #1) > Vlad: are you able to compile a build locally and, if so, does the problem > repro if you put "return true" at the head of TryEnablingIon in > ion/AsmJS.cpp? I doubt it has something to do with FFI, but since it is hard to debug these problems, I'll test. I'm currently running a build to answer that question in 2h-3h.

(In reply to Luke Wagner [:luke] from comment #1) > The graphical corruption sounds like what Douglas was reporting earlier > (can't find the bug maybe that was in an email thread?). > > Vlad: are you able to compile a build locally and, if so, does the problem > repro if you put "return true" at the head of TryEnablingIon in > ion/AsmJS.cpp? I can reproduce this on a plain FF nightly on a Nexus 7. A debug build with TryEnablingIon giving up on the fast path works without the corruption.

I can confirm this is a FFI issue. Disabling the fast path fixes the issue. Tested on a Samsung Galaxy SIII. My own build on the chromebook can't show the canvas, making it a bit harder to debug. But I'm on this.

Simplified shell testcase used for testing

When getting the values from the stack instead of from a register was faulty under arm. I checked and the interpreter path uses "ShadowStackSpace" for arm too. This fixes the issue on soft float. The testcase still fails on hardfloat, but that is not related to the ffi fastpath. It is also present when disabling it totally. I'll create a bug report for it. I started a try build and will test it on my brothers phone, to see if the gears are now rendered correctly. https://tbpl.mozilla.org/?tree=Try&rev=e9b4449320f9

Confirmed. Gears aren't corrupted anymore with this patch.

Comment on attachment 771338 [details] [diff] [review] Patch Review of attachment 771338 [details] [diff] [review]: ----------------------------------------------------------------- Flipping review, since luke is absent and this should get fixed as fast as possible. Also I want to uplift as soon as possible.

Comment on attachment 771338 [details] [diff] [review] Patch Review of attachment 771338 [details] [diff] [review]: ----------------------------------------------------------------- I feel like this patch should be unnecessary, so I'm going to look into it. In the mean time, this should get landed.

I modified jsfunfuzz to generate FFI calls with more args (fuzzing rev 99522150a2f7).

(In reply to Jesse Ruderman from comment #11) > I modified jsfunfuzz to generate FFI calls with more args (fuzzing rev > 99522150a2f7). Could I suggest also adding support for generating FFI callee Ion functions that use a lot of local integer and floating point registers, enough to have used all the registers. Also, for the benefit of the ARM, test that an asm.js out-of-bounds heap load of a floating point number still returns NaN to ensure that the NANReg has been restored.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a25f41748ec2 (In reply to Jesse Ruderman from comment #11) > I modified jsfunfuzz to generate FFI calls with more args (fuzzing rev > 99522150a2f7). Thanks. The tips from dougc would also help to catch bugs.

Comment on attachment 771338 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 860838 User impact if declined: The wrong values are copied as arguments to ffi calls when providing to much arguments. So pretty serious. ARM only. Testing completed (on m-c, etc.): m-c:2 days + tested a browser build on softfloat locally and confirmed fixed. Risk to taking this patch (and alternatives if risky): No risk, everything is better than the current code for ARM. String or IDL/UUID changes made by this patch: /