192465 - Object.toSource recursion does not check for stack overflow

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20021003
Build Identifier: js shell build from CVS, 2003-02-08  on Linux i686

Current Object.toSource implementation does not check for stack overflow during
recursion which allows to crash js shell or mozilla. 

A proper fix should take into account that during recursion a script function
implementing toSource() can be called which may call toSource on other objects
or can be deeply recursive on its own (see the following attachment).


Reproducible: Always

Steps to Reproduce:
Run the following example in js shell:

var head = {};
var cursor = head;
for (var i = 0; i != 10000; ++i) {
	cursor.next = {};
	cursor = cursor.next;
}
head.toSource();

Actual Results:  
~/x> ulimit -s
1024
~/x> ~/w/js/mozilla/js/src/Linux_All_DBG.OBJ/js rec_test.js 
Segmentation fault


Expected Results:  
A message about too deep recusrsion error should be printed

Comment 1

[Igor Bukanov]

Attachment: Test with mixture of toSource and script function recursion

Comment 2

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/js1_5/Regress/regress-192465.js


I've also added this to the rhino-n.tests skip list,
since toSource() is not implemented in Rhino:

Checking in rhino-n.tests;
/cvsroot/mozilla/js/tests/rhino-n.tests,v  <--  rhino-n.tests
new revision: 1.53; previous revision: 1.52
done

Comment 3

[Phil Schwartau]

Reassigning and cc'ing Brendan -

Comment 4

[Phil Schwartau]

I believe the patch in bug 192414 covers this bug as well, 
so I'm making this bug dependent on the other.

Please correct me if I'm wrong -

Comment 5

[Phil Schwartau]

I've had to add an early return to the testcase for this bug,
because it is badly hurting users of Linux RedHat8.

See bug 174341 comment 24 and following for a discussion.
Until we find a solution, I will run this test manually to
verify any fix for this bug, with the return removed -

Comment 6

[Phil Schwartau]

Another idea: in the test, I could just set the constant N lower.
I will experiment with that -

Comment 7

[Phil Schwartau]

I have removed the early return from the test so that it can run
again. I've adjusted the constant |N| downward from 1000 to 90.

With N=90, the test still produces the desired stack overflow error
on my WinNT and Linux RH7 boxes, and so is still valuable. And now,
the test will complete on Linux RH8 in a reasonable amount of time.

Comment 8

[Igor Bukanov]

Fixed as a part of the fix for bug 192414

Comment 9

[timeless]

Test results, smdebug

Test List: All tests
Skip List: lc2, lc3
1112 test(s) selected, 1112 test(s) completed, 6 failures reported (0.53% failed)
Engine command line: .\..\src\WINNT5.0_DBG.OBJ\js.exe
OS type: WIN
Testcase execution time: 14 minutes, 10 seconds.
Tests completed on Fri Nov 7 13:44:01 2003.


    Testcase js1_5/Regress/regress-192465.js failed
    [ Previous Failure | Next Failure | Top of Page ]
    Expected exit code 0, got 253
    Testcase terminated with signal 0
    Complete testcase output was:
    Testcase produced no output!

(Build env hasn't changed, it's still late october, which is well past a
november fix.)

Comment 10

[zack-weg]

The testcase consumes about 460 MB of memory on my Linux box. If less memory is
available, js will be killed by the kernel.