Last Comment Bug 489682 - TM: wrong number with nested type-unstable loops, >>>
: TM: wrong number with nested type-unstable loops, >>>
[sg:fixed] fixed-in-tracemonkey
: fixed1.9.1, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
P2 major (vote)
: mozilla1.9.1
Assigned To: Andreas Gal :gal
: Jason Orendorff [:jorendorff]
Depends on: 495897
Blocks: js-differential-test
  Show dependency treegraph
Reported: 2009-04-22 16:05 PDT by Jesse Ruderman
Modified: 2013-02-25 16:52 PST (History)
10 users (show)
sayrer: blocking1.9.1+
dveditz: wanted1.9.0.x-
choller: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch by danderson (1.94 KB, patch)
2009-05-06 18:55 PDT, Andreas Gal :gal
gal: review+
Details | Diff | Splinter Review
a fix (1.47 KB, patch)
2009-05-06 19:02 PDT, David Anderson [:dvander]
no flags Details | Diff | Splinter Review

Description User image Jesse Ruderman 2009-04-22 16:05:30 PDT
var v = 0; 
for each (var a in [0, {}, {}, {}]) {
  v = v >>> 0;
  for each (var b in [{}, {}, new String(''), 42, new String(''), {}, 42]) { 

Without JIT: 0
With JIT:    -6.277435978499887e+66
Comment 1 User image Andreas Gal :gal 2009-04-22 16:34:00 PDT
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 User image Jeff Walden [:Waldo] (remove +bmo to email) 2009-04-22 16:37:58 PDT
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

A small help in breakpointing usefully...
Comment 3 User image Andreas Gal :gal 2009-05-05 18:31:11 PDT
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 User image Andreas Gal :gal 2009-05-05 18:42:07 PDT
#ifdef DEBUG
    memset(stack_buffer, 0xCD, sizeof(stack_buffer));
    memset(global, 0xCD, (globalFrameSize+1)*sizeof(double));
    JS_ASSERT(globalFrameSize <= MAX_GLOBAL_SLOTS);

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 User image Andreas Gal :gal 2009-05-05 18:43:24 PDT
Could be used for malicious integer to pointer conversion.
Comment 6 User image Andreas Gal :gal 2009-05-05 18:44:30 PDT
This is a total pain to diagnose but should be an easy fix without too much code motion.
Comment 7 User image Andreas Gal :gal 2009-05-05 19:04:53 PDT
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 User image Andreas Gal :gal 2009-05-05 19:20:22 PDT
This is related to bug 476653
Comment 9 User image Andreas Gal :gal 2009-05-06 18:55:41 PDT
Created attachment 376152 [details] [diff] [review]
patch by danderson

Fixes the bug. Passes trace-tests. tryserver running.
Comment 10 User image David Anderson [:dvander] 2009-05-06 19:02:25 PDT
Created attachment 376154 [details] [diff] [review]
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 User image Andreas Gal :gal 2009-05-07 12:16:54 PDT

This should remained locked until rc1 is out.
Comment 14 User image Bob Clary [:bc:] 2010-02-22 11:27:22 PST
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 User image Christian Holler (:decoder) 2013-02-25 16:52:42 PST
Bug in removed tracer code, setting in-testsuite- flag.

Note You need to log in before you can comment on or make changes to this bug.