Debugger: use debug break slots to break at function exit.

Debugger: use debug break slots to break at function exit.

By not having to patch the return sequence (we patch the debug
break slot right before it), we don't overwrite it and therefore
don't have to keep the original copy of the code around.

R=ulan@chromium.org
BUG=v8:4269
LOG=N

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

[Yang, 2015-07-13 14:21:02]

Please take a look at the ia32 port. I'll continue with other platforms later.

[ulan, 2015-07-14 09:12:45]

looks good

https://codereview.chromium.org/1234833003/diff/20001/src/ia32/assembler-ia32...
File src/ia32/assembler-ia32-inl.h (right):

https://codereview.chromium.org/1234833003/diff/20001/src/ia32/assembler-ia32...
src/ia32/assembler-ia32-inl.h:78: return;
I think it is cleaner to just set flush_icache to false.

https://codereview.chromium.org/1234833003/diff/20001/src/liveedit.cc
File src/liveedit.cc (left):

https://codereview.chromium.org/1234833003/diff/20001/src/liveedit.cc#oldcode...
src/liveedit.cc:1191: if (shared_info->debug_info()->IsDebugInfo()) {
DCHECK(shared_info->debug_info()->IsDebugInfo()) ?

https://codereview.chromium.org/1234833003/diff/20001/src/runtime/runtime-deb...
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/1234833003/diff/20001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:1399: if (!ignore_nested_scopes &&
!shared_info->debug_info()->IsUndefined()) {
What happens if there is no debug info?

[Yang, 2015-07-14 12:00:57]

https://codereview.chromium.org/1234833003/diff/20001/src/ia32/assembler-ia32...
File src/ia32/assembler-ia32-inl.h (right):

https://codereview.chromium.org/1234833003/diff/20001/src/ia32/assembler-ia32...
src/ia32/assembler-ia32-inl.h:78: return;
On 2015/07/14 09:12:45, ulan wrote:
> I think it is cleaner to just set flush_icache to false.

apply is only ever called with SKIP_ICACHE_FLUSH. So I'm going to remove this
parameter and make this simpler. I added a comment to the function declaration.

Also icache_flush_mode is not even honored on ARM and ARM64.

https://codereview.chromium.org/1234833003/diff/20001/src/liveedit.cc
File src/liveedit.cc (left):

https://codereview.chromium.org/1234833003/diff/20001/src/liveedit.cc#oldcode...
src/liveedit.cc:1191: if (shared_info->debug_info()->IsDebugInfo()) {
On 2015/07/14 09:12:45, ulan wrote:
> DCHECK(shared_info->debug_info()->IsDebugInfo()) ?

Since we don't keep a copy of the original code anymore, we don't have to keep
it updated. So here we simply don't care whether we have debug info.

https://codereview.chromium.org/1234833003/diff/20001/src/runtime/runtime-deb...
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/1234833003/diff/20001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:1399: if (!ignore_nested_scopes &&
!shared_info->debug_info()->IsUndefined()) {
On 2015/07/14 09:12:45, ulan wrote:
> What happens if there is no debug info?

We can only ever end up here from a return statement if we set a break point at
the return statement, which requires a debug info to exist. I updated the
comment to better reflect this.

[Yang, 2015-07-14 12:01:19]

yangguo@chromium.org changed reviewers:
+ plind44@gmail.com

[Yang, 2015-07-14 12:01:20]

Paul, could you help me with the MIPS/MIPS64 ports?

[Yang, 2015-07-14 13:03:30]

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

[commit-bot: I haz the power, 2015-07-14 13:03:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1234833003/60001

[commit-bot: I haz the power, 2015-07-14 13:10:14]

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

[commit-bot: I haz the power, 2015-07-14 13:10:15]

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

(exceeded global retry quota)

[balazs.kilvady, 2015-07-14 19:37:37]

On 2015/07/14 12:01:20, Yang wrote:
> Paul, could you help me with the MIPS/MIPS64 ports?

We made the MIPS port of the CL: https://codereview.chromium.org/1233213002/

[Yang, 2015-07-14 19:40:45]

On 2015/07/14 19:37:37, balazs.kilvady wrote:
> On 2015/07/14 12:01:20, Yang wrote:
> > Paul, could you help me with the MIPS/MIPS64 ports?
> 
> We made the MIPS port of the CL: https://codereview.chromium.org/1233213002/

Thanks Balazs, that was quick! As MIPS is a tree closer (and commit queue
blocker), do you prefer me to manually land both changes in succession or should
I take your patch and merge it into my CL?

[paul.l..., 2015-07-14 19:46:05]

On 2015/07/14 19:40:45, Yang wrote:
> Thanks Balazs, that was quick! As MIPS is a tree closer (and commit queue
> blocker), do you prefer me to manually land both changes in succession or
should
> I take your patch and merge it into my CL?

Yang, I think it's best to merge it into your CL. Then if anything goes awry,
it's easy to revert a single CL. This is the plan we worked out with Michael H.
(answering for Balázs, since he's done for the day.)

[Yang, 2015-07-15 07:18:36]

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

[commit-bot: I haz the power, 2015-07-15 07:18:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1234833003/80001

[Yang, 2015-07-15 07:18:49]

Ulan, please take a look.

[ulan, 2015-07-15 07:53:46]

lgtm

[commit-bot: I haz the power, 2015-07-15 08:06:47]

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

[commit-bot: I haz the power, 2015-07-15 08:06:48]

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

[Yang, 2015-07-15 08:55:24]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2015-07-15 08:55:25]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/1234833003/#ps100001
(title: "fix for arm")

[commit-bot: I haz the power, 2015-07-15 08:55:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1234833003/100001

[commit-bot: I haz the power, 2015-07-15 09:22:40]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-07-15 09:22:57]

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