121658 - too much recursion doesn't unwind

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

timeless@timeless-bsd:~/mozilla/obj-gtk-i386-unknown-freebsd4.4/dist/bin: ./run-mozilla.sh ./xpcshell
Type Manifest File: /home/timeless/mozilla/obj-gtk-i386-unknown-freebsd4.4/dist/bin/components/xpti.dat
nsNativeComponentLoader: autoregistering begins.
nsNativeComponentLoader: autoregistering succeeded
nNCL: registering deferred (0)
js> var i=0;function a(){++i;try{arguments.callee()}catch(e){}}; i=0;a(); print (i); //this doesn't work, but i'd like it to
typein:1: InternalError: too much recursion
js>

<brendan> timeless: when errors-as-exceptions went in, someone failed to look at the "too much recursion" cases and fix the control flow

For my freebsd system, i expect output of 1000 [which appears to be the value of i after the Error]

Comment 1

[Phil Schwartau]

timeless' example above is:

var i=0;function a(){++i;try{arguments.callee()}catch(e){}}; i=0; a(); print(i);

For my own understanding, note inside the function a(), arguments.callee === a. 
Therefore the example is equivalent to: 

var i=0;function a(){++i;try{a()}catch(e){}}; i=0; a(); print(i);

Comment 2

[Brendan Eich [:brendan]]

Patch coming right up.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 4

[rogerl (gone)]

Comment on attachment 66360 [details] [diff] [review]
proposed fix

r=rogerl.
There is one other place where the interpreter simply returns JS_FALSE - line
3617 after CHECK_ACCESS has failed - is that a different scenario?

Comment 5

[Brendan Eich [:brendan]]

Attachment: patch revised to fix that return that rogerl caught

Thanks, fixed that return JS_FALSE to capture false in ok and goto out.

/be

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 66366 [details] [diff] [review]
patch revised to fix that return that rogerl caught

Recording rogerl's patch.

/be

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 66366 [details] [diff] [review]
patch revised to fix that return that rogerl caught

Ja. sr=shaver.

Comment 8

[Brendan Eich [:brendan]]

Fixed, thanks all.

/be

Comment 9

[Phil Schwartau]

Testcase added to JS testsuite:

         mozilla/js/tests/js1_5/Exceptions/regress-121658.js

Comment 10

[Phil Schwartau]

Verified FIXED.

Before the patch, testcase failed with "InternalError: too much recursion".
Now, testcase passes in debug/optimized JS shells on WinNT, Linux, and Mac9.1.