Don't crash on stack overflow entering the debugger.

Don't crash on stack overflow entering the debugger.

R=ager@chromium.org, sgjesse@chromium.org
BUG=chromium:119429
TEST= test/mjsunit/regress/regress-119429.js

Committed: https://code.google.com/p/v8/source/detail?r=11219

[danno, 2012-04-03 13:00:52]

PTAL

[danno, 2012-04-03 13:04:50]

adding mstarzinger

[Michael Starzinger, 2012-04-03 13:31:32]

LGTM (with comments addressed).

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc
File src/debug.cc (right):

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc#newcode782
src/debug.cc:782: // If a stack overflow happened white creating the message
object, silently
s/white/while/

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc#newcode784
src/debug.cc:784: if (isolate->has_pending_exception()) {
Can this actually happen? If not, I would prefer an assertion here instead.

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc
File src/execution.cc (right):

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc#newcode829
src/execution.cc:829: StackLimitCheck check(isolate);
As discussed offline, those are just "better-safe-than-sorry" checks. But I am
also in favor of keeping them in.

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc#newcode870
src/execution.cc:870: StackLimitCheck check(isolate);
Likewise.

http://codereview.chromium.org/9965101/diff/7001/test/mjsunit/regress/regress...
File test/mjsunit/regress/regress-119429.js (right):

http://codereview.chromium.org/9965101/diff/7001/test/mjsunit/regress/regress...
test/mjsunit/regress/regress-119429.js:38: recurse();
Can we use assertThrows("recurse();") instead?

[danno, 2012-04-03 13:41:27]

Feedback addressed.

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc
File src/debug.cc (right):

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc#newcode782
src/debug.cc:782: // If a stack overflow happened white creating the message
object, silently
On 2012/04/03 13:31:32, Michael Starzinger wrote:
> s/white/while/

Done.

http://codereview.chromium.org/9965101/diff/5001/src/debug.cc#newcode784
src/debug.cc:784: if (isolate->has_pending_exception()) {
Done.
On 2012/04/03 13:31:32, Michael Starzinger wrote:
> Can this actually happen? If not, I would prefer an assertion here instead.

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc
File src/execution.cc (right):

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc#newcode829
src/execution.cc:829: StackLimitCheck check(isolate);
On 2012/04/03 13:31:32, Michael Starzinger wrote:
> As discussed offline, those are just "better-safe-than-sorry" checks. But I am
> also in favor of keeping them in.

Done.

http://codereview.chromium.org/9965101/diff/5001/src/execution.cc#newcode870
src/execution.cc:870: StackLimitCheck check(isolate);
On 2012/04/03 13:31:32, Michael Starzinger wrote:
> Likewise.

Done.

http://codereview.chromium.org/9965101/diff/7001/test/mjsunit/regress/regress...
File test/mjsunit/regress/regress-119429.js (right):

http://codereview.chromium.org/9965101/diff/7001/test/mjsunit/regress/regress...
test/mjsunit/regress/regress-119429.js:38: recurse();
On 2012/04/03 13:31:32, Michael Starzinger wrote:
> Can we use assertThrows("recurse();") instead?

Done.

[Erik Corry, 2012-04-03 13:41:35]

LGTM but outside my area of expertise.

[Søren Thygesen Gjesse, 2012-04-03 16:19:57]

LGTM

I once had the idea that we should raise the stack limit when entering the
debugger. That should make it possible to debug when close to the stack limit
and also make it possible to enter the debugger when a stack overflow exception
happens.