Bug 150760 - Wrong value recovery for DFG try/catch with a getter that throws during an IC miss
Summary: Wrong value recovery for DFG try/catch with a getter that throws during an IC...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks: 149409
  Show dependency treegraph
 
Reported: 2015-10-31 16:22 PDT by Saam Barati
Modified: 2015-11-02 17:20 PST (History)
11 users (show)

See Also:


Attachments
patch (4.96 KB, patch)
2015-11-02 16:49 PST, Saam Barati
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 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.
Comment 1 Saam Barati 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.
Comment 2 Saam Barati 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.
Comment 3 Saam Barati 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().
Comment 4 Saam Barati 2015-11-02 16:49:22 PST
Created attachment 264646 [details]
patch
Comment 5 Geoffrey Garen 2015-11-02 16:51:03 PST
Comment on attachment 264646 [details]
patch

r=me
Comment 6 WebKit Commit Bot 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.
Comment 7 Saam Barati 2015-11-02 17:20:26 PST
landed in:
http://trac.webkit.org/changeset/191930