256501 - Still unchecked recursion in parser

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Igor Bukanov]

Statement function from jsparse.c misses a case of uncontrollable recursion. It
does not protect with CHECK_RECURSION() macro the recursion while parsing
do-while loop and simply calls self after consuming the do token.

The following test case demonstrate the problem in js shell:


var N = 100*1000;
var buffer = new Array(N * 2 + 1);
for (var i = 0; i != N; ++i) {
	buffer[i] = 'do ';
	buffer[buffer.length - i - 1] = ' while(0);'
}
buffer[N] = 'print("TEST");'

// text is do do ... do print("TEST"); while(0); while(0); ... while(0);
var text = buffer.join('');

eval(text)


The test cause N recursive call to Statement during do-parsing and if N is
100*1000 and I limit the stack size explicitly with shell invocation to 10K then
I expect that it should detect to deep recursion. Instead with native stack
limited to 1MB it segfaults:

~> ulimit -s 1024
~> js -S 10000 x.js 
Segmentation fault

Comment 1

[Igor Bukanov]

Attachment: Fix: added CHECK_RECURSION

Strictly speaking it is necessary to add the check only to "case TOK_DO" since
the rest of recursive calls to Statement comes after calling Condition etc.
that in turn call Expr which contain the missed check. 

But I prefer to be sure since I managed to miss this case during initial
CHECK_RECURSION work.

Comment 2

[Igor Bukanov]

With the patch applied I got:

~> js -S 10000 x.js 
/home/igor/s/x.js:15: InternalError: too much recursion:
/home/igor/s/x.js:15: do do do do do do do do do do do do do do do do do do do
do do do do do do do do do do do do do do do do do do do do do do do do do do do
do do do do do do do do do do do do do do do do do do do do do do do do do do do
do do do do do do do do do do do do 
/home/igor/s/x.js:15:
........................................................................^

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 156729 [details] [diff] [review]
Fix: added CHECK_RECURSION

Sure, can't hurt perf much.

/be

Comment 4

[Asa Dotzler [:asa]]

Comment on attachment 156729 [details] [diff] [review]
Fix: added CHECK_RECURSION

a=asa for branch checkin.

Comment 5

[Brendan Eich [:brendan]]

Fixed everywhere.

/be

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-256501.js

Igor, with your permission this will be included in the javascript test
library.

Comment 7

[Bob Clary [:bc] (inactive)]

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

Comment 8

[Bob Clary [:bc] (inactive)]

note to self: 1.8/1.8.0.1 dbg linux assert vp + 1 < end

Comment 9

[Bob Clary [:bc] (inactive)]

I am getting abnormal exits in mac os 10.4.4 shell opt builds for 1.8.0.1, 1.8, 1.9a1, but not in dbg builds nor on windows/linux for opt/dbg.

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060824 windows/mac*/linux