[debug] Untangle DebugInfo from break point support

[debug] Untangle DebugInfo from break point support

DebugInfo was very closely tied to break point support:
* It contained only information relevant to break points.
* It was created and freed by break point implementation.
* Existence of a DebugInfo on the shared function info implied existence of
  break points.

This CL is a step towards making DebugInfo usable by other debugging
functionality such as block coverage by decoupling it from break point support,
which is now only one kind of information stored on the DebugInfo object.

BUG=v8:6000
Review-Url: https://codereview.chromium.org/2909893002
Cr-Commit-Position: refs/heads/master@{#45640}
Committed: https://chromium.googlesource.com/v8/v8/+/d3371c23cb4b89da4f7b4ff09c11d594af213bc0

[jgruber, 2017-05-29 11:06:37]

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

[commit-bot: I haz the power, 2017-05-29 11:06:45]

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-05-29 11:06:46]

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

[commit-bot: I haz the power, 2017-05-29 11:06:47]

Dry run: CLs for remote refs other than refs/heads/master must contain
NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

[jgruber, 2017-05-29 11:11:13]

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

[commit-bot: I haz the power, 2017-05-29 11:11:16]

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-05-29 11:12:25]

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

[commit-bot: I haz the power, 2017-05-29 11:12:26]

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

[jgruber, 2017-05-29 11:28:36]

jgruber@chromium.org changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org, yangguo@chromium.org

[jgruber, 2017-05-29 11:28:37]

+rmcilroy for InterpreterEntryTrampoline in src/builtins
+bmeurer for compiler/, crankshaft/, deoptimizer/ and ic/
+yangguo, franzih FYI

PTAL, this is one more prerequisite step for block code coverage.

[jgruber, 2017-05-29 11:41:35]

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

[commit-bot: I haz the power, 2017-05-29 11:41:39]

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-05-29 11:46:29]

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

[commit-bot: I haz the power, 2017-05-29 11:46:30]

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

[jgruber, 2017-05-29 16:40:09]

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

[commit-bot: I haz the power, 2017-05-29 16:40: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, 2017-05-29 16:41:47]

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

[commit-bot: I haz the power, 2017-05-29 16:41:48]

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/...)
  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_gcc_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gcc_compile_dbg/...)
  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...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/26650)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/26648)
  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_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/26554)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/41994)

[Benedikt Meurer, 2017-05-30 04:09:48]

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

[Benedikt Meurer, 2017-05-30 04:09:49]

LGTM on src/compiler and src/crankshaft.

[jgruber, 2017-05-30 09:33:50]

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

[commit-bot: I haz the power, 2017-05-30 09:33: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, 2017-05-30 10:19:32]

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

[commit-bot: I haz the power, 2017-05-30 10:19:33]

Dry run: This issue passed the CQ dry run.

[jgruber, 2017-05-30 10:32:39]

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

[commit-bot: I haz the power, 2017-05-30 10:32:45]

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-05-30 11:02:32]

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

[commit-bot: I haz the power, 2017-05-30 11:02:33]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-05-31 09:01:04]

On 2017/05/30 04:09:49, Benedikt Meurer wrote:
> LGTM on src/compiler and src/crankshaft.

InterpreterEntryTrampoline changes LGTM, thanks.

[jgruber, 2017-05-31 11:06:54]

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

[commit-bot: I haz the power, 2017-05-31 11:07:07]

Dry run: CQ is trying da patch.

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

[jgruber, 2017-05-31 11:10:46]

jgruber@chromium.org changed reviewers:
+ georgia.kouveli@arm.com

[jgruber, 2017-05-31 11:10:47]

+georgia fyi for src/builtins/arm & arm64

[commit-bot: I haz the power, 2017-05-31 11:42:53]

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

[commit-bot: I haz the power, 2017-05-31 11:42:54]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2017-05-31 13:14:58]

LGTM (disclaimer: not deeply familiar with the debugger code).

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc#new...
src/debug/debug.cc:823: DebugInfoListNode* curr = debug_info_list_;
nit: s/curr/current/

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc#new...
src/debug/debug.cc:828: RemoveBreakInfoAndMaybeFree(debug_info);
question: This seems like it could have quadratic complexity. Is this relevant
or negligible here?

https://codereview.chromium.org/2909893002/diff/160001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2909893002/diff/160001/src/objects.cc#newcode...
src/objects.cc:13383: bool has_debug_info = info->HasBreakInfo();
nit: s/has_debug_info/has_break_info/

[jgruber, 2017-05-31 13:44:45]

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

[commit-bot: I haz the power, 2017-05-31 13:44:52]

Dry run: CQ is trying da patch.

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

[jgruber, 2017-05-31 13:46:46]

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc#new...
src/debug/debug.cc:823: DebugInfoListNode* curr = debug_info_list_;
On 2017/05/31 13:14:58, Michael Starzinger wrote:
> nit: s/curr/current/

Done.

https://codereview.chromium.org/2909893002/diff/160001/src/debug/debug.cc#new...
src/debug/debug.cc:828: RemoveBreakInfoAndMaybeFree(debug_info);
On 2017/05/31 13:14:57, Michael Starzinger wrote:
> question: This seems like it could have quadratic complexity. Is this relevant
> or negligible here?

Changed this slightly to avoid the linked-list search.

https://codereview.chromium.org/2909893002/diff/160001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2909893002/diff/160001/src/objects.cc#newcode...
src/objects.cc:13383: bool has_debug_info = info->HasBreakInfo();
On 2017/05/31 13:14:58, Michael Starzinger wrote:
> nit: s/has_debug_info/has_break_info/

Done.

[commit-bot: I haz the power, 2017-05-31 14:18:52]

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

[commit-bot: I haz the power, 2017-05-31 14:18:53]

Dry run: This issue passed the CQ dry run.

[jgruber, 2017-05-31 14:25:23]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2017-05-31 14:25:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, rmcilroy@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2909893002/#ps180001
(title: "Address comments")

[commit-bot: I haz the power, 2017-05-31 14:25:28]

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-05-31 14:26:59]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1496240723244110,
"parent_rev": "45618a9ab5cc98a5de200b0116670e8c272f0c5f", "commit_rev":
"d3371c23cb4b89da4f7b4ff09c11d594af213bc0"}

[commit-bot: I haz the power, 2017-05-31 14:27:05]

Description was changed from

==========
[debug] Untangle DebugInfo from break point support

DebugInfo was very closely tied to break point support:
* It contained only information relevant to break points.
* It was created and freed by break point implementation.
* Existence of a DebugInfo on the shared function info implied existence of
  break points.

This CL is a step towards making DebugInfo usable by other debugging
functionality such as block coverage by decoupling it from break point support,
which is now only one kind of information stored on the DebugInfo object.

BUG=v8:6000
==========

to

==========
[debug] Untangle DebugInfo from break point support

DebugInfo was very closely tied to break point support:
* It contained only information relevant to break points.
* It was created and freed by break point implementation.
* Existence of a DebugInfo on the shared function info implied existence of
  break points.

This CL is a step towards making DebugInfo usable by other debugging
functionality such as block coverage by decoupling it from break point support,
which is now only one kind of information stored on the DebugInfo object.

BUG=v8:6000

Review-Url: https://codereview.chromium.org/2909893002
Cr-Commit-Position: refs/heads/master@{#45640}
Committed:
https://chromium.googlesource.com/v8/v8/+/d3371c23cb4b89da4f7b4ff09c11d594af2...
==========

[commit-bot: I haz the power, 2017-05-31 14:27:06]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/v8/v8/+/d3371c23cb4b89da4f7b4ff09c11d594af2...