[debugger] handle stack overflow. Fail silently.

[debugger] handle stack overflow. Fail silently.

R=jgruber@chromium.org
BUG=v8:5654
Committed: https://crrev.com/5ebde77e3b2804831acae0d897c7a303058fc3a1
Cr-Commit-Position: refs/heads/master@{#41139}

[jgruber, 2016-11-17 13:56:55]

Nice, thanks! lgtm with comments.

https://codereview.chromium.org/2511733002/diff/1/src/d8.cc
File src/d8.cc (left):

https://codereview.chromium.org/2511733002/diff/1/src/d8.cc#oldcode1809
src/d8.cc:1809: CHECK(!result.IsEmpty());  // Listeners may not throw.
If this is removed, maybe we should remove similar checks for C++ listeners to
stay consistent.

Is there any way to get the best of both worlds, i.e. not crashing on stack
overflows within the listener but still handling exceptions within listeners? We
don't necessarily need to disallow them, as long as they're not silently
swallowed.

[Yang, 2016-11-21 11:32:36]

On 2016/11/17 13:56:55, jgruber wrote:
> Nice, thanks! lgtm with comments.
> 
> https://codereview.chromium.org/2511733002/diff/1/src/d8.cc
> File src/d8.cc (left):
> 
> https://codereview.chromium.org/2511733002/diff/1/src/d8.cc#oldcode1809
> src/d8.cc:1809: CHECK(!result.IsEmpty());  // Listeners may not throw.
> If this is removed, maybe we should remove similar checks for C++ listeners to
> stay consistent.
> 
> Is there any way to get the best of both worlds, i.e. not crashing on stack
> overflows within the listener but still handling exceptions within listeners?
We
> don't necessarily need to disallow them, as long as they're not silently
> swallowed.

My low-tech solution is to match the exception we get in d8 with a stack
overflow by string. It's not sophisticated at all, but I think adding extra API
for this would be overkill.

[jgruber, 2016-11-21 11:47:43]

On 2016/11/21 11:32:36, Yang wrote:
> On 2016/11/17 13:56:55, jgruber wrote:
> > Nice, thanks! lgtm with comments.
> > 
> > https://codereview.chromium.org/2511733002/diff/1/src/d8.cc
> > File src/d8.cc (left):
> > 
> > https://codereview.chromium.org/2511733002/diff/1/src/d8.cc#oldcode1809
> > src/d8.cc:1809: CHECK(!result.IsEmpty());  // Listeners may not throw.
> > If this is removed, maybe we should remove similar checks for C++ listeners
to
> > stay consistent.
> > 
> > Is there any way to get the best of both worlds, i.e. not crashing on stack
> > overflows within the listener but still handling exceptions within
listeners?
> We
> > don't necessarily need to disallow them, as long as they're not silently
> > swallowed.
> 
> My low-tech solution is to match the exception we get in d8 with a stack
> overflow by string. It's not sophisticated at all, but I think adding extra
API
> for this would be overkill.

lgtm.

[Yang, 2016-11-21 11:51:00]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2016-11-21 11:51:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 11:59:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 11:59:22]

Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12858)

[Yang, 2016-11-21 12:14:50]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-11-21 12:14:50]

The patchset sent to the CQ was uploaded after l-g-t-m from jgruber@chromium.org
Link to the patchset: https://codereview.chromium.org/2511733002/#ps40001
(title: "fix")

[commit-bot: I haz the power, 2016-11-21 12:14:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 12:17:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 12:17:04]

Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/16523)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12861)

[Yang, 2016-11-21 12:52:41]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-11-21 12:52:42]

The patchset sent to the CQ was uploaded after l-g-t-m from jgruber@chromium.org
Link to the patchset: https://codereview.chromium.org/2511733002/#ps60001
(title: ".")

[commit-bot: I haz the power, 2016-11-21 12:52:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 12:57:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 12:57:15]

Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12871)

[Yang, 2016-11-21 13:02:15]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-11-21 13:02:15]

The patchset sent to the CQ was uploaded after l-g-t-m from jgruber@chromium.org
Link to the patchset: https://codereview.chromium.org/2511733002/#ps80001
(title: "fix")

[commit-bot: I haz the power, 2016-11-21 13:02:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 13:28:00]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479733335428480,
"parent_rev": ["ea6d3d91e782412d34ec81e7709b1704b24d4770", null], "commit_rev":
["0f766640514699c79d427cf0779a76d114a6b698", null]}

[commit-bot: I haz the power, 2016-11-21 13:28:06]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-21 13:28:49]

Description was changed from

==========
[debugger] handle stack overflow. Fail silently.

R=jgruber@chromium.org
BUG=v8:5654
==========

to

==========
[debugger] handle stack overflow. Fail silently.

R=jgruber@chromium.org
BUG=v8:5654

Committed: https://crrev.com/5ebde77e3b2804831acae0d897c7a303058fc3a1
Cr-Commit-Position: refs/heads/master@{#41139}
==========

[commit-bot: I haz the power, 2016-11-21 13:28:50]

Patchset 5 (id:??) landed as
https://crrev.com/5ebde77e3b2804831acae0d897c7a303058fc3a1
Cr-Commit-Position: refs/heads/master@{#41139}