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

src/debug/debug-evaluate.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/debug/debug-evaluate.h"
 
 #include "src/accessors.h"
 #include "src/compiler.h"
 #include "src/contexts.h"
 #include "src/debug/debug-frames.h"
 #include "src/debug/debug-scopes.h"
 #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 {
 namespace internal {
 
 static inline bool IsDebugContext(Isolate* isolate, Context* context) {
   return context->native_context() == *isolate->debug()->debug_context();
 }
 
 MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
@@ skipping 60 matching lines @@
     Handle<Context> context, Handle<Object> receiver, Handle<String> source) {
   Handle<JSFunction> eval_fun;
   ASSIGN_RETURN_ON_EXCEPTION(
       isolate, eval_fun,
       Compiler::GetFunctionFromEval(source, outer_info, context, SLOPPY,
                                     NO_PARSE_RESTRICTION, kNoSourcePosition,
                                     kNoSourcePosition),
       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.
   if (result->IsJSGlobalProxy()) {
     PrototypeIterator iter(isolate, Handle<JSGlobalProxy>::cast(result));
     // TODO(verwaest): This will crash when the global proxy is detached.
     result = PrototypeIterator::GetCurrent<JSObject>(iter);
   }
 
   return result;
@@ skipping 134 matching lines @@
     // 'this' is allocated in an outer context and is is already being
     // referenced by the current function, so it can be correctly resolved.
     return;
   } else if (local_function->shared()->scope_info()->HasReceiver() &&
              !frame_->receiver()->IsTheHole(isolate_)) {
     recv = handle(frame_->receiver(), isolate_);
   }
   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