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

src/debug/debug-evaluate.cc

Index: src/debug/debug-evaluate.cc
diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc
index e93b5952c876ad2ffc7ae80579cf7ed08520efbb..35003604bc014fa1a8beaa180104602262909176 100644
--- a/src/debug/debug-evaluate.cc
+++ b/src/debug/debug-evaluate.cc
@@ -12,6 +12,8 @@
 #include "src/debug/debug.h"
 #include "src/frames-inl.h"
 #include "src/globals.h"
+#include "src/interpreter/bytecode-array-iterator.h"
+#include "src/interpreter/bytecodes.h"
 #include "src/isolate-inl.h"
 
 namespace v8 {
@@ -92,9 +94,13 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
       Object);
 
   Handle<Object> result;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, result, Execution::Call(isolate, eval_fun, receiver, 0, NULL),
-      Object);
+  {
+    NoSideEffectScope no_side_effect(isolate->debug(),
+                                     FLAG_side_effect_free_debug_evaluate);
+    ASSIGN_RETURN_ON_EXCEPTION(
+        isolate, result, Execution::Call(isolate, eval_fun, receiver, 0, NULL),
+        Object);
+  }
 
   // Skip the global proxy as it has no properties and always delegates to the
   // real global object.
@@ -249,5 +255,65 @@ void DebugEvaluate::ContextBuilder::MaterializeReceiver(
   JSObject::SetOwnPropertyIgnoreAttributes(target, name, recv, NONE).Check();
 }
 
+bool DebugEvaluate::HasNoSideEffect(Handle<BytecodeArray> bytecode_array) {
+  if (FLAG_trace_side_effect_free_debug_evaluate) {
+    PrintF("[debug-evaluate] Checking bytecode array\n");
+    bytecode_array->Print();
+  }
+  for (interpreter::BytecodeArrayIterator it(bytecode_array); !it.done();
+       it.Advance()) {
+    typedef interpreter::Bytecode Bytecode;
+    typedef interpreter::Bytecodes Bytecodes;
+    Bytecode bytecode = it.current_bytecode();
+
+    // TODO(yangguo): whitelist more bytecodes.
+    if (Bytecodes::IsWithoutExternalSideEffects(bytecode)) continue;
+    // if (Bytecodes::IsCallRuntime(bytecode)) continue;
+    if (Bytecodes::IsCallOrNew(bytecode)) continue;
+    if (Bytecodes::IsCallRuntime(bytecode)) {
+      Runtime::FunctionId runtime_function;
+      if (bytecode == Bytecode::kInvokeIntrinsic) {
+        runtime_function = it.GetIntrinsicIdOperand(0);
+      } else {
+        runtime_function = it.GetRuntimeIdOperand(0);
+      }
+      switch (runtime_function) {
+        case Runtime::kToObject:
+        case Runtime::kInlineToObject:
+        case Runtime::kLoadLookupSlotForCall:
+        case Runtime::kThrowReferenceError:
+          continue;
+        default:
+          if (FLAG_trace_side_effect_free_debug_evaluate) {
+            PrintF("[debug-evaluate] %s is not a read-only runtime function.\n",
+                   Runtime::FunctionForId(runtime_function)->name);
+          }
+          return false;
+      }
+    }
+    switch (bytecode) {
+      case Bytecode::kStackCheck:
+      case Bytecode::kLdaLookupSlot:
+      case Bytecode::kLdaGlobal:
+      case Bytecode::kLdaNamedProperty:
+      case Bytecode::kLdaKeyedProperty:
+      case Bytecode::kAdd:

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

You probably want to add kAddSmi as since kLdaSmi + kAdd might get transformed
into kAddSmi by the peephole optimizer.

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

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.

+      case Bytecode::kReturn:
+      case Bytecode::kCreateCatchContext:
+      case Bytecode::kSetPendingMessage:
+      case Bytecode::kPushContext:
+      case Bytecode::kPopContext:

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

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? 

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

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]

Done.

+        continue;
+      default:
+        if (FLAG_trace_side_effect_free_debug_evaluate) {
+          PrintF("[debug-evaluate] %s is not a read-only bytecode.\n",
+                 interpreter::Bytecodes::ToString(bytecode));
+        }
+        return false;
+    }
+  }
+  return true;
+}
+
 }  // namespace internal
 }  // namespace v8