27902 - eval('var whatever =...'); crashes browser

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Neil Marshall]

It seems that if you place a var Form in an eval statement, the browser will 
crash.  Expected result: it shouldn't crash :)  It doesn't matter what does 
after the variable... eval( 'var Form = "a";' ); also crashes the browser.

// Crashes browser (Works in IE 5)
eval( 'var Form = document.getElementById("' + object + 'display");' );

// Works 
var Form = eval( 'document.getElementById("' + object + 'display");' );

// Works
var Form = document.getElementById("image1display");

Comment 1

[Neil Marshall]

Attachment: This is a minimal test case which will crash the browser

Comment 2

[Robert Ginda]

Ouch.
The stack looks something like this:
#0  0x40098815 in js_Interpret (cx=0x84f1e08, result=0xbfffd080) at
jsinterp.c:2628
#1  0x4008cfe5 in js_Execute (cx=0x84f1e08, chain=0x84ffcb8, script=0x87da330, 
    fun=0x85a39a8, down=0xbfffda80, special=0, result=0xbfffd080) at
jsinterp.c:836
#2  0x400a21d1 in obj_eval (cx=0x84f1e08, obj=0x84ffa40, argc=1, argv=0x87f6f54, 
    rval=0xbfffd080) at jsobj.c:814
#3  0x4008ca4d in js_Invoke (cx=0x84f1e08, argc=1, flags=0) at jsinterp.c:665
#4  0x40097257 in js_Interpret (cx=0x84f1e08, result=0xbfffda70) at
jsinterp.c:2292
#5  0x4008caae in js_Invoke (cx=0x84f1e08, argc=1, flags=0) at jsinterp.c:681
#6  0x40097257 in js_Interpret (cx=0x84f1e08, result=0xbfffe490) at
jsinterp.c:2292
#7  0x4008caae in js_Invoke (cx=0x84f1e08, argc=1, flags=2) at jsinterp.c:681
#8  0x4008cdbf in js_InternalInvoke (cx=0x84f1e08, obj=0x85f64b8,
fval=140469448, 
    flags=0, argc=1, argv=0xbfffe75c, rval=0xbfffe634) at jsinterp.c:754
#9  0x40067330 in JS_CallFunctionValue (cx=0x84f1e08, obj=0x85f64b8, 
    fval=140469448, argc=1, argv=0xbfffe75c, rval=0xbfffe634) at jsapi.c:2787
#10 0x403226ba in nsJSContext::CallEventHandler (this=0x84f1dd8, 
    aTarget=0x85f64b8, aHandler=0x85f64c8, argc=1, argv=0xbfffe75c, 
    aBoolResult=0xbfffe6ac) at nsJSEnvironment.cpp:562


FWIW, Form is a constructor for objects created by <FORM> tags, and you really
shouldn't assign to it.  But, we really shouldn't crash if you do.

Roger: think this should go into js1.5

Comment 3

[rogerl (gone)]

Happens because the 'eval' is not setting the heavyweight flag because the code 
that does that is covered by a test of the context version. Here's a proposed 
patch:
Index: jsemit.c
===================================================================
RCS file: /m/pub/mozilla/js/src/jsemit.c,v
retrieving revision 3.30
diff -u -r3.30 jsemit.c
--- jsemit.c    2000/02/07 07:28:25     3.30
+++ jsemit.c    2000/02/23 23:39:51
@@ -2239,10 +2239,10 @@
         op = JSOP_CALL;
       emit_call:
         pn2 = pn->pn_head;
-        if (JSVERSION_IS_ECMA(cx->version) &&
-            pn2->pn_op == JSOP_NAME &&
+        if (pn2->pn_op == JSOP_NAME &&
             pn2->pn_atom == cx->runtime->atomState.evalAtom) {
-            op = JSOP_EVAL;
+            if (JSVERSION_IS_ECMA(cx->version))
+                op = JSOP_EVAL;
             cg->treeContext.flags |= TCF_FUN_HEAVYWEIGHT;
         }

Comment 4

[rogerl (gone)]

checked in.

Comment 5

[Robert Ginda]

Marking Verified.