Closed
Bug 496185
Opened 15 years ago
Closed 15 years ago
TM: wrong number with >>>, various types of loops
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
RESOLVED
DUPLICATE
of bug 495958
mozilla1.9.1
People
(Reporter: jruderman, Assigned: gal)
Details
(Keywords: testcase, Whiteboard: [sg:dupe 495958])
Attachments
(3 files, 4 obsolete files)
8.14 KB,
patch
|
Details | Diff | Splinter Review | |
6.07 KB,
text/plain
|
Details | |
1.07 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
var v = 0; for each (var a in [0, 1]) { print(v); for (var b = 0; b < 3; ++b) { v = a >>> 0; for each (var c in [{}, {}, {}, 1]) { } } } Without JIT: 0 0 With JIT: 0 -6.277435978499887e+66 I think this is a recent regression, since my fuzzer is hitting it a lot this morning.
Assignee | ||
Comment 1•15 years ago
|
||
****. Can we bisect?
Assignee | ||
Comment 2•15 years ago
|
||
Confirmed with TM tip. Not a happy day.
Flags: blocking1.9.1?
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1
Assignee | ||
Comment 3•15 years ago
|
||
Regressed by: changeset: 28910:daf5763d7da4 user: Andreas Gal <gal@uci.edu> date: Tue Jun 02 22:28:59 2009 -0400 summary: Bug 495897 - Assertion failure: ti->typeMap.matches(ti_other->typeMap) with undeclared global. r=dvander
Updated•15 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Assignee | ||
Comment 4•15 years ago
|
||
My best guess here is that the inner loop gets specialized further and becomes incompatible with the loop that embeds it. It will take a little while to confirm this.
Updated•15 years ago
|
Assignee: general → gal
Assignee | ||
Comment 5•15 years ago
|
||
graydon is helping too
Assignee | ||
Comment 6•15 years ago
|
||
Reporter | ||
Comment 7•15 years ago
|
||
var v = 0; for each (var e in [1, 3, 1, 1]) { v += true; for each (var h in [4, 4, 4, {}, 4, {}]) { for (var a = 0; a < 1; ++a) { } } } print(v); Without JIT: 4 With JIT: -6.27743597849989e+66
Comment 8•15 years ago
|
||
Reporter | ||
Comment 9•15 years ago
|
||
for each (var d in [1, 1, 1, 1]) { d >>>= 0; for (var c = 0; c < 2; ++c) { ("" + function () {}); } } print(d); Without JIT: 1 With JIT: -6.2774359784998874e+66
Assignee | ||
Comment 10•15 years ago
|
||
Slight simplification: var c = 0; for each (var d in [1, 1, 1, 1]) { d >>>= 0; for (var c = 0; c < 2; ++c) { +function () {}; } } print(d);
Assignee | ||
Comment 11•15 years ago
|
||
Attachment #381411 -
Attachment is obsolete: true
Attachment #381435 -
Attachment is obsolete: true
Assignee | ||
Comment 12•15 years ago
|
||
Attachment #381443 -
Attachment is obsolete: true
Assignee | ||
Comment 13•15 years ago
|
||
Attachment #381444 -
Attachment is obsolete: true
Assignee | ||
Comment 14•15 years ago
|
||
I am pretty confident these are dups: +[(e = {}, (function () e)()) for each (e in ["", {}, "", {}, ""])][4]; Crash [@ js_ValueToNumber] touching 0x20000000 +[(e = {}, (function () e)()) for each (e in ["", {}, "", {}, ""])]; Crash [@ JS_Enumerate] touching 0x20000000 https://bugzilla.mozilla.org/show_bug.cgi?id=496270 Might be easier to work with these.
Group: core-security
Assignee | ||
Comment 15•15 years ago
|
||
Actually never mind #14. #14 looks like upvar on trace.
Comment 16•15 years ago
|
||
Here's a log of the minimal run above. It's kinda nutty. There's no emitted treecall -- just a couple executed treecalls while recording the outer loop that BRANCH_EXIT -- and the only dependency that grows is between two differently type-specialized peers on the inner tree. The outer->inner call appears to be type compatible. I'm perplexed, but maybe this helps shed light? I'll keep looking.
Assignee | ||
Comment 17•15 years ago
|
||
Attachment #381664 -
Flags: review?
Assignee | ||
Comment 18•15 years ago
|
||
I think we were digging in the wrong direction here. Look at this: This is the last invocation of FlushNativeStackFrame. (gdb) x/8bx &np[83] 0xbfffcea0: 0xcd 0xcd 0xcd 0xcd 0xcd 0xcd 0xcd 0xcd (gdb) p np[83] $6 = -6.2774359784998874e+66 (gdb) p gslots[1] $7 = 83 (gdb) p ngslots $8 = 2 (gdb) We write back a value that was never imported or written to. It contains the original poison value (memset 0xcd). This is not a tested tree.
Assignee | ||
Comment 19•15 years ago
|
||
This is the last entry: #0 0x00140796 in BuildNativeGlobalFrame (cx=0x30bc70, ngslots=2, gslots=0x30aea0, mp=0x30de52 "\001\001", '?' <repeats 12 times>, "\003", np=0xbfffc77c) at ../jstracer.cpp:1811 Types are int and int: (gdb) x/2bx mp 0x30de52: 0x01 0x01 Object contains 1 and 1 in the 2 slots we are looking at: 82 = 1 83 = 1 We unpack those correctly: (gdb) x/8bx &global[82] 0xbfffca0c: 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd (gdb) x/8bx &global[83] 0xbfffca14: 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd (gdb)
Assignee | ||
Comment 20•15 years ago
|
||
(gdb) x/8bx &global[82] 0xbfffca0c: 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd (gdb) x/8bx &global[83] 0xbfffca14: 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd (gdb) n 4400 u.code = f->code(); (gdb) n 4403 state->startTime = rdtsc(); (gdb) n 4406 JS_ASSERT(!tm->tracecx); (gdb) n 4407 tm->tracecx = cx; (gdb) n 4408 state->prev = cx->interpState; (gdb) n 4409 cx->interpState = state; (gdb) 4411 debug_only(fflush(NULL);) (gdb) 4416 rec = u.func(state, NULL); We enter with int 1 and 1 in the slots. We leave with the same state: (gdb) x/8bx &global[83] 0xbfffca14: 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd However, when writing back we think its a double: 4621 if (innermost->numGlobalSlots == ngslots) { (gdb) 4622 globalTypeMap = getGlobalTypeMap(innermost); (gdb) 4644 stack, NULL); (gdb) x/2bx globalTypeMap 0x2c58f5: 0x01 0x02 (gdb)
Assignee | ||
Comment 21•15 years ago
|
||
global[83] contains an integer, and its the 2nd slot, and its 0x02 which means double. So we try to box back 0x01 0x00 0x00 0x00 0xcd 0xcd 0xcd 0xcd as a double, instead of 0x01 0x00 0x00 0x00 as an int.
Updated•15 years ago
|
Attachment #381664 -
Flags: review? → review+
Assignee | ||
Comment 22•15 years ago
|
||
This is the last flush which causes he bad value to be read: Breakpoint 1, FlushNativeGlobalFrame (cx=0x30bc70, ngslots=2, gslots=0x30aea0, mp=0x2c58f5 "\001\002", np=0xbfffc77c) at ../jstracer.cpp:1838 1838 uint8* mp_base = mp; (gdb) up #1 0x001581ee in LeaveTree (state=@0xbfffc730, lr=0x2c58b4) at ../jstracer.cpp:4649 4649 FlushNativeGlobalFrame(cx, ngslots, gslots, globalTypeMap, global); (gdb) p lr->from->root $7 = (class nanojit::Fragment *) 0x30dcd0 (gdb) p *lr $8 = { <nanojit::SideExit> = { guards = 0x2c5914, from = 0x30e370, target = 0x0, switchInfo = 0x0 }, members of VMSideExit: block = 0x0, pc = 0x1d03c2 "5", imacpc = 0x30daad "#Q?", sp_adj = 40, rp_adj = 0, calldepth = 0, numGlobalSlots = 2, numStackSlots = 5, numStackSlotsBelowCurrentFrame = 0, exitType = BRANCH_EXIT, nativeCalleeWord = 0 } (gdb) p lr $9 = (VMSideExit *) 0x2c58b4 We are coming from fragment 0x30dcd0. These are all recordings: Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x0, f=0x30dcd0, ti=0x30ddf0, stackSlots=2, ngslots=0, typeMap=0x30de50 "", expectedInnerExit=0x0, outer=0x0, outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x0, f=0x30dc20, ti=0x30e110, stackSlots=2, ngslots=1, typeMap=0x30dee0 "", expectedInnerExit=0x0, outer=0x0, outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x0, f=0x30df40, ti=0x30e110, stackSlots=2, ngslots=2, typeMap=0x30ddb0 "", expectedInnerExit=0x0, outer=0x30da94 "?h", outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x0, f=0x30dc20, ti=0x30e070, stackSlots=2, ngslots=2, typeMap=0x30ddc0 "", expectedInnerExit=0x0, outer=0x0, outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x27cc04, f=0x30e1d0, ti=0x30e110, stackSlots=5, ngslots=2, typeMap=0x27cc40 "", expectedInnerExit=0x0, outer=0x30da94 "?h", outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x27a3b4, f=0x30e370, ti=0x30ddf0, stackSlots=5, ngslots=0, typeMap=0x27a3f0 "", expectedInnerExit=0x0, outer=0x0, outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x0, f=0x30dc20, ti=0x30e270, stackSlots=2, ngslots=2, typeMap=0x30e4e0 "", expectedInnerExit=0x0, outer=0x0, outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c Continuing. Breakpoint 3, 0x0015c5b7 in js_StartRecorder (cx=0x30bc70, anchor=0x2c6414, f=0x30e540, ti=0x30e110, stackSlots=5, ngslots=2, typeMap=0x2c6450 "", expectedInnerExit=0x0, outer=0x30da94 "?h", outerArgc=0) at ../jstracer.cpp:3472 3472 VMSideExit* expectedInnerExit, jsbytecode* outer, uint32 outerArgc) (gdb) c So this is the first recording.
Assignee | ||
Comment 23•15 years ago
|
||
Since its the first recording and we import lazily, there are initially no global slots. The two integer slots must have been gained later. Question: why does the side exit know about 2 slots then? And if those were added lazily, why is there no associated read which would properly initialize the slot with a double?
Assignee | ||
Comment 24•15 years ago
|
||
Ok, there is more than one trace in play here. This are unstable traces that are linked together. So we enter tree 1, and then fall out of the side exit for tree 2. tree 1 was recorded with just one global known and later probably grew a global but tree 2 always was recorded with 2 slots.
Assignee | ||
Comment 25•15 years ago
|
||
This is with the latest patch applied, minus the EmitTreeCall stuff we put in: Starting program: /Users/gal/workspace/tracemonkey-repository/js/src/Darwin_DBG.OBJ/js -j x.js Reading symbols for shared libraries . done Error in re-setting breakpoint 1: Function "../jstracer.cpp:js_AttemptToExtendTree(JSContext*, VMSideExit*, VMSideExit*, unsigned char*)" not defined. Breakpoint 2, js_AttemptToExtendTree (cx=0x30bc70, anchor=0x27cc04, exitedFrom=0x0, outer=0x30da94 "?h") at ../jstracer.cpp:4002 4002 ngslots, typeMap, exitedFrom, outer, cx->fp->argc); (gdb) p ngslots $6 = 2 (gdb) p anchor->from->root $7 = (class nanojit::Fragment *) 0x30df40 (gdb) p anchor->from->root->first $8 = (class nanojit::Fragment *) 0x30dcd0 (gdb) c Continuing. Breakpoint 2, js_AttemptToExtendTree (cx=0x30bc70, anchor=0x27a3b4, exitedFrom=0x0, outer=0x0) at ../jstracer.cpp:4002 4002 ngslots, typeMap, exitedFrom, outer, cx->fp->argc); (gdb) p ngslots $9 = 0 (gdb) p anchor->from->root $10 = (class nanojit::Fragment *) 0x30dcd0 (gdb) As you can see we have 2 peer traces here, but the 2nd one has ngslots == 0. So its out of sync with its friend, and ends up being linked together and specialized wrong.
Assignee | ||
Comment 26•15 years ago
|
||
Ok, the slot lists are actually correct. ngslots is wrong. Reading symbols for shared libraries . done Error in re-setting breakpoint 1: Function "../jstracer.cpp:js_AttemptToExtendTree(JSContext*, VMSideExit*, VMSideExit*, unsigned char*)" not defined. Breakpoint 2, js_AttemptToExtendTree (cx=0x30bc70, anchor=0x27cc04, exitedFrom=0x0, outer=0x30da94 "?h") at ../jstracer.cpp:4002 4002 ngslots, typeMap, exitedFrom, outer, cx->fp->argc); (gdb) p anchor->from->root $11 = (class nanojit::Fragment *) 0x30df40 (gdb) p *((TreeInfo*)anchor->from->root->vmprivate)->globalSlots $12 = { <avmplus::GCObject> = {<No data fields>}, members of Queue<short unsigned int>: _data = 0x30aea0, _len = 2, _max = 16 } (gdb) p *((TreeInfo*)anchor->from->root->first->vmprivate)->globalSlots $13 = { <avmplus::GCObject> = {<No data fields>}, members of Queue<short unsigned int>: _data = 0x30aea0, _len = 2, _max = 16 } (gdb) c Continuing. Breakpoint 2, js_AttemptToExtendTree (cx=0x30bc70, anchor=0x27a3b4, exitedFrom=0x0, outer=0x0) at ../jstracer.cpp:4002 4002 ngslots, typeMap, exitedFrom, outer, cx->fp->argc); (gdb) p *((TreeInfo*)anchor->from->root->vmprivate)->globalSlots $14 = { <avmplus::GCObject> = {<No data fields>}, members of Queue<short unsigned int>: _data = 0x30aea0, _len = 2, _max = 16 } (gdb)
Assignee | ||
Comment 27•15 years ago
|
||
This bug was the result of a bad patch for bug 495958, which we re-opened and fixed for real. With the new patch this bug no longer occurs. We should probably still add the test cases to trace-test though, just as future back stop against making the same mistake again.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 28•15 years ago
|
||
This bug should be opened up when it lands in branch. It was never broken in anything we shipped and is a recent regression only.
Updated•15 years ago
|
Group: core-security
Flags: wanted1.9.0.x-
Whiteboard: [sg:dupe 495958]
You need to log in
before you can comment on or make changes to this bug.
Description
•