489040 - TM: Crash [@ TraceRecorder::getThis]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

for (var y = 0; y < 2; ++y) { (/x/)[this] }

Bug 483940 has morphed into a debug js shell crash with -j at TraceRecorder::getThis at 0xfffffffc. Waldo suggests to file a new bug...

I'm sure this is still a regression but not sure of what bug due to all the morphing that happened.

Comment 1

[Bob Clary [:bc] (inactive)]

regresses by bug 488816 http://hg.mozilla.org/tracemonkey/rev/aae39925c259

covered by at least js1_5/Regress/regress-452495.js debug shell tracemonkey/mozilla-central w or wo jit. mac only so far.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0xfffffffc
0x001420e9 in TraceRecorder::getThis (this=0x312ec0, this_ins=@0xbfffea2c) at ../jstracer.cpp:6326
6326	        JS_ASSERT(JSVAL_IS_OBJECT(cx->fp->argv[-1]));
(gdb) bt
#0  0x001420e9 in TraceRecorder::getThis (this=0x312ec0, this_ins=@0xbfffea2c) at ../jstracer.cpp:6326
#1  0x001423de in TraceRecorder::record_JSOP_THIS (this=0x312ec0) at ../jstracer.cpp:8544
#2  0x0015148b in TraceRecorder::monitorRecording (cx=0x30bd40, tr=0x312ec0, op=JSOP_THIS) at jsopcode.tbl:186
#3  0x0006bca6 in js_Interpret (cx=0x30bd40) at ../jsinterp.cpp:3030
#4  0x000959f2 in js_Execute (cx=0x30bd40, chain=0x2a9000, script=0x3121c0, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1614
#5  0x0001db30 in JS_ExecuteScript (cx=0x30bd40, obj=0x2a9000, script=0x3121c0, rval=0x0) at ../jsapi.cpp:5033
#6  0x00007e73 in Process (cx=0x30bd40, obj=0x2a9000, filename=0xbffff5ee "regress-452495.js", forceTTY=0) at ../../shell/js.cpp:412
#7  0x0000953a in ProcessArgs (cx=0x30bd40, obj=0x2a9000, argv=0xbffff480, argc=8) at ../../shell/js.cpp:746
#8  0x0000ab35 in main (argc=8, argv=0xbffff480, envp=0xbffff4a4) at ../../shell/js.cpp:4726

Comment 2

[Bob Clary [:bc] (inactive)]

ditto js1_8/extensions/regress-452476.js

Comment 3

[Andreas Gal :gal]

Attachment: patch

Bogus assert.

Comment 4

[Andreas Gal :gal]

Bogus assert only. Making P1 merely to make sure it catches the b4 train otherwise debug builds can get burned.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 373605 [details] [diff] [review]
patch

>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -6318,17 +6318,16 @@ TraceRecorder::getThis(LIns*& this_ins)
>     /*
>      * In global code, bake in the global object as 'this' object.
>      */
>     if (!cx->fp->callee) {
>         JS_ASSERT(callDepth == 0);
>         JSObject* thisObj = js_ComputeThisForFrame(cx, cx->fp);
>         if (!thisObj)
>             ABORT_TRACE("js_ComputeThis failed");
>-        JS_ASSERT(JSVAL_IS_OBJECT(cx->fp->argv[-1]));

Rather than remove it, you could move it above the if and use JS_ASSERT_IF(cx->fp->callee, JSVAL_IS_OBJECT(cx->fp->argv[-1])). r=me either way, IIRC we're covered by other assertions.

/be

Comment 6

[Andreas Gal :gal]

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

Comment 7

[Robert Sayre]

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

Comment 8

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/20cdb84dbf74

Comment 9

[Aakash Desai [:aakashd]]

Where's the simplified testcase? There's nothing in the bug report, so I'm removing the keyword since it's already been fixed and the in-testsuite? flag is marked.

Comment 10

[Aakash Desai [:aakashd]]

Never mind, I'm an idiot by not checking comment 0.

Comment 11

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929