Fix crash in d8 when external array ctor hits stack overflow

Fix crash in d8 when external array ctor hits stack overflow


BUG=100859
TEST=mjsunit/regress/regress-crbug-100859

Committed: http://code.google.com/p/v8/source/detail?r=10242

[Jakob Kummerow, 2011-12-13 10:39:46]

PTAL.

[Rico, 2011-12-13 11:28:39]

LGTM, consider making a local TryCatch

http://codereview.chromium.org/8898021/diff/1/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode299
src/d8.cc:299: if (args[0]->IsUint32()) {
Below: How about just having a local TryCatch? The functionality in isolate.cc
should throw to the TryCatch nearest to the top of the stack

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode316
src/d8.cc:316: return ThrowException(String::New("Array length must be a
number."));
we already established that length is a number, so this exception seems wrong.
Can we ever end up here?

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
File test/mjsunit/regress/regress-crbug-100859.js (right):

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:33: exception = false;
var exeception

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:41: assertTrue(exception);
we don't really need this: exception=false => we should have hit
assertUnreachable

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:48: var exception = false;
not var

[Jakob Kummerow, 2011-12-13 13:04:36]

Thanks for the comments; I ended up simplifying this quite a bit. Please take
another quick look.

http://codereview.chromium.org/8898021/diff/1/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode299
src/d8.cc:299: if (args[0]->IsUint32()) {
On 2011/12/13 11:28:39, Rico wrote:
> Below: How about just having a local TryCatch? The functionality in isolate.cc
> should throw to the TryCatch nearest to the top of the stack

Done.

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode316
src/d8.cc:316: return ThrowException(String::New("Array length must be a
number."));
On 2011/12/13 11:28:39, Rico wrote:
> we already established that length is a number, so this exception seems wrong.
> Can we ever end up here?

On second thought, no, we can't. Done.

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
File test/mjsunit/regress/regress-crbug-100859.js (right):

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:33: exception = false;
On 2011/12/13 11:28:39, Rico wrote:
> var exeception

Done. Copy-paste-error.

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:41: assertTrue(exception);
On 2011/12/13 11:28:39, Rico wrote:
> we don't really need this: exception=false => we should have hit
> assertUnreachable

Right. Done.

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-cr...
test/mjsunit/regress/regress-crbug-100859.js:48: var exception = false;
On 2011/12/13 11:28:39, Rico wrote:
> not var

Done.