489682 - TM: wrong number with nested type-unstable loops, >>>

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

var v = 0; 
for each (var a in [0, {}, {}, {}]) {
  v = v >>> 0;
  for each (var b in [{}, {}, new String(''), 42, new String(''), {}, 42]) { 
  }
}
print(v); 

Without JIT: 0
With JIT:    -6.277435978499887e+66

Comment 1

[Andreas Gal :gal]

Something interesting happens here when we capture the types. v is 0 all the time, but some of the captures snapshot it as double, probably due to an oracle collision. This is not the actual bug though. The bug is that we connect incompatible trees badly. The testcase just makes the collision happen just in the right way.

stack0=O/O stack1=O/O stack2=O/O stack3=O/O global0=I/I global1=O/O global2=I/I  (demotes 1)
checking nested types 0x317420: stack0=O/O stack1=O/O stack2=O/O stack3=O/O global0=I/O object != tag5
checking nested types 0x318c30: stack0=O/O stack1=O/O stack2=O/O stack3=O/I int != tag0(value=2796544) 
adjusting will fail, global2, slot 2
Abort recording of tree x.js:2@42 at x.js:4@113: No compatible inner tree.
capture stack type stack0: 0=O
capture stack type stack1: 0=O
capture stack type stack2: 0=O
capture stack type stack3: 0=O
capture global type global0: 1=I
capture global type global1: 0=O
capture global type global2: 2=D

Comment 2

[Jeff Walden [:Waldo]]

h-118:~/moz/shell-js/js/src jwalden$ echo "var v = 0; for each (var a in [0, {}, {}, {}]) { print(v); v = v>>>0; for each (var b in [{}, {}, new String(''), 42, new String(''), {}, 42]) ; }; print(v);" | dbg/js -j
0
0
0
0
-6.277435978499887e+66

A small help in breakpointing usefully...

Comment 3

[Andreas Gal :gal]

I am going to look at this a bit. Waldo, feel free to steal back if you get around to work on this.

Comment 4

[Andreas Gal :gal]

#ifdef DEBUG
    memset(stack_buffer, 0xCD, sizeof(stack_buffer));
    memset(global, 0xCD, (globalFrameSize+1)*sizeof(double));
    JS_ASSERT(globalFrameSize <= MAX_GLOBAL_SLOTS);
#endif

long long l = 0xCDCDCDCDCDCDCDCD;
*(double*)&l = -6.27744e+66

An inner tree is reading a value from the global frame that wasn't unpacked there.

Comment 5

[Andreas Gal :gal]

Could be used for malicious integer to pointer conversion.

Comment 6

[Andreas Gal :gal]

This is a total pain to diagnose but should be an easy fix without too much code motion.

Comment 7

[Andreas Gal :gal]

We seem to have the global slot already at that point, just with a different type (int).

global: object<0x2add00:Object> object<0x2ad160:Object> int<0> 
stack: stack0=object<0x2ad1e0:Iterator> stack1=object<0x2ad160:Object> stack2=object<0x2add80:Iterator> stack3=object<0x2add20:String> 
leaving trace at x.js:5@123, op=loop, lr=0x2b802c, exitType=8, sp=4, calldepth=0, cycles=34850
global0=double<42> global1=object<0x2ad160:Object> global2=double<-6.27744e+66> 
stack0=object<0x2ad1e0:Iterator> stack1=object<0x2ad160:Object> stack2=object<0x2add80:Iterator> stack3=object<0x2add40:String>

Comment 8

[Andreas Gal :gal]

This is related to bug 476653

Comment 9

[Andreas Gal :gal]

Attachment: patch by danderson

Fixes the bug. Passes trace-tests. tryserver running.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: a fix

1. A trace is compiled with global typemap "O", with an unstable exit that also has global typemap "O".
2. Two new globals appear and that entry becomes specialized as "OOI" (the exit is still "O" since exits don't change).
3. Another trace is compiled with global typemap "OOD".
4. The original trace is run. The unstable exit triggers stabilization code which accidentally links these two trees together.
5. Now we have "OOI" going through an "O" exit which jumps into an "OOD" tree - invalid.

The stabilization code is assuming that if an exit has missing globals, then it's okay to stitch it to any types for those slots. A fix is to merge the exit and anchor typemaps.

Why this code exists in the first place, I can't remember. The logic could probably be moved into js_JoinPeersIfCompatible someday.

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/9d7ca9e8c8a5

This should remained locked until rc1 is out.

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/9d7ca9e8c8a5

Comment 13

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f181184427b9

Comment 14

[Bob Clary [:bc] (inactive)]

test checked into 1.9.0, 1.9.1, 1.9.2, tracemonkey. 1.9.3 will get picked up in the next merge.

Comment 15

[Christian Holler (:decoder)]

Bug in removed tracer code, setting in-testsuite- flag.