Correctly format builtin constructors in stack traces

Correctly format builtin constructors in stack traces

CallSite::IsConstructor() was unable to recognize builtin construct stubs
(NumberConstructor_ConstructStub and StringConstructor_ConstructStub) as
constructors, and thus these frames were not formatted correctly in stack
traces.

Fix this by explicitly marking their Code objects as construct stubs and
passing along a special receiver value when we encounter such cases in
CaptureSimpleStackTrace.

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

Committed: https://crrev.com/be5808bff899b503f1ff026cec9e5c28d13fed7d
Cr-Commit-Position: refs/heads/master@{#37631}

[jgruber, 2016-07-07 15:09:08]

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

[commit-bot: I haz the power, 2016-07-07 15:09:18]

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-07 15:35:18]

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

[commit-bot: I haz the power, 2016-07-07 15:35:19]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-07-08 07:08:04]

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1378: string_fun->shared()->set_construct_stub(*code);
Would it make sense to introduce a SharedFunctionInfo::SetConstructStub that
wraps both?

https://codereview.chromium.org/2125163004/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2125163004/diff/1/src/objects.h#newcode5418
src/objects.h:5418: static const int kIsConstructStub = kCanHaveWeakObjects + 1;
We could be smarter here. Aside from IsTurbofannedBit, none of the existing
fields is used for BUILTIN. And the IsTurbofannedBit is actually not relevant
for BUILTIN either, so we could just only set it if the code kind is
OPTIMIZED_FUNCTION.

So we could actually choose 0 for kIsConstructStub to reuse the bit.

That being said, I would also be fine with leaving this as is and simply add a
comment for the future, since we still have a bunch of bits left at this point.

[Yang, 2016-07-08 07:42:06]

On 2016/07/08 07:08:04, Yang wrote:
> https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc
> File src/bootstrapper.cc (right):
> 
>
https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc#newcode...
> src/bootstrapper.cc:1378: string_fun->shared()->set_construct_stub(*code);
> Would it make sense to introduce a SharedFunctionInfo::SetConstructStub that
> wraps both?
> 
> https://codereview.chromium.org/2125163004/diff/1/src/objects.h
> File src/objects.h (right):
> 
> https://codereview.chromium.org/2125163004/diff/1/src/objects.h#newcode5418
> src/objects.h:5418: static const int kIsConstructStub = kCanHaveWeakObjects +
1;
> We could be smarter here. Aside from IsTurbofannedBit, none of the existing
> fields is used for BUILTIN. And the IsTurbofannedBit is actually not relevant
> for BUILTIN either, so we could just only set it if the code kind is
> OPTIMIZED_FUNCTION.
> 
> So we could actually choose 0 for kIsConstructStub to reuse the bit.
> 
> That being said, I would also be fine with leaving this as is and simply add a
> comment for the future, since we still have a bunch of bits left at this
point.

lgtm otherwise.

[Michael Starzinger, 2016-07-08 08:28:27]

LGTM modulo Yang's comments.

[jgruber, 2016-07-08 09:10:46]

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

[commit-bot: I haz the power, 2016-07-08 09:10:55]

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-08 09:29:34]

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

[commit-bot: I haz the power, 2016-07-08 09:29:35]

Dry run: Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[jgruber, 2016-07-08 10:02:41]

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1378: string_fun->shared()->set_construct_stub(*code);
On 2016/07/08 07:08:04, Yang wrote:
> Would it make sense to introduce a SharedFunctionInfo::SetConstructStub that
> wraps both?

set_construct_stub is also called on non-builtin shared function infos, so
probably not. We only need to set this bit for hand-written assembly construct
stubs.

https://codereview.chromium.org/2125163004/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2125163004/diff/1/src/objects.h#newcode5418
src/objects.h:5418: static const int kIsConstructStub = kCanHaveWeakObjects + 1;
On 2016/07/08 07:08:04, Yang wrote:
> We could be smarter here. Aside from IsTurbofannedBit, none of the existing
> fields is used for BUILTIN. And the IsTurbofannedBit is actually not relevant
> for BUILTIN either, so we could just only set it if the code kind is
> OPTIMIZED_FUNCTION.
> 
> So we could actually choose 0 for kIsConstructStub to reuse the bit.
> 
> That being said, I would also be fine with leaving this as is and simply add a
> comment for the future, since we still have a bunch of bits left at this
point.

Done.

[jgruber, 2016-07-08 10:11:00]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-07-08 10:11:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2125163004/#ps20001
(title: "Address comments & rebase on master")

[commit-bot: I haz the power, 2016-07-08 10:11:13]

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

[jgruber, 2016-07-08 10:11:59]

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2125163004/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1378: string_fun->shared()->set_construct_stub(*code);
On 2016/07/08 10:02:41, jgruber wrote:
> On 2016/07/08 07:08:04, Yang wrote:
> > Would it make sense to introduce a SharedFunctionInfo::SetConstructStub that
> > wraps both?
> 
> set_construct_stub is also called on non-builtin shared function infos, so
> probably not. We only need to set this bit for hand-written assembly construct
> stubs.

Disregard my previous comment, I did add SetConstructStub as suggested after
all.

[commit-bot: I haz the power, 2016-07-08 10:16:45]

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

[commit-bot: I haz the power, 2016-07-08 10:16:46]

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

[jgruber, 2016-07-11 07:52:09]

The CQ bit was checked by jgruber@chromium.org

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

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-11 07:53:25]

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

[commit-bot: I haz the power, 2016-07-11 07:53:26]

Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  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_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[jgruber, 2016-07-11 09:11:57]

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

[commit-bot: I haz the power, 2016-07-11 09:12:01]

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-11 09:43:13]

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

[commit-bot: I haz the power, 2016-07-11 09:43:14]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-07-11 10:00:52]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-07-11 10:00:53]

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

[commit-bot: I haz the power, 2016-07-11 10:01:03]

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-11 10:03:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-11 10:03:13]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-11 10:05:13]

Description was changed from

==========
Correctly format builtin constructors in stack traces

CallSite::IsConstructor() was unable to recognize builtin construct stubs
(NumberConstructor_ConstructStub and StringConstructor_ConstructStub) as
constructors, and thus these frames were not formatted correctly in stack
traces.

Fix this by explicitly marking their Code objects as construct stubs and
passing along a special receiver value when we encounter such cases in
CaptureSimpleStackTrace.

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

to

==========
Correctly format builtin constructors in stack traces

CallSite::IsConstructor() was unable to recognize builtin construct stubs
(NumberConstructor_ConstructStub and StringConstructor_ConstructStub) as
constructors, and thus these frames were not formatted correctly in stack
traces.

Fix this by explicitly marking their Code objects as construct stubs and
passing along a special receiver value when we encounter such cases in
CaptureSimpleStackTrace.

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

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

[commit-bot: I haz the power, 2016-07-11 10:05:14]

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