Summary: | Nitro JIT produces incorrect math on 32-bit iOS devices | ||
---|---|---|---|
Product: | WebKit | Reporter: | Lee Byron <lee> |
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ggaren, msaboff |
Priority: | P2 | Keywords: | InRadar |
Version: | 525.x (Safari 3.2) | ||
Hardware: | iPhone / iPad | ||
OS: | iOS 7.0 | ||
URL: | https://github.com/facebook/immutable-js/issues/92 |
Description
Lee Byron
2014-09-03 15:29:39 PDT
Also filed RDAR #18220947 I have tried to reproduce this issue on an iPad mini, iPad 3rd gen and iPad 4th gen all running 7.1.2 using the websites http://codepen.io/conradz/pen/bKClx and http://codepen.io/conradz/full/bKClx/ and I am unable. I always get "done". My agent is Mozilla/5.0 (iPhone; CPU iPhone OS 7_1_2 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Version/7.0 Mobile/11D257 Safari/9537.53. Is there another website that reproduces the bug? Are you hooked up to a Safari debugger when loading the page? That seems to prevent the issue from appearing. It was a 100% repro on 32bit iOS devices for the handful of us investigating this. It seems that small changes to the function body can also result in the error evaporating, but this URL was a consistent repro for us. I also forked the codepen at http://codepen.io/anon/pen/ElLah to start logging more and got a repro (In reply to comment #4) > I also forked the codepen at http://codepen.io/anon/pen/ElLah to start logging more and got a repro I am not using the Safari debugger (Web Inspector), I am just using Safari directly. Neither the original nor this URL repros the problem. We think the defect is in one of the JIT engines which we tier up to after executing code repeatedly. That fully explains why it fails after so many iterations. It could be that I need to sign up / log in. Can you repro without being logged in? I wasn't logged in when getting this to repro. I agree with your theory. For those running it, some see it fail at iteration 37, others at 25. There's some externality that's causing one of these two (or no repro at all). Also, when trying to narrow this down, unrelated changes such as adding a console.log would cause it to stop appearing. I was only able to narrow it down to an `n >>> 0`. It also failed with >>. If the second operand was not 0, the error vanished again. I patched the issue with `s === 0 ? n : n >>> s`, so my guess is that this is a JIT optimization around >> always being applied to a 0 value, but my guess is that it's incorrectly looking at the second operand instead of the first when short circuiting to 0. (In reply to comment #6) > I wasn't logged in when getting this to repro. > > I agree with your theory. For those running it, some see it fail at iteration 37, others at 25. There's some externality that's causing one of these two (or no repro at all). Also, when trying to narrow this down, unrelated changes such as adding a console.log would cause it to stop appearing. > > I was only able to narrow it down to an `n >>> 0`. It also failed with >>. If the second operand was not 0, the error vanished again. I patched the issue with `s === 0 ? n : n >>> s`, so my guess is that this is a JIT optimization around >> always being applied to a 0 value, but my guess is that it's incorrectly looking at the second operand instead of the first when short circuiting to 0. So I looked at the page source for http://codepen.io/anon/pen/ElLah, and I believe it has the workaround: 1381: var bit = (1 << ((shift === 0 ? hash : hash >>> shift) & MASK)); The other websites said to reproduce that I checked seem to have the workaround as well. Can you either provide a website without the workaround or attach a tar file with all the files I would need to make a local "failing" website? OHHH, crap of course. Those demos are pointed at a master build of immutable and I've already committed the work around. If you use tag 2.0.16 instead of master then I'm sure you'll see the repro. Sorry for the moving target. Ok, sorry about that. I've edited http://codepen.io/anon/pen/ElLah such that it loads https://rawgit.com/facebook/immutable-js/2.0.17/dist/Immutable.js which is before my workaround for this issue and should now repro on a 32bit iOS device. (In reply to comment #9) > Ok, sorry about that. I've edited http://codepen.io/anon/pen/ElLah such that it loads https://rawgit.com/facebook/immutable-js/2.0.17/dist/Immutable.js which is before my workaround for this issue and should now repro on a 32bit iOS device. NOW I can repro. Thanks. I have verified that the top WebKit trunk JavaScript does not have this issue, including verifying that 32 bit iOS devices work correctly with the updated test URL. Given that the issue exists in iOS 7.1.2, it is suggested that the workaround be used at least in the near term. |