Bug 143104

Summary: REGRESSION: js/regress/inline-arguments-local-escape.html is flaky
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, commit-queue, fpizlo, ggaren, mark.lam, mhahnenb, mmirman, msaboff, nrotem, oliver, saam, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
hopefully the patch none

Comment 1 Radar WebKit Bug Importer 2015-03-26 10:11:04 PDT
<rdar://problem/20310708>
Comment 2 Alexey Proskuryakov 2015-03-28 22:36:40 PDT
This test is still flakily failing.
Comment 3 Filip Pizlo 2015-03-30 08:30:59 PDT
(In reply to comment #2)
> This test is still flakily failing.

OK, looking at it now.
Comment 4 Filip Pizlo 2015-03-30 08:51:44 PDT
Turns out to be a pretty bad bug.  PreciseLocalClobberize's readTop() method is totally wrong for accesses to "escaped" arguments.  There are a handful of ways to fix this and I'll investigate.  Here's a version of the test that crashes every time:


function foo() {
    return arguments;
}

function bar(a, b, c, i) {
    var a = foo(b, c, 42);
    return a[i];
}

noInline(bar);

var expected = [2, 3, 42];
for (var i = 0; i < 10000; ++i) {
    var result = bar(1, 2, 3, i % 3);
    if (result != expected[i % 3])
        throw "Error: bad result: " + result;
}
Comment 5 Filip Pizlo 2015-03-30 08:57:56 PDT
Same bug, involving ForwardVarargs:

function foo() {
    return arguments;
}

function baz(a, b, c) {
    return a + b + c;
}

function bar(a, b, c) {
    var args = foo(b, c, 42);
    return baz.apply(void 0, args);
}

noInline(bar);

for (var i = 0; i < 10000; ++i) {
    var result = bar(1, 2, 3);
    if (result != 47)
        throw "Error: bad result: " + result;
}
Comment 6 Filip Pizlo 2015-03-30 09:36:17 PDT
Created attachment 249738 [details]
hopefully the patch

Not yet ready for review.  I'm still running tests.
Comment 7 Filip Pizlo 2015-03-30 09:58:40 PDT
Comment on attachment 249738 [details]
hopefully the patch

Looks like it works!
Comment 8 Geoffrey Garen 2015-03-30 10:50:44 PDT
Comment on attachment 249738 [details]
hopefully the patch

r=me
Comment 9 WebKit Commit Bot 2015-03-30 11:36:55 PDT
Comment on attachment 249738 [details]
hopefully the patch

Clearing flags on attachment: 249738

Committed r182148: <http://trac.webkit.org/changeset/182148>
Comment 10 WebKit Commit Bot 2015-03-30 11:37:00 PDT
All reviewed patches have been landed.  Closing bug.