Summary: | Setting array index -1 and looping over array causes bad behavior | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Kistner <gavin> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Major | CC: | ap, barraclough, fpizlo, msaboff, oliver | ||||||
Priority: | P1 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac (Intel) | ||||||||
OS: | OS X 10.7 | ||||||||
URL: | http://stackoverflow.com/questions/10629083/unexplained-behavior-in-safari-with-negative-array-indices | ||||||||
Attachments: |
|
Description
Gavin Kistner
2012-05-17 07:50:07 PDT
One additional note: I experienced the unstable behavior multiple times over multiple restarts of Safari, but was unable to consistently reproduce it. When it does not occur immediately, it still may occur if you repeatedly type a character followed by a backspace many times. Here is a simpler test case: setInterval(function(){ var a=[10,20,30,40], i=-1, x, c=a.length; a[-1] = 42; while (i<10000) x = a[i++ % a]; console.log(a[-1],a[4294967295]); },100); The above code produces the following console output: 42 undefined undefined 42 42 undefined 37x undefined 42 42 undefined undefined 42 42 undefined 41x undefined 42 42 undefined undefined 42 42 undefined 41x undefined 42 42 undefined undefined 42 42 undefined 37x undefined 42 (In reply to comment #2) > while (i<10000) x = a[i++ % a]; I intended to write: while (i<10000) x = a[i++ % c]; but either version results in similar output. Can you create a reduction that doesn't involve the developer console? It looks very dependent on what Web Inspector does now. Created attachment 142529 [details]
Reproducing the problem without using the console
I've added an attachment that shows the bug without using the console. on Safari 5.1.7 on OS X I see this in the textarea: 42:undefined 42:undefined undefined:42 42:undefined undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 42:undefined undefined:42 42:undefined undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 undefined:42 Further investigation shows that the same problem occurs with `-2` set as a value, but the problem is fixed if the key is explicitly set as the string "-1" instead of the integer -1. My guess is incorrect reification in an OSR exit, based purely on the symptoms (In reply to comment #8) > My guess is incorrect reification in an OSR exit, based purely on the symptoms Nope, OSR is fine. It's the slow path C function that the DFG calls for out-of-bounds indices. It assumes that the value is non-negative even though the whole point of the function is to handle both negative and too-large positive indices. Created attachment 142602 [details]
the patch
Comment on attachment 142602 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=142602&action=review r=me, but switch to Identifier::from() rather than using toString() > Source/JavaScriptCore/dfg/DFGOperations.cpp:465 > + Identifier property(exec, jsNumber(index).toString(exec)->value(exec)); > + PutPropertySlot slot(true); Use Identifier::from(exec, index) > Source/JavaScriptCore/dfg/DFGOperations.cpp:482 > + Identifier property(exec, jsNumber(index).toString(exec)->value(exec)); ditto (In reply to comment #12) > (From update of attachment 142602 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142602&action=review > > r=me, but switch to Identifier::from() rather than using toString() > > > Source/JavaScriptCore/dfg/DFGOperations.cpp:465 > > + Identifier property(exec, jsNumber(index).toString(exec)->value(exec)); > > + PutPropertySlot slot(true); > > Use Identifier::from(exec, index) > > > Source/JavaScriptCore/dfg/DFGOperations.cpp:482 > > + Identifier property(exec, jsNumber(index).toString(exec)->value(exec)); > > ditto Ah! Changed to use ::from(). Landed in http://trac.webkit.org/changeset/117523 |