[modules] Fix setting variables via debug-scopes.

I incorrectly assumed that ScopeIterator::SetModuleVariableValue gets called
when the frame is the module function.

R=jgruber@chromium.org, kozyatinskiy@chromium.org

Bug: v8:1569, v8:6484
Change-Id: I1fbad8ccde57280149547c78e679527f7a0c89dd
Reviewed-on: https://chromium-review.googlesource.com/535620Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45961}

src/debug/debug-scopes.cc

@@ -773,38 +773,36 @@ bool ScopeIterator::SetLocalVariableValue(Handle<String> variable_name,
 bool ScopeIterator::SetModuleVariableValue(Handle<String> variable_name,
                                            Handle<Object> new_value) {
   DCHECK_NOT_NULL(frame_inspector_);
-  JavaScriptFrame* frame = GetFrame();
+
+  // Get module context and its scope info.
   Handle<Context> context = CurrentContext();
-  Handle<ScopeInfo> scope_info(frame->function()->shared()->scope_info());
+  while (!context->IsModuleContext()) {
+    context = handle(context->previous(), isolate_);
+  }
+  Handle<ScopeInfo> scope_info(context->scope_info(), isolate_);
+  DCHECK_EQ(scope_info->scope_type(), MODULE_SCOPE);
 
   if (SetContextVariableValue(scope_info, context, variable_name, new_value)) {
     return true;
   }
 
-  bool found = false;
   int cell_index;
   {
-    int module_variable_count =
-        Smi::cast(scope_info->get(scope_info->ModuleVariableCountIndex()))
-            ->value();
-    for (int i = 0; i < module_variable_count; ++i) {
-      String* name;
-      scope_info->ModuleVariable(i, &name, &cell_index);
-      if (ModuleDescriptor::GetCellIndexKind(cell_index) ==
-              ModuleDescriptor::kExport &&
-          name->Equals(*variable_name)) {
-        found = true;
-        break;
-      }
-    }
+    VariableMode mode;
+    InitializationFlag init_flag;
+    MaybeAssignedFlag maybe_assigned_flag;
+    cell_index = scope_info->ModuleIndex(variable_name, &mode, &init_flag,
+                                         &maybe_assigned_flag);
   }
+
+  // Setting imports is currently not supported.
+  bool found = ModuleDescriptor::GetCellIndexKind(cell_index) ==
+               ModuleDescriptor::kExport;
   if (found) {
     Module::StoreVariable(handle(context->module(), isolate_), cell_index,
                           new_value);
-    return true;
   }
-
-  return false;
+  return found;
 }
 
 bool ScopeIterator::SetInnerScopeVariableValue(Handle<String> variable_name,

src/objects/scope-info.cc

@@ -614,7 +614,6 @@ int ScopeInfo::ModuleIndex(Handle<String> name, VariableMode* mode,
                            InitializationFlag* init_flag,
                            MaybeAssignedFlag* maybe_assigned_flag) {
   DCHECK_EQ(scope_type(), MODULE_SCOPE);
-  DCHECK(name->IsInternalizedString());
   DCHECK_NOT_NULL(mode);
   DCHECK_NOT_NULL(init_flag);
   DCHECK_NOT_NULL(maybe_assigned_flag);
@@ -622,7 +621,8 @@ int ScopeInfo::ModuleIndex(Handle<String> name, VariableMode* mode,
   int module_vars_count = Smi::cast(get(ModuleVariableCountIndex()))->value();
   int entry = ModuleVariablesIndex();
   for (int i = 0; i < module_vars_count; ++i) {
-    if (*name == get(entry + kModuleVariableNameOffset)) {
+    String* var_name = String::cast(get(entry + kModuleVariableNameOffset));
+    if (name->Equals(var_name)) {
       int index;
       ModuleVariable(i, nullptr, &index, mode, init_flag, maybe_assigned_flag);
       return index;

test/debugger/debug/debug-modules-set-variable-value.js

@@ -265,6 +265,28 @@ let salat = 12;
 }
 
 
+// Local (non-exported) variable, nested access.
+let salad = 12;
+{
+  function listener(event, exec_state) {
+    if (event == Debug.DebugEvent.Break) {
+      let scope_count = exec_state.frame().scopeCount();
+      let module_scope = exec_state.frame().scope(2);
+      assertEquals(debug.ScopeType.Module, module_scope.scopeType());
+      module_scope.setVariableValue('salad', 42);
+    }
+  }
+
+  Debug.setListener(listener);
+  function foo() {
+    assertEquals(12, salad);
+    debugger;
+    assertEquals(42, salad);
+  };
+  foo();
+}
+
+
 // Exported variable.
 export let salami = 1;
 {
@@ -283,6 +305,28 @@ export let salami = 1;
 }
 
 
+// Exported variable, nested access.
+export let ham = 1;
+{
+  function listener(event, exec_state) {
+    if (event == Debug.DebugEvent.Break) {
+      let scope_count = exec_state.frame().scopeCount();
+      let module_scope = exec_state.frame().scope(2);
+      assertEquals(debug.ScopeType.Module, module_scope.scopeType());
+      module_scope.setVariableValue('ham', 2);
+    }
+  }
+
+  Debug.setListener(listener);
+  function foo() {
+    assertEquals(1, ham);
+    debugger;
+    assertEquals(2, ham);
+  };
+  foo();
+}
+
+
 // Imported variable. Setting is currently not supported.
 import { salami as wurst } from "./debug-modules-set-variable-value.js";
 {
@@ -305,4 +349,30 @@ import { salami as wurst } from "./debug-modules-set-variable-value.js";
 }
 
 
+// Imported variable, nested access. Setting is currently not supported.
+import { salami as wurstl } from "./debug-modules-set-variable-value.js";
+{
+  let exception;
+  function listener(event, exec_state) {
+    if (event == Debug.DebugEvent.Break) {
+      let scope_count = exec_state.frame().scopeCount();
+      let module_scope = exec_state.frame().scope(2);
+      assertEquals(debug.ScopeType.Module, module_scope.scopeType());
+      try {
+        module_scope.setVariableValue('wurstl', 3);
+      } catch(e) { exception = e; }
+    }
+  }
+
+  Debug.setListener(listener);
+  function foo() {
+    assertEquals(2, wurstl);
+    debugger;
+    assertEquals(2, wurstl);
+    assertTrue(exception !== undefined);
+  };
+  foo();
+}
+
+
 Debug.setListener(null);