[debugger] infrastructure for side-effect-free debug-evaluate.

[debugger] infrastructure for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
Review-Url: https://codereview.chromium.org/2622863003
Cr-Commit-Position: refs/heads/master@{#42270}
Committed: https://chromium.googlesource.com/v8/v8/+/aa75904e3c2072f6b0d6ed92f75d1fb729d7cefa

[Yang, 2017-01-10 11:26:42]

Please take a look. I'll add the ports later.

[jgruber, 2017-01-10 12:46:38]

🙌

lgtm with comments

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.cc#newcode2314
src/debug/debug.cc:2314: if (debug_->readonly_check_failed_ &&
isolate->has_pending_exception() &&
Doesn't readonly_check_failed_ imply the remaining conditions?

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h
File src/debug/debug.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h#newcode678
src/debug/debug.h:678: bool hook_on_function_call_;
A brief comment about what each of these new variables is used for would be
great.

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h#newcode821
src/debug/debug.h:821: class ReadOnlyEvaluate {
Maybe ReadOnlyEvaluateScope?

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h#newcode833
src/debug/debug.h:833: bool old_state_;
Maybe old_needs_readonly_check_?

https://codereview.chromium.org/2622863003/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:699: "use read-only debug-evaluate for testing")
s/use/trace

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.cc
File src/interpreter/bytecodes.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.c...
src/interpreter/bytecodes.cc:296: if (IsCallRuntime(bytecode)) return true;
Is CallRuntime really side-effect-free? What about e.g. desugarings into runtime
calls in the parser?

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.c...
src/interpreter/bytecodes.cc:297: if (IsCallOrNew(bytecode)) return true;
This is considered side-effect-free because we re-check on calls, correct?

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:807: static bool BytecodeIsReadOnly(Bytecode
bytecode);
What does read-only mean in this context? A comment would be great. Maybe all
'IsReadOnly'-like names would be clearer as 'IsSideEffectFree'?

https://codereview.chromium.org/2622863003/diff/1/src/messages.h
File src/messages.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/messages.h#newcode657
src/messages.h:657: T(ReadOnlyDebugEvaluate, "debug-evaluate may cause
side-effects")            \
I'd rephrase this as 'Encountered possible side-effect in debug-evaluate' or
similar.

https://codereview.chromium.org/2622863003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/objects.cc#newcode13949
src/objects.cc:13949: Handle<BytecodeArray> array(bytecode_array(), isolate);
Do we need to create this handle?

And what about adding IsReadOnly to BytecodeArray instead of DebugEvaluate?
Seems more natural and we could avoid additional includes here and in
debug-evaluate.

https://codereview.chromium.org/2622863003/diff/1/src/runtime/runtime-debug.cc
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/runtime/runtime-debug.c...
src/runtime/runtime-debug.cc:1866: if (isolate->debug()->last_step_action() >=
StepIn) {
Maybe we should refactor these '>= StepIn' checks to improve readability and
robustness (e.g. what if we add a new step action). It's also not documented in
debug.h that the order of StepActions is significant.

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64...
src/x64/macro-assembler-x64.cc:4339: CallRuntime(Runtime::kDebugOnFunctionCall);
Can we use this in builtins-x64.cc (Generate_ResumeGeneratorTrampoline) as well
and remove DebugPrepareStepInIfStepping?

[Yang, 2017-01-10 14:14:07]

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.cc#newcode2314
src/debug/debug.cc:2314: if (debug_->readonly_check_failed_ &&
isolate->has_pending_exception() &&
On 2017/01/10 12:46:37, jgruber wrote:
> Doesn't readonly_check_failed_ imply the remaining conditions?

Done.

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h
File src/debug/debug.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h#newcode678
src/debug/debug.h:678: bool hook_on_function_call_;
On 2017/01/10 12:46:37, jgruber wrote:
> A brief comment about what each of these new variables is used for would be
> great.

Done.

https://codereview.chromium.org/2622863003/diff/1/src/debug/debug.h#newcode821
src/debug/debug.h:821: class ReadOnlyEvaluate {
On 2017/01/10 12:46:37, jgruber wrote:
> Maybe ReadOnlyEvaluateScope?

Done.

https://codereview.chromium.org/2622863003/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/flag-definitions.h#newc...
src/flag-definitions.h:699: "use read-only debug-evaluate for testing")
On 2017/01/10 12:46:37, jgruber wrote:
> s/use/trace

Done.

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.cc
File src/interpreter/bytecodes.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.c...
src/interpreter/bytecodes.cc:297: if (IsCallOrNew(bytecode)) return true;
On 2017/01/10 12:46:37, jgruber wrote:
> This is considered side-effect-free because we re-check on calls, correct?

Yes.

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/interpreter/bytecodes.h...
src/interpreter/bytecodes.h:807: static bool BytecodeIsReadOnly(Bytecode
bytecode);
On 2017/01/10 12:46:37, jgruber wrote:
> What does read-only mean in this context? A comment would be great. Maybe all
> 'IsReadOnly'-like names would be clearer as 'IsSideEffectFree'?

Done.

https://codereview.chromium.org/2622863003/diff/1/src/messages.h
File src/messages.h (right):

https://codereview.chromium.org/2622863003/diff/1/src/messages.h#newcode657
src/messages.h:657: T(ReadOnlyDebugEvaluate, "debug-evaluate may cause
side-effects")            \
On 2017/01/10 12:46:38, jgruber wrote:
> I'd rephrase this as 'Encountered possible side-effect in debug-evaluate' or
> similar.

Done.

https://codereview.chromium.org/2622863003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/objects.cc#newcode13949
src/objects.cc:13949: Handle<BytecodeArray> array(bytecode_array(), isolate);
On 2017/01/10 12:46:38, jgruber wrote:
> Do we need to create this handle?
> 
> And what about adding IsReadOnly to BytecodeArray instead of DebugEvaluate?
> Seems more natural and we could avoid additional includes here and in
> debug-evaluate.

We need to create the handle for the BytecodeArrayIterator.

I'd like to keep it this way. In fact, I should move the whitelist to src/debug.

https://codereview.chromium.org/2622863003/diff/1/src/runtime/runtime-debug.cc
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/runtime/runtime-debug.c...
src/runtime/runtime-debug.cc:1866: if (isolate->debug()->last_step_action() >=
StepIn) {
On 2017/01/10 12:46:38, jgruber wrote:
> Maybe we should refactor these '>= StepIn' checks to improve readability and
> robustness (e.g. what if we add a new step action). It's also not documented
in
> debug.h that the order of StepActions is significant. 

We do have static asserts in many places to guard the order. I'd like to do that
clean up in a separate CL.

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64...
src/x64/macro-assembler-x64.cc:4339: CallRuntime(Runtime::kDebugOnFunctionCall);
On 2017/01/10 12:46:38, jgruber wrote:
> Can we use this in builtins-x64.cc (Generate_ResumeGeneratorTrampoline) as
well
> and remove DebugPrepareStepInIfStepping?

We probably can. Let me think this through.

[ofrobots, 2017-01-10 17:23:12]

ofrobots@google.com changed reviewers:
+ ofrobots@google.com

[ofrobots, 2017-01-10 17:23:13]

Looking great! 😺

https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:696: DEFINE_BOOL(side_effect_free_debug_evaluate, false,
A flag is fine for the prototype, but eventually would this be something that we
could configure at either the time the Debug context is created, or when
evaluate is called?

[Yang, 2017-01-10 17:59:33]

On 2017/01/10 17:23:13, ofrobots wrote:
> Looking great! 😺
> 
> https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h
> File src/flag-definitions.h (right):
> 
>
https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h#...
> src/flag-definitions.h:696: DEFINE_BOOL(side_effect_free_debug_evaluate,
false,
> A flag is fine for the prototype, but eventually would this be something that
we
> could configure at either the time the Debug context is created, or when
> evaluate is called?

yes. that would be an additional argument to evaluate. I just haven't bothered
adding it to the inspector protocol yet.

[Yang, 2017-01-10 18:13:57]

On 2017/01/10 17:59:33, Yang wrote:
> On 2017/01/10 17:23:13, ofrobots wrote:
> > Looking great! 😺
> > 
> > https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h
> > File src/flag-definitions.h (right):
> > 
> >
>
https://codereview.chromium.org/2622863003/diff/40001/src/flag-definitions.h#...
> > src/flag-definitions.h:696: DEFINE_BOOL(side_effect_free_debug_evaluate,
> false,
> > A flag is fine for the prototype, but eventually would this be something
that
> we
> > could configure at either the time the Debug context is created, or when
> > evaluate is called?
> 
> yes. that would be an additional argument to evaluate. I just haven't bothered
> adding it to the inspector protocol yet.

Note to self: we also need a way to whitelist builtins.

[rmcilroy, 2017-01-10 18:17:37]

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

[rmcilroy, 2017-01-10 18:17:38]

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:305: case Bytecode::kPopContext:
It seems like this might bitrot as we add new bytecodes. Is there anyway you
could use Bytecodes::IsSideEffectFree (in addition to your runtime check above)
instead?

[Yang, 2017-01-10 18:20:46]

On 2017/01/10 18:17:38, rmcilroy wrote:
>
https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
> File src/debug/debug-evaluate.cc (right):
> 
>
https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
> src/debug/debug-evaluate.cc:305: case Bytecode::kPopContext:
> It seems like this might bitrot as we add new bytecodes. Is there anyway you
> could use Bytecodes::IsSideEffectFree (in addition to your runtime check
above)
> instead?

I think that's fine though. Same applies to runtime functions and builtins. The
idea is to only whitelist what we need for common use cases. Worst case this new
feature fails due to false negatives. But that is fine.

[rmcilroy, 2017-01-10 21:10:25]

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:300: case Bytecode::kAdd:
You probably want to add kAddSmi as since kLdaSmi + kAdd might get transformed
into kAddSmi by the peephole optimizer.

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:305: case Bytecode::kPopContext:
On 2017/01/10 18:17:38, rmcilroy wrote:
> It seems like this might bitrot as we add new bytecodes. Is there anyway you
> could use Bytecodes::IsSideEffectFree (in addition to your runtime check
above)
> instead? 

Ahh sorry, I missed the call to IsWithoutExternalSideEffects above (I got the
name wrong). Sounds fine in that case, although when you say "new feature will
fail" we might add a bytecode which gets triggered for existing functions even
if they worked before - as long as you are ok with this potentially breaking
then sounds fine to me.

Small nit, kPushContext and kPopContext should be dealt with in
Bytecodes::IsWithoutExternalSideEffects above.

[Yang, 2017-01-11 10:32:10]

Updated CL.

- Added checks to accessor and interceptor callbacks.
- Whitelist for builtins.
- Refactored whitelists to be close together in src/debug/debug-evaluate.cc

@Ross I think the burden on making sure existing functions work even when
changing bytecode is on the debug-evaluate implementation to have enough test
cases. False negatives are, like previously mentioned, fine, because we will run
into false negatives for other reasons anyways.

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):

https://codereview.chromium.org/2622863003/diff/1/src/x64/macro-assembler-x64...
src/x64/macro-assembler-x64.cc:4339: CallRuntime(Runtime::kDebugOnFunctionCall);
On 2017/01/10 14:14:07, Yang wrote:
> On 2017/01/10 12:46:38, jgruber wrote:
> > Can we use this in builtins-x64.cc (Generate_ResumeGeneratorTrampoline) as
> well
> > and remove DebugPrepareStepInIfStepping?
> 
> We probably can. Let me think this through.

Done.

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:300: case Bytecode::kAdd:
On 2017/01/10 21:10:25, rmcilroy wrote:
> You probably want to add kAddSmi as since kLdaSmi + kAdd might get transformed
> into kAddSmi by the peephole optimizer.

Yes. This whitelist is by far not complete. I'll populate it in a separate CL
with more tests. This CL is to set up the infrastructure.

https://codereview.chromium.org/2622863003/diff/40001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:305: case Bytecode::kPopContext:
On 2017/01/10 21:10:25, rmcilroy wrote:
> On 2017/01/10 18:17:38, rmcilroy wrote:
> > It seems like this might bitrot as we add new bytecodes. Is there anyway you
> > could use Bytecodes::IsSideEffectFree (in addition to your runtime check
> above)
> > instead? 
> 
> Ahh sorry, I missed the call to IsWithoutExternalSideEffects above (I got the
> name wrong). Sounds fine in that case, although when you say "new feature will
> fail" we might add a bytecode which gets triggered for existing functions even
> if they worked before - as long as you are ok with this potentially breaking
> then sounds fine to me.
> 
> Small nit, kPushContext and kPopContext should be dealt with in
> Bytecodes::IsWithoutExternalSideEffects above.

Done.

[Yang, 2017-01-11 10:32:18]

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

[commit-bot: I haz the power, 2017-01-11 10:32:25]

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-11 10:55:13]

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

[commit-bot: I haz the power, 2017-01-11 10:55:15]

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

[rmcilroy, 2017-01-11 14:58:29]

> @Ross I think the burden on making sure existing functions work even when
> changing bytecode is on the debug-evaluate implementation to have enough test
> cases. False negatives are, like previously mentioned, fine, because we will
run
> into false negatives for other reasons anyways.

Cool, sounds good. Thanks for the context.

[jgruber, 2017-01-11 16:12:39]

Still lgtm.

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug-evaluat...
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:258: namespace {
Nit: whitespace.

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:361: }
Maybe an explicit 'else return false' would be clearer.

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug.cc#newc...
src/debug/debug.cc:2295: bool Debug::PerformSideEffectCheck(Handle<JSFunction>
function) {
Double-checking: I suppose this can never be entered recursively?

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

https://codereview.chromium.org/2622863003/diff/60001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:1850: // Perform %DebugPrepareStepInIfStepping and
a read-only check.
This comment is out of date.

https://codereview.chromium.org/2622863003/diff/60001/src/x64/macro-assembler...
File src/x64/macro-assembler-x64.h (right):

https://codereview.chromium.org/2622863003/diff/60001/src/x64/macro-assembler...
src/x64/macro-assembler-x64.h:393: void CheckDebugHook(Register fun, Register
new_target,
A comment would be great.

[Yang, 2017-01-12 06:04:51]

Addressed comments and added platform ports.

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug-evaluat...
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug-evaluat...
src/debug/debug-evaluate.cc:361: }
On 2017/01/11 16:12:39, jgruber wrote:
> Maybe an explicit 'else return false' would be clearer.

Done.

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2622863003/diff/60001/src/debug/debug.cc#newc...
src/debug/debug.cc:2295: bool Debug::PerformSideEffectCheck(Handle<JSFunction>
function) {
On 2017/01/11 16:12:39, jgruber wrote:
> Double-checking: I suppose this can never be entered recursively?

Yup. Now guarding that too.

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

https://codereview.chromium.org/2622863003/diff/60001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:1850: // Perform %DebugPrepareStepInIfStepping and
a read-only check.
On 2017/01/11 16:12:39, jgruber wrote:
> This comment is out of date.

Done.

https://codereview.chromium.org/2622863003/diff/60001/src/x64/macro-assembler...
File src/x64/macro-assembler-x64.h (right):

https://codereview.chromium.org/2622863003/diff/60001/src/x64/macro-assembler...
src/x64/macro-assembler-x64.h:393: void CheckDebugHook(Register fun, Register
new_target,
On 2017/01/11 16:12:39, jgruber wrote:
> A comment would be great.

Done.

[Yang, 2017-01-12 06:05:17]

Description was changed from

==========
[debugger] initial prototype for read-only debug evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
==========

to

==========
[debugger] initial prototype for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
==========

[Yang, 2017-01-12 06:05:37]

Description was changed from

==========
[debugger] initial prototype for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
==========

to

==========
[debugger] infrastructure for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
==========

[Yang, 2017-01-12 06:05:49]

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

[commit-bot: I haz the power, 2017-01-12 06:05:57]

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-12 06:07:26]

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

[commit-bot: I haz the power, 2017-01-12 06:07:27]

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_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_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_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/15388)

[Yang, 2017-01-12 06:27:37]

Patchset #6 (id:100001) has been deleted

[Yang, 2017-01-12 06:44:13]

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

[commit-bot: I haz the power, 2017-01-12 06:44:17]

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-12 06:50:26]

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

[commit-bot: I haz the power, 2017-01-12 06:50:27]

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

[Yang, 2017-01-12 06:57:29]

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

[commit-bot: I haz the power, 2017-01-12 06:57:36]

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-12 07:02:06]

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

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

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

[Yang, 2017-01-12 07:33:16]

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

[commit-bot: I haz the power, 2017-01-12 07:33: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-01-12 07:53:48]

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

[commit-bot: I haz the power, 2017-01-12 07:53:49]

Dry run: This issue passed the CQ dry run.

[Yang, 2017-01-12 10:55:36]

Michael or Ross, do you want to take a look at this before I commit?

[Michael Starzinger, 2017-01-12 14:14:09]

LGTM on the plumbing outside the debugger, only skimmed over the debugger
internals.

[Yang, 2017-01-12 14:16:18]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2017-01-12 14:16:19]

The patchset sent to the CQ was uploaded after l-g-t-m from jgruber@chromium.org
Link to the patchset: https://codereview.chromium.org/2622863003/#ps180001
(title: "fix mips one more time")

[commit-bot: I haz the power, 2017-01-12 14:16: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-12 14:18:46]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484230578953500,
"parent_rev": "e00eae9e89dba96c523fec1ab5a60714ecb23567", "commit_rev":
"aa75904e3c2072f6b0d6ed92f75d1fb729d7cefa"}

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

Description was changed from

==========
[debugger] infrastructure for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821
==========

to

==========
[debugger] infrastructure for side-effect-free debug-evaluate.

R=jgruber@chromium.org, mstarzinger@chromium.org
BUG=v8:5821

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

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

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