496185 - TM: wrong number with >>>, various types of loops

Status: RESOLVED DUPLICATE of bug 495958

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

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.

Comment 1

[Andreas Gal :gal]

****. Can we bisect?

Comment 2

[Andreas Gal :gal]

Confirmed with TM tip. Not a happy day.

Comment 3

[Andreas Gal :gal]

Regressed by:

changeset:   28910:daf5763d7da4
user:        Andreas Gal <[e-mail]>
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

Comment 4

[Andreas Gal :gal]

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.

Comment 5

[Andreas Gal :gal]

graydon is helping too

Comment 6

[Andreas Gal :gal]

Attachment: patch to flush out whats wrong (not a fix yet)

Comment 7

[Jesse Ruderman]

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

[Graydon Hoare :graydon]

Attachment: more wip

Comment 9

[Jesse Ruderman]

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

Comment 10

[Andreas Gal :gal]

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);

Comment 11

[Andreas Gal :gal]

Attachment: more wip, still borked

Comment 12

[Andreas Gal :gal]

Attachment: even more propagation, of course still doesn't work

Comment 13

[Andreas Gal :gal]

Attachment: this propagates in every direction, still broken

Comment 14

[Andreas Gal :gal]

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.

Comment 15

[Andreas Gal :gal]

Actually never mind #14. #14 looks like upvar on trace.

Comment 16

[Graydon Hoare :graydon]

Attachment: a slightly informative log

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.

Comment 17

[Andreas Gal :gal]

Attachment: also aggressively unspeculate on globals, not just stack slots

Comment 18

[Andreas Gal :gal]

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.

Comment 19

[Andreas Gal :gal]

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)

Comment 20

[Andreas Gal :gal]

(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)

Comment 21

[Andreas Gal :gal]

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.

Comment 22

[Andreas Gal :gal]

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.

Comment 23

[Andreas Gal :gal]

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?

Comment 24

[Andreas Gal :gal]

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.

Comment 25

[Andreas Gal :gal]

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.

Comment 26

[Andreas Gal :gal]

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)

Comment 27

[Andreas Gal :gal]

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.

Comment 28

[Andreas Gal :gal]

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.