[wasm] Implement frame inspection for interpreted frames

[wasm] Implement frame inspection for interpreted frames

Frame inspection is currently limited to locations of execution.
Further details like local variables or stack content will follow later.

The FrameInspector now stores a pointer to the interpreted wasm frame,
and redirects certain requests there, just as for deoptimized frames.
Hitting breakpoints is now also supported for wasm frames.

R=yangguo@chromium.org, titzer@chromium.org
BUG=v8:5822
Review-Url: https://codereview.chromium.org/2629823003
Cr-Commit-Position: refs/heads/master@{#42551}
Committed: https://chromium.googlesource.com/v8/v8/+/09525c8f90f2bc307e080e3a1fea0b804a77d11d

[Clemens Hammacher, 2017-01-13 12:21:38]

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

[commit-bot: I haz the power, 2017-01-13 12:21:48]

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, 2017-01-13 12:26:00]

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

[commit-bot: I haz the power, 2017-01-13 12:26:01]

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

[Clemens Hammacher, 2017-01-13 12:33:31]

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

[commit-bot: I haz the power, 2017-01-13 12:33:37]

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, 2017-01-13 12:43:06]

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

[commit-bot: I haz the power, 2017-01-13 12:43:07]

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

[Clemens Hammacher, 2017-01-13 12:49:03]

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

[commit-bot: I haz the power, 2017-01-13 12:49:06]

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

[Clemens Hammacher, 2017-01-13 12:59:32]

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

[commit-bot: I haz the power, 2017-01-13 12:59:38]

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, 2017-01-13 13:39:16]

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

[commit-bot: I haz the power, 2017-01-13 13:39:17]

Dry run: This issue passed the CQ dry run.

[titzer, 2017-01-16 10:18:25]

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
File src/wasm/wasm-interpreter.cc (right):

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
src/wasm/wasm-interpreter.cc:1049: Frame* fr = &frames_[index];
Might as well spell out {frame} or call it {f}.

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
File src/wasm/wasm-interpreter.h (right):

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
src/wasm/wasm-interpreter.h:84:
//==========================================================================
These methods are not going to be possible to implement directly on the
InterpretedFrame class, unless it has a reference back to the interpreter. This
is why these methods were directly on the WasmInterpreter, so that each frame
didn't have to close over the interpreter.

[Clemens Hammacher, 2017-01-16 11:53:26]

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
File src/wasm/wasm-interpreter.cc (right):

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
src/wasm/wasm-interpreter.cc:1049: Frame* fr = &frames_[index];
On 2017/01/16 at 10:18:25, titzer wrote:
> Might as well spell out {frame} or call it {f}.

Done. It's {frame} now.

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
File src/wasm/wasm-interpreter.h (right):

https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
src/wasm/wasm-interpreter.h:84:
//==========================================================================
On 2017/01/16 at 10:18:25, titzer wrote:
> These methods are not going to be possible to implement directly on the
InterpretedFrame class, unless it has a reference back to the interpreter. This
is why these methods were directly on the WasmInterpreter, so that each frame
didn't have to close over the interpreter.

Yes, I was aware of that. I planned to include a pointer to the WasmInterpreter
once that's needed. Otherwise, we need to store both the WasmInterpreter and the
InterpretedFrame in the FrameInspector (debug-frames.h).
Having the pointer in the InterpretedFrame also makes it impossible to pass a
frame from another interpreter, which would give odd results.

Do you want me to put those methods back to the interpreter?

[titzer, 2017-01-16 12:01:18]

On 2017/01/16 11:53:26, Clemens Hammacher wrote:
>
https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
> File src/wasm/wasm-interpreter.cc (right):
> 
>
https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
> src/wasm/wasm-interpreter.cc:1049: Frame* fr = &frames_[index];
> On 2017/01/16 at 10:18:25, titzer wrote:
> > Might as well spell out {frame} or call it {f}.
> 
> Done. It's {frame} now.
> 
>
https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
> File src/wasm/wasm-interpreter.h (right):
> 
>
https://codereview.chromium.org/2629823003/diff/60001/src/wasm/wasm-interpret...
> src/wasm/wasm-interpreter.h:84:
> //==========================================================================
> On 2017/01/16 at 10:18:25, titzer wrote:
> > These methods are not going to be possible to implement directly on the
> InterpretedFrame class, unless it has a reference back to the interpreter.
This
> is why these methods were directly on the WasmInterpreter, so that each frame
> didn't have to close over the interpreter.
> 
> Yes, I was aware of that. I planned to include a pointer to the
WasmInterpreter
> once that's needed. Otherwise, we need to store both the WasmInterpreter and
the
> InterpretedFrame in the FrameInspector (debug-frames.h).
> Having the pointer in the InterpretedFrame also makes it impossible to pass a
> frame from another interpreter, which would give odd results.
> 
> Do you want me to put those methods back to the interpreter?

