[inspector] Properly catch side effecting iterators.

Array spread syntax `[...obj]` is compiled to a special bytecode that
tries to take fast-paths for values special kinds of `obj`s, including
Set, Map, and Array iterator instances. But these fast-paths skip the
side-effect checks of `Runtime.evaluate` and friends, and thus lead to
surprises for developers.

This CL alters the behavior to always call the `next()` builtins when
the debugger is active to make sure we catch the side effects correctly.

Fixed: chromium:1255896
Change-Id: If3fc48a119cfa791c4fde7b5c586acc22dd973e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3226329
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77409}

src/builtins/builtins-iterator-gen.cc

@@ -316,6 +316,10 @@ void IteratorBuiltinsAssembler::FastIterableToList(
     TVariable<JSArray>* var_result, Label* slow) {
   Label done(this), check_string(this), check_map(this), check_set(this);
 
+  // Always call the `next()` builtins when the debugger is
+  // active, to ensure we capture side-effects correctly.
+  GotoIf(IsDebugActive(), slow);
+
   GotoIfNot(
       Word32Or(IsFastJSArrayWithNoCustomIteration(context, iterable),
                IsFastJSArrayForReadWithNoCustomIteration(context, iterable)),

test/inspector/debugger/side-effect-free-debug-evaluate-expected.txt

@@ -27,3 +27,16 @@ new Date().getHours() : ok
 
 Running test: testPromiseReject
 Promise.reject() : throws
+
+Running test: testSpread
+[...someGlobalArray] : ok
+[...someGlobalArray.values()] : ok
+[...someGlobalArrayIterator] : throws
+[...someGlobalMap] : ok
+[...someGlobalMap.keys()] : ok
+[...someGlobalMap.values()] : ok
+[...someGlobalMapKeysIterator] : throws
+[...someGlobalMapValuesIterator] : throws
+[...someGlobalSet] : ok
+[...someGlobalSet.values()] : ok
+[...someGlobalSetIterator] : throws

test/inspector/debugger/side-effect-free-debug-evaluate.js

@@ -5,7 +5,14 @@
 let {session, contextGroup, Protocol} = InspectorTest.start('Tests side-effect-free evaluation');
 
 contextGroup.addScript(`
+var someGlobalArray = [1, 2];
+var someGlobalArrayIterator = someGlobalArray[Symbol.iterator]();
 var someGlobalDate = new Date();
+var someGlobalMap = new Map([[1, 2], [3, 4]]);
+var someGlobalMapKeysIterator = someGlobalMap.keys();
+var someGlobalMapValuesIterator = someGlobalMap.values();
+var someGlobalSet = new Set([1, 2])
+var someGlobalSetIterator = someGlobalSet.values();
 function testFunction()
 {
   var o = 0;
@@ -20,7 +27,7 @@ async function testAsyncFunction(action) {
     case "reject": throw new Error();
   }
 }
-//# sourceURL=foo.js`);
+`, 0, 0, 'foo.js');
 
 const check = async (expression) => {
   const {result:{exceptionDetails}} = await Protocol.Runtime.evaluate({expression, throwOnSideEffect: true});
@@ -67,5 +74,19 @@ InspectorTest.runAsyncTestSuite([
 
   async function testPromiseReject() {
     await check('Promise.reject()');
+  },
+
+  async function testSpread() {
+    await check('[...someGlobalArray]');
+    await check('[...someGlobalArray.values()]');
+    await check('[...someGlobalArrayIterator]');
+    await check('[...someGlobalMap]');
+    await check('[...someGlobalMap.keys()]');
+    await check('[...someGlobalMap.values()]');
+    await check('[...someGlobalMapKeysIterator]');
+    await check('[...someGlobalMapValuesIterator]');
+    await check('[...someGlobalSet]');
+    await check('[...someGlobalSet.values()]');
+    await check('[...someGlobalSetIterator]');
   }
 ]);