Remove stack overflow boilerplate

Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Committed: https://crrev.com/46e896e76d69741268c0b18b4d3f0c29e96dfc65
Cr-Original-Commit-Position: refs/heads/master@{#37923}
Cr-Commit-Position: refs/heads/master@{#37949}

[jgruber, 2016-07-19 14:01:06]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 14:01:11]

Dry run: 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-07-19 14:26:51]

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

[commit-bot: I haz the power, 2016-07-19 14:26:52]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-07-19 15:06:47]

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc#newcode965
src/isolate.cc:965: exception = factory()->NewStringFromAsciiChecked(
I wonder whether we could remove this special case. What happens if we have a
stack overflow in our native JS code at bootstrapping, and go the other path? It
should still just work, right?

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc#newcode976
src/isolate.cc:976: exception = handle(ConstructError(this, fun, fun, msg),
this);
What if we have an exception in ConstructError? We would wrap the exception
sentinel into a handle.

https://codereview.chromium.org/2161953003/diff/1/src/messages.cc
File src/messages.cc (right):

https://codereview.chromium.org/2161953003/diff/1/src/messages.cc#newcode451
src/messages.cc:451: Object* ConstructError(Isolate* isolate, Handle<JSFunction>
target,
Let's return a MaybeHandle<Object> here.

https://codereview.chromium.org/2161953003/diff/1/test/mjsunit/stack-traces-o...
File test/mjsunit/stack-traces-overflow.js (right):

https://codereview.chromium.org/2161953003/diff/1/test/mjsunit/stack-traces-o...
test/mjsunit/stack-traces-overflow.js:43:
assertTrue(first_frame.indexOf("stack-traces-overflow.js:32:3") > 0);
why does this change? Shouldn't we notice the stack overflow on function entry?
Is it because we skip the first frame on the stack?

[jgruber, 2016-07-20 14:45:09]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-20 14:45:20]

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

[jgruber, 2016-07-20 14:48:39]

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc#newcode965
src/isolate.cc:965: exception = factory()->NewStringFromAsciiChecked(
On 2016/07/19 15:06:46, Yang wrote:
> I wonder whether we could remove this special case. What happens if we have a
> stack overflow in our native JS code at bootstrapping, and go the other path?
It
> should still just work, right?

Done. Will update the other sites that check for bootstrapping in an upcoming
commit.

https://codereview.chromium.org/2161953003/diff/1/src/isolate.cc#newcode976
src/isolate.cc:976: exception = handle(ConstructError(this, fun, fun, msg),
this);
On 2016/07/19 15:06:46, Yang wrote:
> What if we have an exception in ConstructError? We would wrap the exception
> sentinel into a handle.

Good catch. Done.

https://codereview.chromium.org/2161953003/diff/1/src/messages.cc
File src/messages.cc (right):

https://codereview.chromium.org/2161953003/diff/1/src/messages.cc#newcode451
src/messages.cc:451: Object* ConstructError(Isolate* isolate, Handle<JSFunction>
target,
On 2016/07/19 15:06:47, Yang wrote:
> Let's return a MaybeHandle<Object> here.

Done.

https://codereview.chromium.org/2161953003/diff/1/test/mjsunit/stack-traces-o...
File test/mjsunit/stack-traces-overflow.js (right):

https://codereview.chromium.org/2161953003/diff/1/test/mjsunit/stack-traces-o...
test/mjsunit/stack-traces-overflow.js:43:
assertTrue(first_frame.indexOf("stack-traces-overflow.js:32:3") > 0);
On 2016/07/19 15:06:47, Yang wrote:
> why does this change? Shouldn't we notice the stack overflow on function
entry?
> Is it because we skip the first frame on the stack?

Yes. I fixed this by changing the way we determine frame skipping behavior; it
is now explicitly passed into CaptureSimpleStackTrace.

[commit-bot: I haz the power, 2016-07-20 15:08:31]

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

[commit-bot: I haz the power, 2016-07-20 15:08:32]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-07-21 06:45:02]

LGTM once comments are addressed.

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode355
src/isolate.cc:355: default:
I don't think you need a default case for the compiler to accept this.

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode968
src/isolate.cc:968: capture_stack_trace_for_uncaught_exceptions_ = false;
I think it's cleaner to pass an additional (optional) boolean argument to
ConstructError for this.

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode974
src/isolate.cc:974: if (!ConstructError(this, fun, fun, msg,
SKIP_NONE).ToHandle(&exception)) {
You could use the ASSIGN_RETURN_FAILURE_ON_EXCEPTION macro here.

[jgruber, 2016-07-21 07:47:45]

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode355
src/isolate.cc:355: default:
On 2016/07/21 06:45:02, Yang wrote:
> I don't think you need a default case for the compiler to accept this.

Done.

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode968
src/isolate.cc:968: capture_stack_trace_for_uncaught_exceptions_ = false;
On 2016/07/21 06:45:02, Yang wrote:
> I think it's cleaner to pass an additional (optional) boolean argument to
> ConstructError for this.

Done.

https://codereview.chromium.org/2161953003/diff/20001/src/isolate.cc#newcode974
src/isolate.cc:974: if (!ConstructError(this, fun, fun, msg,
SKIP_NONE).ToHandle(&exception)) {
On 2016/07/21 06:45:02, Yang wrote:
> You could use the ASSIGN_RETURN_FAILURE_ON_EXCEPTION macro here.

Done.

[jgruber, 2016-07-21 07:50:43]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-07-21 07:50:43]

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

[commit-bot: I haz the power, 2016-07-21 07:50:52]

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

[Yang, 2016-07-21 07:51:13]

lgtm

[commit-bot: I haz the power, 2016-07-21 08:14:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-21 08:17:10]

Description was changed from

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=
==========

to

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

[commit-bot: I haz the power, 2016-07-21 08:17:11]

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

[jgruber, 2016-07-21 08:54:22]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2169563003/ by jgruber@chromium.org.

The reason for reverting is: Clusterfuzz failures in parent CL
https://codereview.chromium.org/2142933003/.

[jgruber, 2016-07-21 08:54:46]

Description was changed from

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

to

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

[Michael Achenbach, 2016-07-21 09:06:47]

This also breaks arm compilation:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Arm%20-%20builder/...

[jgruber, 2016-07-21 13:45:44]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 13:45:53]

Dry run: 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-07-21 14:16:40]

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

[commit-bot: I haz the power, 2016-07-21 14:16:41]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-21 14:28:08]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-07-21 14:28:08]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2161953003/#ps60001
(title: "Satisfy older mips/arm gcc versions")

[commit-bot: I haz the power, 2016-07-21 14:28:20]

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-07-21 14:29:38]

Description was changed from

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

to

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

[commit-bot: I haz the power, 2016-07-21 14:29:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-21 14:32:13]

Description was changed from

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Cr-Commit-Position: refs/heads/master@{#37923}
==========

to

==========
Remove stack overflow boilerplate

We no longer need to prepare the stack overflow error in advance now that
Errors are constructed in C++.

R=yangguo@chromium.org
BUG=

Committed: https://crrev.com/ba95d10ccbe13e2fca427228483b045576f2dc4c
Committed: https://crrev.com/46e896e76d69741268c0b18b4d3f0c29e96dfc65
Cr-Original-Commit-Position: refs/heads/master@{#37923}
Cr-Commit-Position: refs/heads/master@{#37949}
==========

[commit-bot: I haz the power, 2016-07-21 14:32:15]

Patchset 4 (id:??) landed as
https://crrev.com/46e896e76d69741268c0b18b4d3f0c29e96dfc65
Cr-Commit-Position: refs/heads/master@{#37949}