I've gone back and forth on this question, since I mostly did not consider the
case of multiple interpreters at once, which is a real possibility, since each
instance needs its own interpreter.
So I'm fine with adding an interpreter field to the frames in the future.

WASM part LGTM

[Clemens Hammacher, 2017-01-18 15:12:15]

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

[commit-bot: I haz the power, 2017-01-18 15:12:19]

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, 2017-01-18 15:32:09]

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

[commit-bot: I haz the power, 2017-01-18 15:32:10]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/21057)
  v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)

[Clemens Hammacher, 2017-01-18 16:44:00]

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

[commit-bot: I haz the power, 2017-01-18 16:44:06]

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, 2017-01-18 17:10:37]

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

[commit-bot: I haz the power, 2017-01-18 17:10:38]

Dry run: This issue passed the CQ dry run.

[Clemens Hammacher, 2017-01-19 15:04:31]

> WASM part LGTM

@Yang: This is the next CL in the sequence, PTAL.

[Yang, 2017-01-20 10:51:17]

On 2017/01/19 15:04:31, Clemens Hammacher wrote:
> > WASM part LGTM
> 
> @Yang: This is the next CL in the sequence, PTAL.

nice. lgtm!

[Clemens Hammacher, 2017-01-20 10:54:17]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2017-01-20 10:54:17]

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

[commit-bot: I haz the power, 2017-01-20 10:54:24]

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, 2017-01-20 10:57:53]

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

[commit-bot: I haz the power, 2017-01-20 10:57:54]

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_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/16049)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/21295)

[Clemens Hammacher, 2017-01-20 11:09:31]

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

[commit-bot: I haz the power, 2017-01-20 11:09:38]

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

[Clemens Hammacher, 2017-01-20 11:40:42]

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

[commit-bot: I haz the power, 2017-01-20 11:40:46]

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, 2017-01-20 12:14:17]

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

[commit-bot: I haz the power, 2017-01-20 12:14:18]

Dry run: This issue passed the CQ dry run.

[Clemens Hammacher, 2017-01-20 12:18:10]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2017-01-20 12:18:10]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2629823003/#ps140001
(title: "Rebase after Yangs revert ಠ益ಠ")

[commit-bot: I haz the power, 2017-01-20 12:18:18]

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, 2017-01-20 12:58:14]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1484914690308570,
"parent_rev": "4714bc15e850eec22e86f0b73c83178b78b4b4c2", "commit_rev":
"09525c8f90f2bc307e080e3a1fea0b804a77d11d"}

[commit-bot: I haz the power, 2017-01-20 12:58:19]

Description was changed from

==========
[wasm] Implement frame inspection for interpreted frames

Frame inspection is currently limited to locations of execution.
Further details like local variables or stack content will follow later.

The FrameInspector now stores a pointer to the interpreted wasm frame,
and redirects certain requests there, just as for deoptimized frames.
Hitting breakpoints is now also supported for wasm frames.

R=yangguo@chromium.org, titzer@chromium.org
BUG=v8:5822
==========

to

==========
[wasm] Implement frame inspection for interpreted frames

Frame inspection is currently limited to locations of execution.
Further details like local variables or stack content will follow later.

The FrameInspector now stores a pointer to the interpreted wasm frame,
and redirects certain requests there, just as for deoptimized frames.
Hitting breakpoints is now also supported for wasm frames.

R=yangguo@chromium.org, titzer@chromium.org
BUG=v8:5822

Review-Url: https://codereview.chromium.org/2629823003
Cr-Commit-Position: refs/heads/master@{#42551}
Committed:
https://chromium.googlesource.com/v8/v8/+/09525c8f90f2bc307e080e3a1fea0b804a7...
==========

[commit-bot: I haz the power, 2017-01-20 12:58:20]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/09525c8f90f2bc307e080e3a1fea0b804a7...