[debugger] Throw exception if 'var x' fails in debug eval in module

This is an extension to the fix landed in https://crrev.com/c/3295348.

We should also throw the exception when we are paused in a module.
This is a constellation that can only happen with debug-evaluate as
'eval's in modules are always indirect, whereas debug-evaluate uses
direct, sloppy eval.

R=bmeurer@chromium.org, leszeks@chromium.org

Bug: chromium:1352303
Change-Id: I7373462dc6ae419e0a1a05a385ab81f204ff03ce
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3976510
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83917}

src/runtime/runtime-scopes.cc

@@ -234,10 +234,17 @@ Object DeclareEvalHelper(Isolate* isolate, Handle<String> name,
   // context and not the declaration context.
   Handle<Context> context(isolate->context().declaration_context(), isolate);
 
+  // For debug-evaluate we always use sloppy eval, in which case context could
+  // also be a module context. As module contexts re-use the extension slot
+  // we need to check for this.
+  const bool is_debug_evaluate_in_module =
+      isolate->context().IsDebugEvaluateContext() && context->IsModuleContext();
+
   DCHECK(context->IsFunctionContext() || context->IsNativeContext() ||
          context->IsScriptContext() || context->IsEvalContext() ||
          (context->IsBlockContext() &&
-          context->scope_info().is_declaration_scope()));
+          context->scope_info().is_declaration_scope()) ||
+         is_debug_evaluate_in_module);
 
   bool is_var = value->IsUndefined(isolate);
   DCHECK_IMPLIES(!is_var, value->IsJSFunction());
@@ -288,10 +295,11 @@ Object DeclareEvalHelper(Isolate* isolate, Handle<String> name,
 
     object = Handle<JSObject>::cast(holder);
 
-  } else if (context->has_extension()) {
+  } else if (context->has_extension() && !is_debug_evaluate_in_module) {
     object = handle(context->extension_object(), isolate);
     DCHECK(object->IsJSContextExtensionObject());
-  } else if (context->scope_info().HasContextExtensionSlot()) {
+  } else if (context->scope_info().HasContextExtensionSlot() &&
+             !is_debug_evaluate_in_module) {
     // Sloppy varblock and function contexts might not have an extension object
     // yet. Sloppy eval will never have an extension object, as vars are hoisted
     // out, and lets are known statically.

test/inspector/regress/regress-crbug-1352303-expected.txt

@@ -0,0 +1,8 @@
+Don't crash when creating a new var in a module via Debugger.evaluateOnCallFrame
+{
+    className : EvalError
+    description : EvalError: Identifier 'test' cannot be declared with 'var' in current evaluation scope, consider trying 'let' instead     at eval (eval at foo (module1:2:48), <anonymous>:1:1)     at foo (module1:2:48)     at module1:2:60
+    objectId : <objectId>
+    subtype : error
+    type : object
+}

test/inspector/regress/regress-crbug-1352303.js

@@ -0,0 +1,21 @@
+// Copyright 2022 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.
+
+const { Protocol, contextGroup } = InspectorTest.start('Don\'t crash when creating a new var in a module via Debugger.evaluateOnCallFrame');
+
+(async () => {
+  await Protocol.Debugger.enable();
+
+  contextGroup.addModule(`
+    (function foo() { eval('var test2 = 42;'); debugger; })();
+  `, 'module1');
+
+  const { params: { callFrames: [{ callFrameId }] }} = await Protocol.Debugger.oncePaused();
+  const { result: { result }} = await Protocol.Debugger.evaluateOnCallFrame({
+    expression: 'var test = 1;',
+    callFrameId,
+  });
+  InspectorTest.logMessage(result);
+  InspectorTest.completeTest();
+})();