Bug 109036 – DFG OSR exit doesn't know which virtual register to use for the last result register for post_inc and post_dec

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael]

Created attachment 186808 [details]
(smallest?) piece of code to reproduce

The jit-compiler produces erroneous code from javascript loops with post de- or increments of variables. This will lead to errors or unexpected behavior during runtime.

Steps to reproduce:
- close console/inspector/debugger (to enable the jit-compiler, important even on iOS!)
- open the attached html file
- click "Click Me!" a few times till output is "from 0 to 501"

Explanation:
Clicking on "Click Me!" recalculates indexes of a list with 500 items. When the error occurs, the last index is set to 501 (which is wrong).

Workarounds:
- don't use post-increments (split access and increment)
- surround code with try/catch (which seems do disable jit)


The bug was originally posted on stackoverflow: http://stackoverflow.com/questions/13147026/disabling-jit-in-safari-6-to-workaround-severe-javascript-jit-bugs
and is also reported to Apple (ID: 12606761)



Full text from stackoverflow:

We found a severe problem with the interpretation of our Javascript code that only occurs on iOS 5/Safari 6 (current iPad release) that we think is due to critical bug in the Just in Time JS compiler in Safari. (See update below for more affected versions).

We originally found the issue in our online demos of our library: the demos crash more or less randomly but this happens only the second time (or even later) that the same code is executed. I.e. if you run the part of the code once, everything works OK, however subsequent runs crash the application.

Interestingly executing the same code in Chrome for iOS the problem does not show, which we believe is due to the missing JIT capabilities of the Webview that is used in Chrome for iOS.

After a lot of fiddling we finally think we found at least one problematic piece of code:

  var a = 0; // counter for index
  for (var b = this.getStart(); b !== null; b = b.getNext()) // iterate over all cells
    b.$f = a++; // assign index to cell and then increment


In essence this is a simple for loop that assigns each cell in a linked list data structure its index. The problem here is the post-increment operation in the loop body. The current count is assigned to the field and updated after the expression is evaluated, basically the same as first assigning a and then incrementing it by one.

This works OK in all browsers we tested and in Safari for the first couple of times, and then suddenly it seems as if the counter variable a is incremented first and then the result is assigned, like a pre-increment operation.

I have created a fiddle that shows the problem here: http://jsfiddle.net/yGuy/L6t5G/

Running the example on an iPad 2 with iOS 6 and all updates the result is OK for the first 2 runs in my case and in the third identic run suddenly the last element in the list has a value assigned that is off by one (the output when you click the "click me" button changes from "from 0 to 500" to "from 0 to 501")

Interestingly if you switch tabs, or wait a little it can happen that suddenly the results are correct for two or so more runs! It seems as if Safari sometimes resets is JIT caches.

So since I think it may take a very long for the Safari team to fix this bug (which I have not yet reported) and there may be other similar bugs like this lurking in the JIT that are equally hard to find, I would like to know whether there is a way to disable the JIT functionality in Safari. Of course this would slow down our code (which is very CPU intensive already), but better slow than crashing.

Update: Unsurprisingly it's not just the post increment operator that is affected, but also the post decrement operator. Less surprisingly and more worryingly is that it makes no difference if the value is assigned, so looking for an assignment in existing code is not enough. E.g. the following the code b.$f = (a++ % 2 == 0) ? 1 : 2; where the variables value is not assigned but just used for the ternary operator condition also "fails" in the sense that sometimes the wrong branch is chosen. Currently it looks as if the problem can only be avoided if the post operators are not used at all.

Update: The same issue does not only exist in iOS devices, but also on Mac OSX in Safari 6 and the latest Safari 5: These have been tested and found to be affected by the bug: Mac OS 10.7.4, Safari 5.1.7 Mac OS X 10.8.2, WebKit Nightly r132968: Safari 6.0.1 (8536.26.14, 537+). Interestingly these do not seem to be affected: iPad 2 (Mobile) Safari 5.1.7, and iPad 1 Mobile Safari 5.1. I have reported these problems to Apple but have not received any response, yet.

[Geoffrey Garen]

<rdar://problem/13292139>

[Filip Pizlo]

Created attachment 190416 [details]
the patch

[Filip Pizlo]

Comment on attachment 190416 [details]
the patch

r=gavin, in person.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/144137

[Yuqiang Xian]

*** Bug 110942 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

(In reply to comment #4)
> Landed in http://trac.webkit.org/changeset/144137

It broke some builds, because canBeOptimizedOrInlined() is defined
inside ENABLE(DFG_JIT) guard, but used in different places:
- http://trac.webkit.org/changeset/144137/trunk/Source/JavaScriptCore/jit/JIT.cpp ( ENABLE(DFG_JIT) || ENABLE(LLINT) )
- http://trac.webkit.org/changeset/144137/trunk/Source/JavaScriptCore/jit/JITArithmetic.cpp (without any ifdef guard)
- http://trac.webkit.org/changeset/144137/trunk/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
- http://trac.webkit.org/changeset/144137/trunk/Source/JavaScriptCore/jit/JITCall.cpp

...

[Julien Brianceau]

(In reply to comment #6)
> It broke some builds, because canBeOptimizedOrInlined() is defined
> inside ENABLE(DFG_JIT) guard, but used in different places:

I agree. Here is our sh4 build bot build for further info:
http://build.webkit.org/builders/Qt%20Linux%20SH4%20Release/builds/21059

[Zoltan Arvai]

Same problem on Qt Windows 32-bit Release
http://build.webkit.org/builders/Qt%20Windows%2032-bit%20Release/builds/62259

and on x86-64 Mountain Lion Qt Release
http://build.webkit.sed.hu/builders/x86-64%20Mountain%20Lion%20Qt%20Release%20%28WebGL%20Tester%29/builds/3361

[Julien Brianceau]

I've created a new bug entry: https://bugs.webkit.org/show_bug.cgi?id=110991
I'll submit a patch soon.