| Summary: | Going to google.com/trends causes a crash | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | jacoco <cococohen1122> | ||||||
| Component: | JavaScriptCore | Assignee: | Oliver Hunt <oliver> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | fpizlo, oliver | ||||||
| Priority: | P1 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Mac (Intel) | ||||||||
| OS: | OS X 10.8 | ||||||||
| Attachments: |
|
||||||||
|
Description
jacoco
2013-06-13 10:35:57 PDT
function g() { f() }
function f() { doStuff(); arguments = undefined; }
function doStuff() { throw {} }
for (var i = 0; i < 100; i++) { try { g() } catch (e) {} }
Okay, so if we have:
function g() { f() }
function f() { doStuff(); arguments; }
function doStuff() { throw {} }
for (var i = 0; i < 100; i++) { try { g() } catch (e) {} }
The initial graph is:
DFG for g#BBXOF8:[0x7fe6ad00f000->0x10935fd70, DFGFunctionCall]:
Fixpoint state: BeforeFixpoint; Form: LoadStore; Unification state: LocallyUnified; Ref count state: EverythingIsLive
ArgumentPosition size: 3
#0:
#1:
#2:
Block #0 (bc#0): (OSR target)
Predecessors:
Phi Nodes:
vars before: <empty>
var links: arg0:- : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:-
0: < 1:-> SetArgument(arg0(a), bc#0) predicting None
1: < 1:-> JSConstant(JS|PureInt, $0 = Undefined, bc#1)
2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1)
3: < 1:-> WeakJSConstant(JS|PureInt, 0x1092decb0, bc#1)
4: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(B~), bc#1) predicting None
5: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(C~), bc#1) predicting None
6: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(D~), bc#1) predicting None
7: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(E~), bc#1) predicting None
8: <!0:-> Phantom(@3, @1, MustGen|CanExit, bc#7)
--> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8>
9: <!0:-> InlineStart(MustGen, bc#0)
10: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r8(F*), bc#0) predicting None
11: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r9(G*), bc#0) predicting None
12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1)
13: <!0:-> Flush(MustGen, r9(G*), bc#1) predicting None
14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting None
15: <!0:-> Flush(MustGen, r8(F*), bc#3) predicting None
16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting None
17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5)
18: < 1:-> WeakJSConstant(JS|PureInt, 0x1092dec70, bc#5)
19: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(J~), bc#5) predicting None
20: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(K~), bc#5) predicting None
21: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(L~), bc#5) predicting None
22: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(M~), bc#5) predicting None
23: <!0:-> Phantom(@18, @1, MustGen|CanExit, bc#11)
--> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18>
24: <!0:-> InlineStart(MustGen, bc#0)
25: < 1:-> NewObject(JS|PureInt|CanExit, struct(0x105ddb6c8: NonArray), bc#1)
26: < 1:-> SetLocal(@25, CanExit|NodeExitsForward, r18(N~), bc#1) predicting None
27: <!0:-> Throw(@25, MustGen|CanExit, bc#5)
Which eventually becomes:
var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:-
0: < 1:-> SetArgument(arg0(a), bc#0) predicting Other
1: < 1:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1)
2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1)
3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1)
4: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(B~<Other>), bc#1) predicting Other
5: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(C~<Function>), bc#1) predicting Function
6: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(D~<Other>), bc#1) predicting Other
7: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(E~<Function>), bc#1) predicting Function
8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen|CanExit, bc#7)
--> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8>
9: <!0:-> InlineStart(MustGen, bc#0)
10: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r8(F*<Other>), bc#0) predicting Other
11: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r9(G*<Other>), bc#0) predicting Other
12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1)
13: <!0:-> Flush(@11, MustGen, r9(G*<Other>), bc#1) predicting Other
14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting Empty
15: <!0:-> Flush(@10, MustGen, r8(F*<Other>), bc#3) predicting Other
16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting Empty
17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5)
18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5)
19: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(J~<Other>), bc#5) predicting Other
20: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(K~<Function>), bc#5) predicting Function
21: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(L~<Other>), bc#5) predicting Other
22: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(M~<Function>), bc#5) predicting Function
23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen|CanExit, bc#11)
--> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18>
24: <!0:-> InlineStart(MustGen, bc#0)
25: < 1:-> NewObject(JS|UseAsOther|CanExit, struct(0x105ddb6c8: NonArray), bc#1)
26: < 1:-> SetLocal(@25<Final>, CanExit|NodeExitsForward, r18(N~<Final>), bc#1) predicting Final
27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5)
And then after DCE
var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:-
0: skipped < 0:-> SetArgument(arg0(a), bc#0)
1: < 4:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1)
2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1)
3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1)
4: skipped < 0:-> MovHint(@1<Other>, r1(B~<Other>), bc#1)
5: skipped < 0:-> MovHint(@3<Function>, r0(C~<Function>), bc#1)
6: skipped < 0:-> MovHint(@1<Other>, r1(D~<Other>), bc#1)
7: skipped < 0:-> MovHint(@3<Function>, r0(E~<Function>), bc#1)
8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen, bc#7)
--> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8>
9: <!0:-> InlineStart(MustGen, bc#0)
10: skipped < 0:-> MovHint(@1<Other>, r8(F*<Other>), bc#0)
11: skipped < 0:-> MovHint(@1<Other>, r9(G*<Other>), bc#0)
12: <!0:-> Phantom(MustGen|CanExit, bc#1)
13: <!0:-> Phantom(@1<Other>, MustGen, bc#1)
14: skipped < 0:-> ZombieHint(r9(H*), bc#1)
15: <!0:-> Phantom(@1<Other>, MustGen, bc#3)
16: skipped < 0:-> ZombieHint(r8(I*), bc#3)
17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5)
18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5)
19: skipped < 0:-> MovHint(@1<Other>, r11(J~<Other>), bc#5)
20: skipped < 0:-> MovHint(@18<Function>, r10(K~<Function>), bc#5)
21: skipped < 0:-> MovHint(@1<Other>, r11(L~<Other>), bc#5)
22: skipped < 0:-> MovHint(@18<Function>, r10(M~<Function>), bc#5)
23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen, bc#11)
--> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18>
24: <!0:-> InlineStart(MustGen, bc#0)
25: < 1:-> NewObject(JS|UseAsOther, struct(0x105ddb6c8: NonArray), bc#1)
26: skipped < 0:-> MovHint(@25<Final>, r18(N~<Final>), bc#1)
27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5)
Nodes @14 and @16 were responsible for initializing the lazy argument slots, but they have been elided, despite being necessary for correct behavior.
I can add
addToGraph(Phantom, get(currentInstruction[1].u.operand));
To init_lazy_reg, and that makes this crash go away, but it feels jacky, and looking at the output graph it is not obvious _why_ we end up with the correct behavior.
(In reply to comment #3) > Okay, so if we have: > function g() { f() } > function f() { doStuff(); arguments; } > function doStuff() { throw {} } > > for (var i = 0; i < 100; i++) { try { g() } catch (e) {} } > > > > The initial graph is: > DFG for g#BBXOF8:[0x7fe6ad00f000->0x10935fd70, DFGFunctionCall]: > Fixpoint state: BeforeFixpoint; Form: LoadStore; Unification state: LocallyUnified; Ref count state: EverythingIsLive > ArgumentPosition size: 3 > #0: > #1: > #2: > Block #0 (bc#0): (OSR target) > Predecessors: > Phi Nodes: > vars before: <empty> > var links: arg0:- : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > 0: < 1:-> SetArgument(arg0(a), bc#0) predicting None > 1: < 1:-> JSConstant(JS|PureInt, $0 = Undefined, bc#1) > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > 3: < 1:-> WeakJSConstant(JS|PureInt, 0x1092decb0, bc#1) > 4: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(B~), bc#1) predicting None > 5: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(C~), bc#1) predicting None > 6: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(D~), bc#1) predicting None > 7: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(E~), bc#1) predicting None > 8: <!0:-> Phantom(@3, @1, MustGen|CanExit, bc#7) > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > 9: <!0:-> InlineStart(MustGen, bc#0) > 10: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r8(F*), bc#0) predicting None > 11: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r9(G*), bc#0) predicting None > 12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1) > 13: <!0:-> Flush(MustGen, r9(G*), bc#1) predicting None > 14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting None > 15: <!0:-> Flush(MustGen, r8(F*), bc#3) predicting None > 16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting None Are 14 and 16 flushed? > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > 18: < 1:-> WeakJSConstant(JS|PureInt, 0x1092dec70, bc#5) > 19: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(J~), bc#5) predicting None > 20: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(K~), bc#5) predicting None > 21: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(L~), bc#5) predicting None > 22: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(M~), bc#5) predicting None > 23: <!0:-> Phantom(@18, @1, MustGen|CanExit, bc#11) > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > 24: <!0:-> InlineStart(MustGen, bc#0) > 25: < 1:-> NewObject(JS|PureInt|CanExit, struct(0x105ddb6c8: NonArray), bc#1) > 26: < 1:-> SetLocal(@25, CanExit|NodeExitsForward, r18(N~), bc#1) predicting None > 27: <!0:-> Throw(@25, MustGen|CanExit, bc#5) > > > Which eventually becomes: > var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > 0: < 1:-> SetArgument(arg0(a), bc#0) predicting Other > 1: < 1:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1) > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > 3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1) > 4: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(B~<Other>), bc#1) predicting Other > 5: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(C~<Function>), bc#1) predicting Function > 6: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(D~<Other>), bc#1) predicting Other > 7: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(E~<Function>), bc#1) predicting Function > 8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen|CanExit, bc#7) > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > 9: <!0:-> InlineStart(MustGen, bc#0) > 10: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r8(F*<Other>), bc#0) predicting Other > 11: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r9(G*<Other>), bc#0) predicting Other > 12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1) > 13: <!0:-> Flush(@11, MustGen, r9(G*<Other>), bc#1) predicting Other > 14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting Empty > 15: <!0:-> Flush(@10, MustGen, r8(F*<Other>), bc#3) predicting Other > 16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting Empty > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > 18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5) > 19: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(J~<Other>), bc#5) predicting Other > 20: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(K~<Function>), bc#5) predicting Function > 21: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(L~<Other>), bc#5) predicting Other > 22: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(M~<Function>), bc#5) predicting Function > 23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen|CanExit, bc#11) > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > 24: <!0:-> InlineStart(MustGen, bc#0) > 25: < 1:-> NewObject(JS|UseAsOther|CanExit, struct(0x105ddb6c8: NonArray), bc#1) > 26: < 1:-> SetLocal(@25<Final>, CanExit|NodeExitsForward, r18(N~<Final>), bc#1) predicting Final > 27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5) > > > And then after DCE > var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > 0: skipped < 0:-> SetArgument(arg0(a), bc#0) > 1: < 4:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1) > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > 3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1) > 4: skipped < 0:-> MovHint(@1<Other>, r1(B~<Other>), bc#1) > 5: skipped < 0:-> MovHint(@3<Function>, r0(C~<Function>), bc#1) > 6: skipped < 0:-> MovHint(@1<Other>, r1(D~<Other>), bc#1) > 7: skipped < 0:-> MovHint(@3<Function>, r0(E~<Function>), bc#1) > 8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen, bc#7) > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > 9: <!0:-> InlineStart(MustGen, bc#0) > 10: skipped < 0:-> MovHint(@1<Other>, r8(F*<Other>), bc#0) > 11: skipped < 0:-> MovHint(@1<Other>, r9(G*<Other>), bc#0) > 12: <!0:-> Phantom(MustGen|CanExit, bc#1) > 13: <!0:-> Phantom(@1<Other>, MustGen, bc#1) > 14: skipped < 0:-> ZombieHint(r9(H*), bc#1) > 15: <!0:-> Phantom(@1<Other>, MustGen, bc#3) > 16: skipped < 0:-> ZombieHint(r8(I*), bc#3) > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > 18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5) > 19: skipped < 0:-> MovHint(@1<Other>, r11(J~<Other>), bc#5) > 20: skipped < 0:-> MovHint(@18<Function>, r10(K~<Function>), bc#5) > 21: skipped < 0:-> MovHint(@1<Other>, r11(L~<Other>), bc#5) > 22: skipped < 0:-> MovHint(@18<Function>, r10(M~<Function>), bc#5) > 23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen, bc#11) > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > 24: <!0:-> InlineStart(MustGen, bc#0) > 25: < 1:-> NewObject(JS|UseAsOther, struct(0x105ddb6c8: NonArray), bc#1) > 26: skipped < 0:-> MovHint(@25<Final>, r18(N~<Final>), bc#1) > 27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5) > > > Nodes @14 and @16 were responsible for initializing the lazy argument slots, but they have been elided, despite being necessary for correct behavior. > > I can add > addToGraph(Phantom, get(currentInstruction[1].u.operand)); > > To init_lazy_reg, and that makes this crash go away, but it feels jacky, and looking at the output graph it is not obvious _why_ we end up with the correct behavior. No that is definitely not the right solution. Why aren't those two nodes flushed? Does the inkined functional ways end in Throw? If so maybe it's that op_throw (and ThrowReferenceError) aren't doing the Flushing that op_ret and op_end do? (In reply to comment #4) > (In reply to comment #3) > > Okay, so if we have: > > function g() { f() } > > function f() { doStuff(); arguments; } > > function doStuff() { throw {} } > > > > for (var i = 0; i < 100; i++) { try { g() } catch (e) {} } > > > > > > > > The initial graph is: > > DFG for g#BBXOF8:[0x7fe6ad00f000->0x10935fd70, DFGFunctionCall]: > > Fixpoint state: BeforeFixpoint; Form: LoadStore; Unification state: LocallyUnified; Ref count state: EverythingIsLive > > ArgumentPosition size: 3 > > #0: > > #1: > > #2: > > Block #0 (bc#0): (OSR target) > > Predecessors: > > Phi Nodes: > > vars before: <empty> > > var links: arg0:- : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > > 0: < 1:-> SetArgument(arg0(a), bc#0) predicting None > > 1: < 1:-> JSConstant(JS|PureInt, $0 = Undefined, bc#1) > > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > > 3: < 1:-> WeakJSConstant(JS|PureInt, 0x1092decb0, bc#1) > > 4: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(B~), bc#1) predicting None > > 5: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(C~), bc#1) predicting None > > 6: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r1(D~), bc#1) predicting None > > 7: < 1:-> SetLocal(@3, CanExit|NodeExitsForward, r0(E~), bc#1) predicting None > > 8: <!0:-> Phantom(@3, @1, MustGen|CanExit, bc#7) > > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > > 9: <!0:-> InlineStart(MustGen, bc#0) > > 10: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r8(F*), bc#0) predicting None > > 11: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r9(G*), bc#0) predicting None > > 12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1) > > 13: <!0:-> Flush(MustGen, r9(G*), bc#1) predicting None > > 14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting None > > 15: <!0:-> Flush(MustGen, r8(F*), bc#3) predicting None > > 16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting None > > Are 14 and 16 flushed? > > > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > > 18: < 1:-> WeakJSConstant(JS|PureInt, 0x1092dec70, bc#5) > > 19: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(J~), bc#5) predicting None > > 20: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(K~), bc#5) predicting None > > 21: < 1:-> SetLocal(@1, CanExit|NodeExitsForward, r11(L~), bc#5) predicting None > > 22: < 1:-> SetLocal(@18, CanExit|NodeExitsForward, r10(M~), bc#5) predicting None > > 23: <!0:-> Phantom(@18, @1, MustGen|CanExit, bc#11) > > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > > 24: <!0:-> InlineStart(MustGen, bc#0) > > 25: < 1:-> NewObject(JS|PureInt|CanExit, struct(0x105ddb6c8: NonArray), bc#1) > > 26: < 1:-> SetLocal(@25, CanExit|NodeExitsForward, r18(N~), bc#1) predicting None > > 27: <!0:-> Throw(@25, MustGen|CanExit, bc#5) > > > > > > Which eventually becomes: > > var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > > 0: < 1:-> SetArgument(arg0(a), bc#0) predicting Other > > 1: < 1:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1) > > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > > 3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1) > > 4: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(B~<Other>), bc#1) predicting Other > > 5: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(C~<Function>), bc#1) predicting Function > > 6: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r1(D~<Other>), bc#1) predicting Other > > 7: < 1:-> SetLocal(@3<Function>, CanExit|NodeExitsForward, r0(E~<Function>), bc#1) predicting Function > > 8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen|CanExit, bc#7) > > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > > 9: <!0:-> InlineStart(MustGen, bc#0) > > 10: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r8(F*<Other>), bc#0) predicting Other > > 11: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r9(G*<Other>), bc#0) predicting Other > > 12: < 1:-> JSConstant(JS|PureInt, $1 = <JSValue()>, bc#1) > > 13: <!0:-> Flush(@11, MustGen, r9(G*<Other>), bc#1) predicting Other > > 14: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r9(H*), bc#1) predicting Empty > > 15: <!0:-> Flush(@10, MustGen, r8(F*<Other>), bc#3) predicting Other > > 16: < 1:-> SetLocal(@12, CanExit|NodeExitsForward, r8(I*), bc#3) predicting Empty > > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > > 18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5) > > 19: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(J~<Other>), bc#5) predicting Other > > 20: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(K~<Function>), bc#5) predicting Function > > 21: < 1:-> SetLocal(@1<Other>, CanExit|NodeExitsForward, r11(L~<Other>), bc#5) predicting Other > > 22: < 1:-> SetLocal(@18<Function>, CanExit|NodeExitsForward, r10(M~<Function>), bc#5) predicting Function > > 23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen|CanExit, bc#11) > > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > > 24: <!0:-> InlineStart(MustGen, bc#0) > > 25: < 1:-> NewObject(JS|UseAsOther|CanExit, struct(0x105ddb6c8: NonArray), bc#1) > > 26: < 1:-> SetLocal(@25<Final>, CanExit|NodeExitsForward, r18(N~<Final>), bc#1) predicting Final > > 27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5) > > > > > > And then after DCE > > var links: arg0:@0 : r0:- r1:- r2:- r3:- r4:- r5:- r6:- r7:- r8:- r9:- r10:- r11:- r12:- r13:- r14:- r15:- r16:- r17:- r18:- > > 0: skipped < 0:-> SetArgument(arg0(a), bc#0) > > 1: < 4:-> JSConstant(JS|UseAsOther, $0 = Undefined, bc#1) > > 2: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global4(0x105dff9c8), bc#1) > > 3: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092decb0, bc#1) > > 4: skipped < 0:-> MovHint(@1<Other>, r1(B~<Other>), bc#1) > > 5: skipped < 0:-> MovHint(@3<Function>, r0(C~<Function>), bc#1) > > 6: skipped < 0:-> MovHint(@1<Other>, r1(D~<Other>), bc#1) > > 7: skipped < 0:-> MovHint(@3<Function>, r0(E~<Function>), bc#1) > > 8: <!0:-> Phantom(@3<Function>, @1<Other>, MustGen, bc#7) > > --> f#AmUsAH:<0x10935fc70, bc#7, Call, known callee: Cell: 0x1092decb0 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r8> > > 9: <!0:-> InlineStart(MustGen, bc#0) > > 10: skipped < 0:-> MovHint(@1<Other>, r8(F*<Other>), bc#0) > > 11: skipped < 0:-> MovHint(@1<Other>, r9(G*<Other>), bc#0) > > 12: <!0:-> Phantom(MustGen|CanExit, bc#1) > > 13: <!0:-> Phantom(@1<Other>, MustGen, bc#1) > > 14: skipped < 0:-> ZombieHint(r9(H*), bc#1) > > 15: <!0:-> Phantom(@1<Other>, MustGen, bc#3) > > 16: skipped < 0:-> ZombieHint(r8(I*), bc#3) > > 17: <!0:-> GlobalVarWatchpoint(MustGen|CanExit, global5(0x105dff9d0), bc#5) > > 18: < 1:-> WeakJSConstant(JS|UseAsOther, 0x1092dec70, bc#5) > > 19: skipped < 0:-> MovHint(@1<Other>, r11(J~<Other>), bc#5) > > 20: skipped < 0:-> MovHint(@18<Function>, r10(K~<Function>), bc#5) > > 21: skipped < 0:-> MovHint(@1<Other>, r11(L~<Other>), bc#5) > > 22: skipped < 0:-> MovHint(@18<Function>, r10(M~<Function>), bc#5) > > 23: <!0:-> Phantom(@18<Function>, @1<Other>, MustGen, bc#11) > > --> doStuff#ASE1VK:<0x10935fb70, bc#11, Call, known callee: Cell: 0x1092dec70 (0x105ddf420: Function, NonArray), numArgs+this = 1, stack >= r18> > > 24: <!0:-> InlineStart(MustGen, bc#0) > > 25: < 1:-> NewObject(JS|UseAsOther, struct(0x105ddb6c8: NonArray), bc#1) > > 26: skipped < 0:-> MovHint(@25<Final>, r18(N~<Final>), bc#1) > > 27: <!0:-> Throw(@25<Final>, MustGen|CanExit, bc#5) > > > > > > Nodes @14 and @16 were responsible for initializing the lazy argument slots, but they have been elided, despite being necessary for correct behavior. > > > > I can add > > addToGraph(Phantom, get(currentInstruction[1].u.operand)); > > > > To init_lazy_reg, and that makes this crash go away, but it feels jacky, and looking at the output graph it is not obvious _why_ we end up with the correct behavior. > > No that is definitely not the right solution. Why aren't those two nodes flushed? Does the inkined functional ways end in Throw? If so maybe it's that op_throw (and ThrowReferenceError) aren't doing the Flushing that op_ret and op_end do? Just reread the original bug code. Yeah, the bug is that op_throw doesn't do the flushing that op_ret does. This can be fixed by either copying or abstracting the code in ByteCodeParser for flushing in return, so that throwing also does it.
> Just reread the original bug code. Yeah, the bug is that op_throw doesn't do the flushing that op_ret does. This can be fixed by either copying or abstracting the code in ByteCodeParser for flushing in return, so that throwing also does it.
Yeah, i was coming to the conclusion as well (there's another bug i cc'd you on which I think is just another symptom of this)
randomly should op_throw be flagged as clobbers the world?
(In reply to comment #6) > > Just reread the original bug code. Yeah, the bug is that op_throw doesn't do the flushing that op_ret does. This can be fixed by either copying or abstracting the code in ByteCodeParser for flushing in return, so that throwing also does it. > > Yeah, i was coming to the conclusion as well (there's another bug i cc'd you on which I think is just another symptom of this) > > randomly should op_throw be flagged as clobbers the world? Although my reading of the code implies that throw, etc think that they _are_ flushing everything (In reply to comment #6) > > Just reread the original bug code. Yeah, the bug is that op_throw doesn't do the flushing that op_ret does. This can be fixed by either copying or abstracting the code in ByteCodeParser for flushing in return, so that throwing also does it. > > Yeah, i was coming to the conclusion as well (there's another bug i cc'd you on which I think is just another symptom of this) > > randomly should op_throw be flagged as clobbers the world? No, throwing doesn't clobber, for the same reason that return doesn't: it's a terminal. (In reply to comment #7) > (In reply to comment #6) > > > Just reread the original bug code. Yeah, the bug is that op_throw doesn't do the flushing that op_ret does. This can be fixed by either copying or abstracting the code in ByteCodeParser for flushing in return, so that throwing also does it. > > > > Yeah, i was coming to the conclusion as well (there's another bug i cc'd you on which I think is just another symptom of this) > > > > randomly should op_throw be flagged as clobbers the world? > > Although my reading of the code implies that throw, etc think that they _are_ flushing everything Lololololo. I see the bug and its awesome. Throw is doing the right thing, if throw behaved like return. Return just "pops" the function you're returning from. But throw pops all functions we've inkined: it is a hard terminal. So throw must flush eveything but it only flushes things from inline stack top. That's why the bug manifests if you throw from something that is inlined into an arguments-using function; presumably throwing directly won't break it. Just make throw flush things for all inline stack entries and not just the top one. *** Bug 117749 has been marked as a duplicate of this bug. *** Okay I believe i have this fixed Created attachment 204956 [details]
Patch
Comment on attachment 204956 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204956&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:452 > + for (InlineStackEntry* stack = m_inlineStackTop; stack; stack = stack->m_caller) { I'd call this variable "entry", not "stack", since it's an entry in the stack, and not the whole stack. Created attachment 204958 [details]
Patch
Comment on attachment 204958 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204958&action=review r=me > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:458 > + void flushInlineStackEntry(InlineStackEntry* inlineStackEntry) You can just call this "flush", since the type disambiguates in C++. Committed r151709: <http://trac.webkit.org/changeset/151709> |