Bug 149066 – REGRESSION(r189585): run-perf-tests Speedometer fails with a console error

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryosuke Niwa]

At r189585:

$ ./Tools/Scripts/run-perf-tests PerformanceTests/Speedometer/ ~/Desktop/speedometer.json 
Path was not found:/Users/rniwa/Desktop/speedometer.json
Running 1 tests
Running Speedometer/Full.html (1 of 1)
RESULT Speedometer: Full: Time= 5276.43625 ms
median= 5279.8225 ms, stdev= 54.5255277219 ms, min= 5151.865 ms, max= 5360.485 ms
Finished: 121.690676 s

At r189586:

$ ./Tools/Scripts/run-perf-tests PerformanceTests/Speedometer/ ~/Desktop/speedometer.json
Path was not found:/Users/rniwa/Desktop/speedometer.json
Running 1 tests
Running Speedometer/Full.html (1 of 1)
ERROR: CONSOLE MESSAGE: line 3285: Cannot call get with 'isDeleted' on an undefined object.
FAILED
Finished: 29.483498 s

[Radar WebKit Bug Importer]

<rdar://problem/22661022>

[Filip Pizlo]

Looking at this now.

[Filip Pizlo]

Created attachment 261047 [details]
the patch

[Filip Pizlo]

Test case:

//@ runMiscNoCJITTest("--useLLInt=false", "--useDFGJIT=false")

function foo(o) {
    return o.i7;
}

var o = {};
for (var i = 0; i < 100; ++i)
    o["i" + i] = i;
for (var i = 0; i < 100; i+=2)
    delete o["i" + i];

for (var i = 0; i < 100; ++i) {
    var result = foo(o);
    if (result != 7)
        throw "Error: bad result: " + result;
}

[Filip Pizlo]

Created attachment 261048 [details]
the patch

And now, with a test.

[Michael Saboff]

Comment on attachment 261048 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261048&action=review

> Source/JavaScriptCore/jit/Repatch.cpp:214
> +static bool forceICFailure(ExecState*)

Why the unused ExecState* parameter?

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:289
> +inline bool shouldJIT(ExecState* exec, CodeBlock*)

Why the unused CodeBlock* parameter?

[Filip Pizlo]

(In reply to comment #6)
> Comment on attachment 261048 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261048&action=review
> 
> > Source/JavaScriptCore/jit/Repatch.cpp:214
> > +static bool forceICFailure(ExecState*)
> 
> Why the unused ExecState* parameter?
> 
> > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:289
> > +inline bool shouldJIT(ExecState* exec, CodeBlock*)
> 
> Why the unused CodeBlock* parameter?

So it's easier to insert bisecting code based on code block or the current code origin.

[Michael Saboff]

Comment on attachment 261048 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261048&action=review

r=me

> Source/JavaScriptCore/ChangeLog:21
> +        Debugging bugs like these requires adding ad hoc bisection code in various places. We already
> +        had the basic hooks for this. This patch makes those hooks a bit more useful.

Add that the two unused parameter additions are for this reason.

[Filip Pizlo]

(In reply to comment #8)
> Comment on attachment 261048 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261048&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:21
> > +        Debugging bugs like these requires adding ad hoc bisection code in various places. We already
> > +        had the basic hooks for this. This patch makes those hooks a bit more useful.
> 
> Add that the two unused parameter additions are for this reason.

Done.  Thanks!

[Filip Pizlo]

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