[debugger] allow some map, set, regexp and array builtins on tmp objects..

.. for side effect free debug evaluate.

R=yangguo@chromium.org

Bug: v8:7588
Change-Id: Iac4d782dbf996d9c11430fc681f38a648d89435b
Reviewed-on: https://chromium-review.googlesource.com/1000527
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52699}

src/debug/debug-evaluate.cc

@@ -349,6 +349,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(HasComplexElements)                  \
   V(NewArray)                            \
   V(NormalizeElements)                   \
+  V(RemoveArrayHoles)                    \
   V(TrySliceSimpleNonFastElements)       \
   V(TypedArrayGetBuffer)                 \
   /* Errors */                           \
@@ -413,7 +414,9 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(HasInPrototypeChain)                 \
   V(MaxSmi)                              \
   V(NewObject)                           \
+  V(SmiLexicographicCompare)             \
   V(StringMaxLength)                     \
+  V(StringToArray)                       \
   /* Test */                             \
   V(GetOptimizationStatus)               \
   V(OptimizeFunctionOnNextCall)          \
@@ -555,8 +558,9 @@ bool BytecodeHasNoSideEffect(interpreter::Bytecode bytecode) {
     case Bytecode::kTestNull:
     // Conversions.
     case Bytecode::kToObject:
-    case Bytecode::kToNumber:
     case Bytecode::kToName:
+    case Bytecode::kToNumber:
+    case Bytecode::kToNumeric:
     case Bytecode::kToString:
     // Misc.
     case Bytecode::kForInEnumerate:
@@ -853,9 +857,36 @@ SharedFunctionInfo::SideEffectState BuiltinGetSideEffectState(
     case Builtins::kMakeSyntaxError:
     case Builtins::kMakeRangeError:
     case Builtins::kMakeURIError:
+    // RegExp builtins.
+    case Builtins::kRegExpConstructor:
       return SharedFunctionInfo::kHasNoSideEffect;
+    // Set builtins.
+    case Builtins::kSetIteratorPrototypeNext:
     case Builtins::kSetPrototypeAdd:
+    case Builtins::kSetPrototypeClear:
+    case Builtins::kSetPrototypeDelete:
+    // Array builtins.
     case Builtins::kArrayIteratorPrototypeNext:
+    case Builtins::kArrayPrototypePop:
+    case Builtins::kArrayPrototypePush:
+    case Builtins::kArrayPrototypeShift:
+    case Builtins::kArrayUnshift:
+    // Map builtins.
+    case Builtins::kMapIteratorPrototypeNext:
+    case Builtins::kMapPrototypeClear:
+    case Builtins::kMapPrototypeDelete:
+    case Builtins::kMapPrototypeSet:
+    // RegExp builtins.
+    case Builtins::kRegExpPrototypeTest:
+    case Builtins::kRegExpPrototypeExec:
+    case Builtins::kRegExpPrototypeSplit:
+    case Builtins::kRegExpPrototypeFlagsGetter:
+    case Builtins::kRegExpPrototypeGlobalGetter:
+    case Builtins::kRegExpPrototypeIgnoreCaseGetter:
+    case Builtins::kRegExpPrototypeMultilineGetter:
+    case Builtins::kRegExpPrototypeDotAllGetter:
+    case Builtins::kRegExpPrototypeUnicodeGetter:
+    case Builtins::kRegExpPrototypeStickyGetter:
       return SharedFunctionInfo::kRequiresRuntimeChecks;
     default:
       if (FLAG_trace_side_effect_free_debug_evaluate) {

src/debug/debug.cc

@@ -2327,6 +2327,10 @@ void Debug::StartSideEffectCheckMode() {
   DCHECK(!temporary_objects_);
   temporary_objects_.reset(new TemporaryObjectsTracker());
   isolate_->heap()->AddHeapObjectAllocationTracker(temporary_objects_.get());
+  Handle<FixedArray> array(
+      isolate_->native_context()->regexp_last_match_info());
+  regexp_match_info_ =
+      Handle<RegExpMatchInfo>::cast(isolate_->factory()->CopyFixedArray(array));
 }
 
 void Debug::StopSideEffectCheckMode() {
@@ -2347,6 +2351,8 @@ void Debug::StopSideEffectCheckMode() {
   DCHECK(temporary_objects_);
   isolate_->heap()->RemoveHeapObjectAllocationTracker(temporary_objects_.get());
   temporary_objects_.reset();
+  isolate_->native_context()->set_regexp_last_match_info(*regexp_match_info_);
+  regexp_match_info_ = Handle<RegExpMatchInfo>::null();
 }
 
 void Debug::ApplySideEffectChecks(Handle<DebugInfo> debug_info) {

src/debug/debug.h

@@ -349,6 +349,7 @@ class Debug {
                               Handle<Object> receiver);
   bool PerformSideEffectCheckForCallback(Handle<Object> callback_info);
   bool PerformSideEffectCheckAtBytecode(InterpretedFrame* frame);
+  bool PerformSideEffectCheckForObject(Handle<Object> object);
 
   // Flags and states.
   DebugScope* debugger_entry() {
@@ -502,7 +503,6 @@ class Debug {
   void FindDebugInfo(Handle<DebugInfo> debug_info, DebugInfoListNode** prev,
                      DebugInfoListNode** curr);
   void FreeDebugInfoListNode(DebugInfoListNode* prev, DebugInfoListNode* node);
-  bool PerformSideEffectCheckForObject(Handle<Object> object);
 
   // Global handles.
   Handle<Context> debug_context_;
@@ -537,6 +537,8 @@ class Debug {
   class TemporaryObjectsTracker;
   std::unique_ptr<TemporaryObjectsTracker> temporary_objects_;
 
+  Handle<RegExpMatchInfo> regexp_match_info_;
+
   // Used to collect histogram data on debugger feature usage.
   DebugFeatureTracker feature_tracker_;
 

src/runtime/runtime-array.cc

@@ -258,6 +258,11 @@ RUNTIME_FUNCTION(Runtime_RemoveArrayHoles) {
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, object, 0);
   CONVERT_NUMBER_CHECKED(uint32_t, limit, Uint32, args[1]);
+  if (isolate->debug_execution_mode() == DebugInfo::kSideEffects) {
+    if (!isolate->debug()->PerformSideEffectCheckForObject(object)) {
+      return isolate->heap()->exception();
+    }
+  }
   if (object->IsJSProxy()) return Smi::FromInt(-1);
   return PrepareElementsForSort(Handle<JSObject>::cast(object), limit);
 }

test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins-2.js

@@ -79,7 +79,7 @@ function listener(event, exec_state, event_data, data) {
     success(undefined, `map.forEach(()=>1)`);
     success(true, `map.has("c")`);
     success(2, `map.size`);
-    fail(`new Map([[1, 2]])`);
+    success(undefined, `new Map([[1, 2]])`);
     fail(`map.delete("a")`);
     fail(`map.clear()`);
     fail(`map.set("x", "y")`);

test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins.js

@@ -72,19 +72,19 @@ function listener(event, exec_state, event_data, data) {
       "map", "findIndex"
     ];
     var fails = ["toString", "join", "toLocaleString", "pop", "push", "reverse",
-      "shift", "unshift", "splice", "sort", "copyWithin"];
+      "shift", "unshift", "splice", "sort", "copyWithin", "fill"];
     for (f of Object.getOwnPropertyNames(Array.prototype)) {
       if (typeof Array.prototype[f] === "function") {
         if (fails.includes(f)) {
           if (function_param.includes(f)) {
-            fail(`[1, 2, 3].${f}(()=>{});`);
+            fail(`array.${f}(()=>{});`);
           } else {
-            fail(`[1, 2, 3].${f}();`);
+            fail(`array.${f}();`);
           }
         } else if (function_param.includes(f)) {
-          exec_state.frame(0).evaluate(`[1, 2, 3].${f}(()=>{});`, true);
+          exec_state.frame(0).evaluate(`array.${f}(()=>{});`, true);
         } else {
-          exec_state.frame(0).evaluate(`[1, 2, 3].${f}();`, true);
+          exec_state.frame(0).evaluate(`array.${f}();`, true);
         }
       }
     }
@@ -200,12 +200,11 @@ function listener(event, exec_state, event_data, data) {
     fail("'abcd'.match(/a/)");
     fail("'abcd'.replace(/a/)");
     fail("'abcd'.search(/a/)");
-    fail("'abcd'.split(/a/)");
 
     // Test RegExp functions.
     fail(`/a/.compile()`);
-    fail(`/a/.exec('abc')`);
-    fail(`/a/.test('abc')`);
+    success('a', `/a/.exec('abc')[0]`);
+    success(true, `/a/.test('abc')`);
     fail(`/a/.toString()`);
 
     // Test JSON functions.

test/debugger/debug/side-effect/debug-evaluate-no-side-effect-regexp.js

+// Copyright 2018 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.
+
+Debug = debug.Debug;
+
+/(\w)(\w)(\w)(\w)(\w)(\w)(\w)(\w)(\w)(\w)/.exec(">>>abcdefghij<<<");
+assertRegExp();
+Debug.evaluateGlobal(`/(\\w)(\\w)(\\w)(\\w)(\\w)(\\w)(\\w)(\\w)(\\w)(\\w)/.exec(">>>hklmnoprst<<<")`, true);
+assertRegExp();
+
+function assertRegExp() {
+  assertEquals("a", RegExp.$1);
+  assertEquals("b", RegExp.$2);
+  assertEquals("c", RegExp.$3);
+  assertEquals("d", RegExp.$4);
+  assertEquals("e", RegExp.$5);
+  assertEquals("f", RegExp.$6);
+  assertEquals("g", RegExp.$7);
+  assertEquals("h", RegExp.$8);
+  assertEquals("i", RegExp.$9);
+
+  assertEquals("abcdefghij", RegExp.lastMatch);
+  assertEquals("j", RegExp.lastParen);
+  assertEquals(">>>", RegExp.leftContext);
+  assertEquals("<<<", RegExp.rightContext);
+  assertEquals(">>>abcdefghij<<<", RegExp.input);
+}

test/debugger/debug/side-effect/debug-evaluate-no-side-effect-runtime-check.js

@@ -60,20 +60,70 @@ function return_literal_with_data_property(a) {
 
 success({foo: 1}, `return_literal_with_data_property('foo')`);
 
+// Set builtins with temporary objects
+var set = new Set([1,2]);
+fail(`set.add(3).size`);
+success(1, `new Set().add(1).size`);
+
+success(0, `(() => { const s = new Set([1]); s.delete(1); return s.size; })()`);
+fail(`set.delete(1)`);
+
+success(0, `(() => { const s = new Set([1]); s.clear(); return s.size; })()`);
+fail(`set.clear()`);
+
+// new set
+success(3, `(() => {
+  let s = 0;
+  for (const a of new Set([1,2]))
+    s += a;
+  return s;
+})()`);
+// existing set
+success(3, `(() => {
+  let s = 0;
+  for (const a of set)
+    s += a;
+  return s;
+})()`);
+// existing iterator
+var setIterator = set.entries();
+fail(`(() => {
+  let s = 0;
+  for (const a of setIterator)
+    s += a;
+  return s;
+})()`);
+
 // Array builtins with temporary objects
+success([1], `(() => { const a = []; a.push(1); return a; })()`);
+fail(`array.push(1)`);
+
+success([1], `(() => { const a = [1,2]; a.pop(); return a; })()`);
+fail(`array.pop()`);
+
+success([1,2,3], `[2,1,3].sort()`);
+fail(`array.sort()`);
 
+success([1,2], `(() => { const a = [2]; a.unshift(1); return a; })()`);
+fail(`array.unshift(1)`);
+success(1, `[1,2].shift()`);
+fail(`array.shift()`);
+
+// new array
 success(6, `(() => {
   let s = 0;
   for (const a of [1,2,3])
     s += a;
   return s;
 })()`);
+// existing array
 success(6, `(() => {
   let s = 0;
   for (const a of array)
     s += a;
   return s;
 })()`);
+// existing iterator
 var arrayIterator = array.entries();
 fail(`(() => {
   let s = 0;
@@ -82,10 +132,55 @@ fail(`(() => {
   return s;
 })()`);
 
-// SetAdd builtin on temporary object
-var set = new Set([1,2]);
-fail(`set.add(3).size`);
-success(1, `new Set().add(1).size`);
+success(6, `array.reduce((a,b) => a + b, 0)`);
+
+// Map builtins with temporary objects
+var map = new Map([[1,2]]);
+fail(`map.set(3, 4).size`);
+success(1, `new Map().set(1, 2).size`);
+
+success(0, `(() => {
+  const m = new Map([[1, 2]]);
+  m.delete(1);
+  return m.size;
+})()`);
+fail(`map.delete(1)`);
+
+success(0, `(() => {
+  const m = new Map([[1, 2]]);
+  m.clear();
+  return m.size;
+})()`);
+fail(`map.clear()`);
+
+// new set
+success(2, `(() => {
+  let s = 0;
+  for (const [a, b] of new Map([[1,2]]))
+    s += b;
+  return s;
+})()`);
+// existing set
+success(2, `(() => {
+  let s = 0;
+  for (const [a,b] of map)
+    s += b;
+  return s;
+})()`);
+// existing iterator
+var mapIterator = map.entries();
+fail(`(() => {
+  let s = 0;
+  for (const [a,b] of mapIterator)
+    s += a;
+  return s;
+})()`);
+
+// Regexps
+var regExp = /a/;
+success(true, `/a/.test('a')`);
+fail(`/a/.test({toString: () => {map.clear(); return 'a';}})`)
+fail(`regExp.test('a')`);
 
 function success(expectation, source) {
   const result = Debug.evaluateGlobal(source, true).value();

test/debugger/debug/side-effect/debug-evaluate-no-side-effect.js

@@ -93,6 +93,7 @@ function listener(event, exec_state, event_data, data) {
     fail("eval('Math.sin(1)')");
     fail("eval('exception = 1')");
     fail("global_eval('1')");
+    success(1, "(() => { var a = 1; return a++; })()")
   } catch (e) {
     exception = e;
     print(e, e.stack);