[debugger,interpreter] add source position to stack checks.

[debugger,interpreter] add source position to stack checks.

DevTools uses the debug interrupt to trap on function entry. Without
source position at the stack check, we would get bogus source positions.

R=mstarzinger@chromium.org
BUG=chromium:595646
LOG=N

Committed: https://crrev.com/5ea84123040a93948ef3e1c49ff53f0c8e6dba33
Cr-Commit-Position: refs/heads/master@{#35864}

[Yang, 2016-04-28 11:44:25]

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

[commit-bot: I haz the power, 2016-04-28 11:44:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925063002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925063002/1

[commit-bot: I haz the power, 2016-04-28 12:01:03]

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

[commit-bot: I haz the power, 2016-04-28 12:01:04]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/4985)
  v8_linux_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)
  v8_win_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/6426)
  v8_win_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng_triggered/bui...)

[Michael Starzinger, 2016-04-28 12:06:52]

mstarzinger@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[Michael Starzinger, 2016-04-28 12:06:53]

LGTM from my end. +Ross

https://codereview.chromium.org/1925063002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/1925063002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:868:
source_position_table_builder_.AddExpressionPosition(bytecodes_.size(),
suggestion: Do you think it would make sense to add a separate
BytecodeArrayBuilder::SetStatementAsExpressionPosition instead of passing in the
position as a plain int? That seems to be more in sync with the rest of the
interface. WDYT?

[Yang, 2016-04-28 12:11:21]

On 2016/04/28 12:06:53, Michael Starzinger wrote:
> LGTM from my end. +Ross
> 
>
https://codereview.chromium.org/1925063002/diff/1/src/interpreter/bytecode-ar...
> File src/interpreter/bytecode-array-builder.cc (right):
> 
>
https://codereview.chromium.org/1925063002/diff/1/src/interpreter/bytecode-ar...
> src/interpreter/bytecode-array-builder.cc:868:
> source_position_table_builder_.AddExpressionPosition(bytecodes_.size(),
> suggestion: Do you think it would make sense to add a separate
> BytecodeArrayBuilder::SetStatementAsExpressionPosition instead of passing in
the
> position as a plain int? That seems to be more in sync with the rest of the
> interface. WDYT?

I've thought about that. But info->literal() is a FunctionLiteral, whereas the
loop is a Statement.

[rmcilroy, 2016-04-28 12:30:22]

LGTM once the bots are happy, but could we add a test for this?

[Yang, 2016-04-28 12:30:55]

On 2016/04/28 12:30:22, rmcilroy wrote:
> LGTM once the bots are happy, but could we add a test for this?

Ah... I have a test. I just forgot to add it :)

[Yang, 2016-04-28 12:34:03]

On 2016/04/28 12:30:55, Yang wrote:
> On 2016/04/28 12:30:22, rmcilroy wrote:
> > LGTM once the bots are happy, but could we add a test for this?
> 
> Ah... I have a test. I just forgot to add it :)

So, uhm, sometimes we have loops that don't have a statement position because
they are the result of desugarings. One option is to add a statement position to
every loop, but the problem here is that FCG would then add debug break slots
for these loops. The other option is to not annotate the stack check with a
position for these loops, and we would just find the closest position that we
have. I think the second option is fine, given that full-codegen has never (and
will not after this CL) reported the correct positions for stack checks in the
loop back edges.

[Yang, 2016-04-28 12:42:54]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-04-28 12:42:54]

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

[commit-bot: I haz the power, 2016-04-28 12:43:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1925063002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1925063002/40001

[commit-bot: I haz the power, 2016-04-28 13:16:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-01 22:02:26]

Description was changed from

==========
[debugger,interpreter] add source position to stack checks.

DevTools uses the debug interrupt to trap on function entry. Without
source position at the stack check, we would get bogus source positions.

R=mstarzinger@chromium.org
BUG=chromium:595646
LOG=N
==========

to

==========
[debugger,interpreter] add source position to stack checks.

DevTools uses the debug interrupt to trap on function entry. Without
source position at the stack check, we would get bogus source positions.

R=mstarzinger@chromium.org
BUG=chromium:595646
LOG=N

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

[commit-bot: I haz the power, 2016-05-01 22:02:27]

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