244619 - Mozilla crashes during object.eval() with the variable declaration [@ Variables ]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Met - Martin Hassman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040514
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040514

If I call eval() as a method of some object Mozilla crashes if the evaluating
code contains declaration of some variable.

See example:

function f1()
{	
	var o = new Object();
	eval.call(o, "var a = 'vodka'"); // <- CRASH !!!
}

When the code is not in the function (so run directly in window object), the
crash never happened.
The crash is dependent on the "var" declaration - evaluating of the "a =
'vodka'" is OK.

During the call eval.call(o, "some code") I can see in the JS console warning
"function eval must be called directly, and not by way of a function of another
name", so probably this is not much likable syntax, but it this working in other
cases and I am for some reason using it.

Tested with Mozilla 1.7rc2 (Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ;
rv:1.7) Gecko/20040514)

Taklback ID: TB59963Z
Source File, Line No.
d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsparse.c, line 2016

  2014 brendan  3.64      for (fp = cx->fp; (fp->flags & JSFRAME_SPECIAL) &&
fp->down; fp = fp->down)
  2015 brendan  3.53          continue;
* 2016 brendan  3.33      obj = fp->varobj;
  2017                    fun = fp->fun;


Reproducible: Always
Steps to Reproduce:

Comment 1

[Met - Martin Hassman]

Attachment: Some testcases

Comment 2

[Boris Zbarsky [:bzbarsky]]

Crashes a current Linux trunk too.  Brendan's away, as I understand, so it might
be a bit before he gets to this...

Comment 3

[Nicholas Allen]

eval aliases are evil and an extremely difficult case to handle.  In fact I
thought they were prohibited (in implementations for performance, not in the
standard).  If one got through here it would probably break the special handling
for vars in eval.

Comment 4

[Met - Martin Hassman]

(In reply to comment #3)
> If one got through here it would probably break the special handling for vars
in eval.

Strange is although eval.call(o, "var a = 'vodka'") crashes, similar code
o.call("var a = 'vodka'") does not crash.
But I thing the first syntax is better, because eval() is not method of o
object,  I only want to call it as a method, so I am using eval.call().

Comment 5

[Met - Martin Hassman]

Sorry for typo.

Although eval.call(o, "var a = 'vodka'") crashes, similar code
o.eval("var a = 'vodka'") does not crash.

Comment 6

[Brendan Eich [:brendan]]

This has probably been broken a while, possibly forever -- can someone verify?

It's fine to call o.eval(s) in SpiderMonkey, which still supports this pre-ECMA 
feature of JS.

/be

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Stacktrace - TB61929W

Since this is a Talkback stack, there are no local variables or registers
available in my FastFind view, but here is the call flow.

Comment 8

[Nicholas Allen]

cx->fp->flags are INTERNAL,SKIP_CALLER,EVAL,COMPILING

The next fp has 0 flags so the loop stops.

fp->varobj is null and fp->fun is not script.

Comment 9

[Brendan Eich [:brendan]]

Patch next.

/be

Comment 10

[Brendan Eich [:brendan]]

Attachment: proposed fix for 1.7final

I want a better patch for the 1.8a2 trunk.

/be

Comment 11

[Mike Shaver (:shaver emeritus)]

Comment on attachment 149865 [details] [diff] [review]
proposed fix for 1.7final

Agreed about a better patch for 1.8a2, but marking this as wanting 1.7
approval, and r=shaver.

Comment 12

[Brendan Eich [:brendan]]

Fixed for 1.7.  Leaving open for a better fix on the trunk.

/be

Comment 13

[Jacek Piskozub]

Shouldn't the blocking1.7+ flag be removed, then?

Comment 14

[Michael Lefevre]

No - generally the actual status of a bug relates to the trunk. Bugs that have
been fixed in the branch have the "fixed1.7" - if you want to query for unfixed
blockers, then query for bugs which have the blocking1.7+ flag but not the
fixed1.7 query.

Comment 15

[Brendan Eich [:brendan]]

Heck, this is fixed as much as I intend to fix it, ever.  I'm turning that FIXME
into an XXX.  Anyone who feels strongly can turn it back, but with a new bug #.

/be

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Crashed is Verified FIXED on Windows XP Seamonkey trunk using the testcase:
https://bugzilla.mozilla.org/attachment.cgi?id=149272 with build 2004-12-21-06.

Comment 17

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-244619.js

Martin, with your permission this will be included in the javascript test
suite.

Comment 18

[Met - Martin Hassman]

(In reply to comment #17)

> Martin, with your permission this will be included in the javascript test
> suite.

Great! No problem 8-)

Comment 19

[Bob Clary [:bc] (inactive)]

js1_5/Regress/regress-244619.js checked in.