This seems to cause trouble: a: CreateActivation() MovHint(@a, loc1) b: NewFunction(@a) MovHint(@b, loc2) c: NewFunction(@b) MovHint(@c, loc3) Escape(@b) I'm seeing code like this: a: PhantomCreateActivation() MovHint(@a, loc1) b: PhantomNewFunction(@a) PutHint(@b, @a, scope) MovHint(@b, loc2) c: PhantomNewFunction(@b) PutHint(@c, @a, scope) MovHint(@c, loc3) d: MaterializeCreateActivation() e: NewFunction(@d) MovHint(@d, loc1) MovHint(@e, loc2) Escape(@e) But this is wrong, because if we exit, then we will recreate @c pointing at a newly materialized activation, rather than at @d. In other words, the phase should have produced this code instead: a: PhantomCreateActivation() MovHint(@a, loc1) b: PhantomNewFunction(@a) PutHint(@b, @a, scope) MovHint(@b, loc2) c: PhantomNewFunction(@b) PutHint(@c, @a, scope) MovHint(@c, loc3) d: MaterializeCreateActivation() e: NewFunction(@d) PutHint(@c, @e, scope) <---- THIS MovHint(@d, loc1) MovHint(@e, loc2) Escape(@e) The phase seems to have code to do this. I'm still investigating what goes wrong.
It looks like this is handled by m_materializationSiteToRecoveries. I wonder why the scope isn't in there.
100% repro: function bar() { } noInline(bar); function foo(p, x) { var value = 1; function inc() { return value + 1; } function dec() { return value - 1; } if (!p) return 0; bar(inc); x += 2000000000; value = 42; return dec(); } noInline(foo); function test(x) { var result = foo(true, x); if (result != 42 - 1) throw "Error: bad result: " + result; } for (var i = 0; i < 100000; ++i) test(0); test(2000000000);
Wow this is an epic one-line fix!
Created attachment 287648 [details] the patch
Comment on attachment 287648 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=287648&action=review r=me > Source/JavaScriptCore/ChangeLog:16 > + it's a special meta-data field initialed on construction. But just because it's immutable /initialed/initialized/ ? > Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:1942 > - > + Please remove.
(In reply to comment #5) > Comment on attachment 287648 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=287648&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:16 > > + it's a special meta-data field initialed on construction. But just because it's immutable > > /initialed/initialized/ ? Fixed. > > > Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:1942 > > - > > + > > Please remove. Fixed.
Landed in http://trac.webkit.org/changeset/205304
<rdar://problem/28120011>