Bug 150760

Summary: Wrong value recovery for DFG try/catch with a getter that throws during an IC miss
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, sukolsak, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149409    
Attachments:
Description Flags
patch ggaren: review+

Saam Barati
Reported 2015-10-31 16:22:13 PDT
This program breaks in the DFG. Probably because we're assigning the base and the result to the same register and the IC fails. ``` function assert(b) { if (!b) throw new Error("bad value") } noInline(assert); let oThrow = { x: 20, y: 40, z: 50, get f() { throw new Error("Hello World!"); } }; let o1 = { x: 20, f: 40 }; let o2 = { x: 20, y: 50, get f() { return 20; } }; function foo(f) { let o = f(); try { o = o.f; } catch(e) { print(o); // Make this not undefined. assert(o === oThrow); } } noInline(foo); let i; let flag = false; function f() { if (flag) return oThrow; if (i % 2) return o1; return o2; } noInline(f); for (i = 0; i < 10000; i++) { foo(f); } flag = true; foo(f); ``` I discovered this while solving this issue in FTL try/catch.
Attachments
patch (4.96 KB, patch)
2015-11-02 16:49 PST, Saam Barati 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2015-10-31 16:26:16 PDT
What I think is happening. The IC misses, but the operationGetByIdOptimize stores the result of the call into the result register. Even though the C call throws, we store the result. And then, we try to recover the base from the register we stored the result into because the result and base are the same register. This is a big bag of fail.
Saam Barati
Comment 2 2015-11-02 16:01:23 PST
(In reply to comment #1) > What I think is happening. > The IC misses, but the operationGetByIdOptimize stores the result > of the call into the result register. Even though the C call throws, > we store the result. And then, we try to recover the base from > the register we stored the result into because the result and base > are the same register. This is a big bag of fail. I've actually realized that the problem is deeper than this. I do think the situation I'm describing is a problem, but I've realized that the result of the call in this example is looking dead to DFG OSR exit when we use PhantomLocal as the liveness preservation mechanism. I'm going to switch the code back to using Flush so that I can land FTL try/catch, and then I will investigate this problem further.
Saam Barati
Comment 3 2015-11-02 16:03:07 PST
(In reply to comment #2) > (In reply to comment #1) > > What I think is happening. > > The IC misses, but the operationGetByIdOptimize stores the result > > of the call into the result register. Even though the C call throws, > > we store the result. And then, we try to recover the base from > > the register we stored the result into because the result and base > > are the same register. This is a big bag of fail. > > I've actually realized that the problem is deeper than this. > I do think the situation I'm describing is a problem, but > I've realized that the result of the call in this example > is looking dead to DFG OSR exit when we use PhantomLocal > as the liveness preservation mechanism. I'm going > to switch the code back to using Flush so that I can > land FTL try/catch, and then I will investigate this problem > further. Let me elaborate: the result of the call looks dead after we do flushRegisters().
Saam Barati
Comment 4 2015-11-02 16:49:22 PST
Created attachment 264646 [details] patch
Geoffrey Garen
Comment 5 2015-11-02 16:51:03 PST
Comment on attachment 264646 [details] patch r=me
WebKit Commit Bot
Comment 6 2015-11-02 16:52:04 PST
Attachment 264646 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:133: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 7 2015-11-02 17:20:26 PST
landed in: http://trac.webkit.org/changeset/191930
Note You need to log in before you can comment on or make changes to this bug.