488421 - Crash [@ js_ValueToString] or Crash [@ js_ValueToBoolean] or "Assertion failure: slot < fp->script->nslots, at ../jsinterp.cpp"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

function f(foo)
{
  var x;
  eval("__defineGetter__(\"y\", function ()x)");
}
f("");
((function(){ throw y })())

crashes opt js shell without -j at js_ValueToString at null and asserts dbg js shell without -j at Assertion failure: slot < fp->script->nslots, at ../jsinterp.cpp:2078

I'm on http://hg.mozilla.org/tracemonkey/rev/99143ce38e68

autoBisect shows this is probably related to bug 488034 or http://hg.mozilla.org/tracemonkey/rev/70111870bcf8 :

The first bad revision is:
changeset:   27186:70111870bcf8
user:        Brendan Eich
date:        Mon Apr 13 14:16:15 2009 -0700
summary:     Bug 488034 - Crash [@ js_GetUpvar] or "Assertion failure: (script)->upvarsOffset != 0, at ../jsinterp.cpp" (r=mrbkap).

Comment 1

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

(In reply to comment #0)
> I'm on http://hg.mozilla.org/tracemonkey/rev/99143ce38e68

This is TM tip at the time of testing and filing.

Comment 2

[Andreas Gal :gal]

This is upvar.

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0x1a7a13 "slot < fp->script->nslots", file=0x1a79e0 "../jsinterp.cpp", ln=2078) at ../jsutil.cpp:69
69	    abort();
(gdb) bt
#0  JS_Assert (s=0x1a7a13 "slot < fp->script->nslots", file=0x1a79e0 "../jsinterp.cpp", ln=2078) at ../jsutil.cpp:69
#1  0x00092e80 in js_GetUpvar (cx=0x30bc90, level=1, cookie=131073) at jsinterp.cpp:2078
#2  0x000876b4 in js_Interpret (cx=0x30bc90) at ../jsinterp.cpp:5732
#3  0x000968bd in js_Invoke (cx=0x30bc90, argc=0, vp=0x814eac, flags=0) at jsinterp.cpp:1388
#4  0x00096eed in js_InternalInvoke (cx=0x30bc90, obj=0x2a7000, fval=2781504, flags=0, argc=0, argv=0x0, rval=0xbffff288) at jsinterp.cpp:1441
#5  0x00097167 in js_InternalGetOrSet (cx=0x30bc90, obj=0x2a7000, id=2791052, fval=2781504, mode=JSACC_READ, argc=0, argv=0x0, rval=0xbffff288) at jsinterp.cpp:1504
#6  0x000a4d88 in js_GetSprop (cx=0x30bc90, sprop=0x810630, obj=0x2a7000, vp=0xbffff288) at jsscope.h:359
#7  0x000a4fd4 in js_NativeGet (cx=0x30bc90, obj=0x2a7000, pobj=0x2a7000, sprop=0x810630, vp=0xbffff288) at ../jsobj.cpp:4185
#8  0x00083feb in js_Interpret (cx=0x30bc90) at ../jsinterp.cpp:5306
#9  0x00095ace in js_Execute (cx=0x30bc90, chain=0x2a7000, script=0x30dbc0, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1614
#10 0x000105da in JS_ExecuteScript (cx=0x30bc90, obj=0x2a7000, script=0x30dbc0, rval=0x0) at ../jsapi.cpp:5076
#11 0x000094b4 in Process (cx=0x30bc90, obj=0x2a7000, filename=0xbffff98c "x.js", forceTTY=0) at ../../shell/js.cpp:412
#12 0x0000a1ca in ProcessArgs (cx=0x30bc90, obj=0x2a7000, argv=0xbffff88c, argc=2) at ../../shell/js.cpp:806
#13 0x0000a57c in main (argc=2, argv=0xbffff88c, envp=0xbffff898) at ../../shell/js.cpp:4726

Comment 3

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

function f(code)
{
  var x;
  eval("__defineGetter__(\"\\u3056\", function ()x)");
  var g = new Function("return eval(\"\\u3056\")");
  if (g()){}
}
f();

crashes opt at js_ValueToBoolean at null without -j, and asserts similarly at debug without -j.

Comment 4

[Brendan Eich [:brendan]]

Easy to fix, more in a bit (possibly tomorrow a.m.).

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: fix

JSCompiler::analyzeFunctions does the right thing, we just do not take heed of its conclusions in this late make-upvar-for-eval function.

Probably recognizing upvars reaching above eval called from a function should be handled by making real JSDefinitions in the parser, but that's for later.

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: slightly tidier fix

The funbox member is null-initialized in JSTreeContext's ctor (jsemit.h). It's set as follows:

$ egrep '(tc|cg2?)->funbox = ' *.cpp
jsemit.cpp:        cg2->funbox = pn->pn_funbox;
jsparse.cpp:    funtc->funbox = funbox;
h-199:src brendaneich$ 

So rather than test TCF_IN_FUNCTION (which vacuously could be asserted in the consequent) this patch tests the member that's about to be used by a dependent load.

/be

Comment 8

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/56d24e7d70a0

/be

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/56d24e7d70a0

Comment 10

[Robert Sayre]

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

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with testcase given in comment 0 on trunk and 1.9.1 with the
following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090422 Minefield/3.6a1pre ID:20090422224452

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre)
Gecko/20090422 Shiretoko/3.5b4pre ID:20090422122043

Comment 12

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

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