528507 - TM: Crash [@ JS_CallTracer]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(new Function("for (e in ((\
  function f(m, n) { \
    ((function g(aaaaaa, p) {\
      if (aaaaaa.length == p) {\
        return p\
      }\
    var x = aaaaaa[p];\
    a = g(aaaaaa, p + 1);\
    gczeal(2)\
  })([0, 0, Infinity, 0, Infinity, Infinity, 0, Infinity], 0));\
  s = m[n] + f(m, n)\
})([0], 0))()) \
for each(a in [0]) {}\
"))()


crashes js debug shell with -j at JS_CallTracer at 0xdadadfec, setting security-sensitive due to scary address.

Stack:

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000dadadfec
Crashed Thread:  0

Thread 0 Crashed:
0   js-dbg-tm-darwin              	0x0006c3e2 JS_CallTracer + 384
1   js-dbg-tm-darwin              	0x0006cadf js_TraceStackFrame + 691
2   js-dbg-tm-darwin              	0x0006d482 js_TraceContext + 596
3   js-dbg-tm-darwin              	0x0006daae js_TraceRuntime + 172
4   js-dbg-tm-darwin              	0x0006ec6d js_GC + 1013
5   js-dbg-tm-darwin              	0x0006f906 __ZL20RefillDoubleFreeListP9JSContext + 206
6   js-dbg-tm-darwin              	0x0006faef js_NewDoubleInRootedValue + 221
7   js-dbg-tm-darwin              	0x00143d05 js_NativeToValue(JSContext*, long&, JSTraceType_, double*) + 555
8   js-dbg-tm-darwin              	0x001710ad FlushNativeStackFrameVisitor::visitStackSlots(long*, unsigned long, JSStackFrame*) + 267
9   js-dbg-tm-darwin              	0x00162896 bool VisitFrameSlots<FlushNativeStackFrameVisitor>(FlushNativeStackFrameVisitor&, unsigned int, JSStackFrame*, JSStackFrame*) + 344
10  js-dbg-tm-darwin              	0x00162776 bool VisitFrameSlots<FlushNativeStackFrameVisitor>(FlushNativeStackFrameVisitor&, unsigned int, JSStackFrame*, JSStackFrame*) + 56
11  js-dbg-tm-darwin              	0x001629fc bool VisitStackSlots<FlushNativeStackFrameVisitor>(FlushNativeStackFrameVisitor&, JSContext*, unsigned int) + 54
12  js-dbg-tm-darwin              	0x00162a6c __ZL21FlushNativeStackFrameP9JSContextjPK12JSTraceType_PdP12JSStackFramej + 106
13  js-dbg-tm-darwin              	0x001640c7 __ZL9LeaveTreeR11InterpStateP10VMSideExit + 2253
14  js-dbg-tm-darwin              	0x00165803 __ZL11ExecuteTreeP9JSContextP12TreeFragmentRjPP10VMSideExit + 1769
15  js-dbg-tm-darwin              	0x00166a59 js_MonitorLoopEdge(JSContext*, unsigned int&, RecordReason) + 955
16  js-dbg-tm-darwin              	0x000896ab js_Interpret + 93369
17  js-dbg-tm-darwin              	0x0009bfed js_Execute + 1169
18  js-dbg-tm-darwin              	0x000117f2 JS_ExecuteScript + 54
19  js-dbg-tm-darwin              	0x0000a827 __ZL7ProcessP9JSContextP8JSObjectPci + 1347
20  js-dbg-tm-darwin              	0x0000b1f0 __ZL11ProcessArgsP9JSContextP8JSObjectPPci + 2272
21  js-dbg-tm-darwin              	0x0000b5bd main + 953 (js.cpp:4841)
22  js-dbg-tm-darwin              	0x000023bb _start + 209
23  js-dbg-tm-darwin              	0x000022e9 start + 41

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(Not exactly the smallest) regression window:

http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=970cd5f87f42&tochange=c159b69e34a1

Comment 2

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

Sorry, I should have noticed this earlier. I think this is your patch, Andreas.

js_NewDoubleInRootedValue() can now re-enter the GC even if we're on trace.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Confirming to be bug 508140, tm changeset 34680:c4e383beacfb did not crash, but 34681:c159b69e34a1 did.

Comment 4

[Andreas Gal :gal]

Yeah, my patch. Taking.

Comment 5

[Andreas Gal :gal]

Attachment: patch

The Refill*FreeList code is enormously ugly and convoluted. The loop is really hard to understand and the possible states involved are difficult to grasp. This patch makes it quite a bit uglier. We should rewrite the code for clarity and less re-use (unroll the loop) once igor has merged the two versions of it (I assume that will happen soon with the bitmap for everything change).

Comment 6

[Andreas Gal :gal]

This is only on TM so I think we can open this up.

Comment 7

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

I would re-run the old trace-tests or cases from bug 523947 to make sure we haven't reopened old wounds.

Comment 8

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/fbd375f19454

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/fbd375f19454

Comment 10

[Christian Holler (:decoder)]

Tracer has been long gone on trunk, marking verified.