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

RESOLVED FIXED in mozilla1.9.1

Status

()

Core
JavaScript Engine
P2
major
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: gal)

Tracking

(Blocks: 1 bug, {fixed1.9.1, testcase})

unspecified
mozilla1.9.1
x86
Mac OS X
fixed1.9.1, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x -
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fixed] fixed-in-tracemonkey)

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
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
Assignee: general → jwalden+bmo
(Assignee)

Comment 1

8 years ago
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
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...
Flags: blocking1.9.1?

Updated

8 years ago
Flags: blocking1.9.1? → blocking1.9.1+
(Assignee)

Comment 3

8 years ago
I am going to look at this a bit. Waldo, feel free to steal back if you get around to work on this.
Assignee: jwalden+bmo → gal
(Assignee)

Comment 4

8 years ago
#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.
(Assignee)

Updated

8 years ago
Severity: normal → major
Priority: -- → P2
Target Milestone: --- → mozilla1.9.1
(Assignee)

Comment 5

8 years ago
Could be used for malicious integer to pointer conversion.
Group: core-security
(Assignee)

Comment 6

8 years ago
This is a total pain to diagnose but should be an easy fix without too much code motion.
(Assignee)

Comment 7

8 years ago
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>
(Assignee)

Comment 8

8 years ago
This is related to bug 476653
(Assignee)

Comment 9

8 years ago
Created attachment 376152 [details] [diff] [review]
patch by danderson

Fixes the bug. Passes trace-tests. tryserver running.
(Assignee)

Updated

8 years ago
Attachment #376152 - Flags: review+
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.
(Assignee)

Comment 11

8 years ago
http://hg.mozilla.org/tracemonkey/rev/9d7ca9e8c8a5

This should remained locked until rc1 is out.
Whiteboard: fixed-in-tracemonkey
Whiteboard: fixed-in-tracemonkey → [sg:fixed] fixed-in-tracemonkey

Comment 12

8 years ago
http://hg.mozilla.org/mozilla-central/rev/9d7ca9e8c8a5
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 13

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f181184427b9
Keywords: fixed1.9.1
Depends on: 495897
Group: core-security
Flags: wanted1.9.0.x-

Comment 14

7 years ago
test checked into 1.9.0, 1.9.1, 1.9.2, tracemonkey. 1.9.3 will get picked up in the next merge.
Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.