Revert "[debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather...

Revert "[debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather than ScopeInfo for inner scopes."

This reverts commit 9e27d473.

Reason for revert: Fails MSan (use of uninitialized value): https://ci.chromium.org/buildbot/client.v8/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/21562

Original change's description:
> [debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather than ScopeInfo for inner scopes.
> 
> This CL also bottlenecks all current scope handling in the ScopeIterator, and cleans up frame handling in debug-frames and the deoptimizer.
> 
> Change-Id: I061922a356ce17794262f8d77d5d7c824558fc50
> Reviewed-on: https://chromium-review.googlesource.com/1095094
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#53741}

TBR=yangguo@chromium.org,jarin@chromium.org,neis@chromium.org,jgruber@chromium.org,verwaest@chromium.org

Change-Id: Ief87c1e79fa2ec40f52fd747ec4ebbacf0da798b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/1101377Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53743}

src/ast/scopes.cc

@@ -2369,8 +2369,21 @@ void Scope::AllocateScopeInfosRecursively(Isolate* isolate,
   }
 }
 
+void Scope::AllocateDebuggerScopeInfos(Isolate* isolate,
+                                       MaybeHandle<ScopeInfo> outer_scope) {
+  if (scope_info_.is_null()) {
+    scope_info_ = ScopeInfo::Create(isolate, zone(), this, outer_scope);
+  }
+  MaybeHandle<ScopeInfo> outer = NeedsContext() ? scope_info_ : outer_scope;
+  for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
+    if (scope->is_function_scope()) continue;
+    scope->AllocateDebuggerScopeInfos(isolate, outer);
+  }
+}
+
 // static
-void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate) {
+void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate,
+                                          AnalyzeMode mode) {
   DeclarationScope* scope = info->literal()->scope();
   if (!scope->scope_info_.is_null()) return;  // Allocated by outer function.
 
@@ -2380,6 +2393,9 @@ void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate) {
   }
 
   scope->AllocateScopeInfosRecursively(isolate, outer_scope);
+  if (mode == AnalyzeMode::kDebugger) {
+    scope->AllocateDebuggerScopeInfos(isolate, outer_scope);
+  }
 
   // The debugger expects all shared function infos to contain a scope info.
   // Since the top-most scope will end up in a shared function info, make sure

src/ast/scopes.h

@@ -78,6 +78,8 @@ class SloppyBlockFunctionMap : public ZoneHashMap {
   int count_;
 };
 
+enum class AnalyzeMode { kRegular, kDebugger };
+
 // Global invariants after AST construction: Each reference (i.e. identifier)
 // to a JavaScript variable (including global properties) is represented by a
 // VariableProxy node. Immediately after AST construction and before variable
@@ -365,8 +367,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // Whether this needs to be represented by a runtime context.
   bool NeedsContext() const {
     // Catch scopes always have heap slots.
-    DCHECK_IMPLIES(is_catch_scope(), num_heap_slots() > 0);
-    DCHECK_IMPLIES(is_with_scope(), num_heap_slots() > 0);
+    DCHECK(!is_catch_scope() || num_heap_slots() > 0);
     return num_heap_slots() > 0;
   }
 
@@ -812,7 +813,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // The local variable 'arguments' if we need to allocate it; nullptr
   // otherwise.
   Variable* arguments() const {
-    DCHECK_IMPLIES(is_arrow_scope(), arguments_ == nullptr);
+    DCHECK(!is_arrow_scope() || arguments_ == nullptr);
     return arguments_;
   }
 
@@ -867,7 +868,8 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 
   // Allocate ScopeInfos for top scope and any inner scopes that need them.
   // Does nothing if ScopeInfo is already allocated.
-  static void AllocateScopeInfos(ParseInfo* info, Isolate* isolate);
+  static void AllocateScopeInfos(ParseInfo* info, Isolate* isolate,
+                                 AnalyzeMode mode);
 
   Handle<StringSet> CollectNonLocals(ParseInfo* info,
                                      Handle<StringSet> non_locals);

src/compiler-dispatcher/unoptimized-compile-job.cc

@@ -288,7 +288,8 @@ void UnoptimizedCompileJob::FinalizeOnMainThread(Isolate* isolate) {
     // Internalize ast values onto the heap.
     parse_info_->ast_value_factory()->Internalize(isolate);
     // Allocate scope infos for the literal.
-    DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate);
+    DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate,
+                                         AnalyzeMode::kRegular);
     if (compilation_job_->state() == CompilationJob::State::kFailed ||
         !Compiler::FinalizeCompilationJob(compilation_job_.release(), shared_,
                                           isolate)) {

src/compiler.cc

@@ -509,7 +509,8 @@ bool FinalizeUnoptimizedCode(
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
   // Allocate scope infos for the literal.
-  DeclarationScope::AllocateScopeInfos(parse_info, isolate);
+  DeclarationScope::AllocateScopeInfos(parse_info, isolate,
+                                       AnalyzeMode::kRegular);
 
   // Finalize the outer-most function's compilation job.
   if (FinalizeUnoptimizedCompilationJob(outer_function_job, shared_info,

src/debug/debug-evaluate.cc

@@ -159,23 +159,20 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
   return result;
 }
 
-Handle<SharedFunctionInfo> DebugEvaluate::ContextBuilder::outer_info() const {
-  return handle(frame_inspector_.GetFunction()->shared(), isolate_);
-}
 
 DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
                                               JavaScriptFrame* frame,
                                               int inlined_jsframe_index)
     : isolate_(isolate),
-      frame_inspector_(frame, inlined_jsframe_index, isolate),
-      scope_iterator_(isolate, &frame_inspector_,
-                      ScopeIterator::COLLECT_NON_LOCALS) {
-  Handle<Context> outer_context(frame_inspector_.GetFunction()->context());
+      frame_(frame),
+      inlined_jsframe_index_(inlined_jsframe_index) {
+  FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
+  Handle<JSFunction> local_function = frame_inspector.GetFunction();
+  Handle<Context> outer_context(local_function->context());
   evaluation_context_ = outer_context;
+  outer_info_ = handle(local_function->shared(), isolate);
   Factory* factory = isolate->factory();
 
-  if (scope_iterator_.Done()) return;
-
   // To evaluate as if we were running eval at the point of the debug break,
   // we reconstruct the context chain as follows:
   //  - To make stack-allocated variables visible, we materialize them and
@@ -192,32 +189,64 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
   //  - Look up in the materialized stack variables.
   //  - Look up in the original context.
   //  - Check the whitelist to find out whether to skip contexts during lookup.
-  for (; scope_iterator_.InInnerScope(); scope_iterator_.Next()) {
-    ScopeIterator::ScopeType scope_type = scope_iterator_.Type();
-    if (scope_type == ScopeIterator::ScopeTypeScript) break;
-    ContextChainElement context_chain_element;
-    if (scope_type == ScopeIterator::ScopeTypeLocal ||
-        scope_iterator_.DeclaresLocals(ScopeIterator::Mode::STACK)) {
-      context_chain_element.materialized_object =
-          scope_iterator_.ScopeObject(ScopeIterator::Mode::STACK);
-    }
-    if (scope_iterator_.HasContext()) {
-      context_chain_element.wrapped_context = scope_iterator_.CurrentContext();
-    }
+  const ScopeIterator::Option option = ScopeIterator::COLLECT_NON_LOCALS;
+  for (ScopeIterator it(isolate, &frame_inspector, option); !it.Done();
+       it.Next()) {
+    ScopeIterator::ScopeType scope_type = it.Type();
     if (scope_type == ScopeIterator::ScopeTypeLocal) {
-      context_chain_element.whitelist = scope_iterator_.GetNonLocals();
+      DCHECK_EQ(FUNCTION_SCOPE, it.CurrentScopeInfo()->scope_type());
+      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
+      Handle<Context> local_context =
+          it.HasContext() ? it.CurrentContext() : outer_context;
+      Handle<StringSet> non_locals = it.GetNonLocals();
+      MaterializeReceiver(materialized, local_context, local_function,
+                          non_locals);
+      MaterializeStackLocals(materialized, local_function, &frame_inspector);
+      ContextChainElement context_chain_element;
+      context_chain_element.scope_info = it.CurrentScopeInfo();
+      context_chain_element.materialized_object = materialized;
+      // Non-locals that are already being referenced by the current function
+      // are guaranteed to be correctly resolved.
+      context_chain_element.whitelist = non_locals;
+      if (it.HasContext()) {
+        context_chain_element.wrapped_context = it.CurrentContext();
+      }
+      context_chain_.push_back(context_chain_element);
+      evaluation_context_ = outer_context;
+      break;
+    } else if (scope_type == ScopeIterator::ScopeTypeCatch ||
+               scope_type == ScopeIterator::ScopeTypeWith ||
+               scope_type == ScopeIterator::ScopeTypeModule) {
+      ContextChainElement context_chain_element;
+      Handle<Context> current_context = it.CurrentContext();
+      if (!current_context->IsDebugEvaluateContext()) {
+        context_chain_element.wrapped_context = current_context;
+      }
+      context_chain_.push_back(context_chain_element);
+    } else if (scope_type == ScopeIterator::ScopeTypeBlock ||
+               scope_type == ScopeIterator::ScopeTypeEval) {
+      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
+      frame_inspector.MaterializeStackLocals(materialized,
+                                             it.CurrentScopeInfo());
+      ContextChainElement context_chain_element;
+      context_chain_element.scope_info = it.CurrentScopeInfo();
+      context_chain_element.materialized_object = materialized;
+      if (it.HasContext()) {
+        context_chain_element.wrapped_context = it.CurrentContext();
+      }
+      context_chain_.push_back(context_chain_element);
+    } else {
+      break;
     }
-    context_chain_.push_back(context_chain_element);
   }
 
-  Handle<ScopeInfo> scope_info =
-      evaluation_context_->IsNativeContext()
-          ? Handle<ScopeInfo>::null()
-          : handle(evaluation_context_->scope_info(), isolate);
   for (auto rit = context_chain_.rbegin(); rit != context_chain_.rend();
        rit++) {
     ContextChainElement element = *rit;
-    scope_info = ScopeInfo::CreateForWithScope(isolate, scope_info);
+    Handle<ScopeInfo> scope_info(ScopeInfo::CreateForWithScope(
+        isolate, evaluation_context_->IsNativeContext()
+                     ? Handle<ScopeInfo>::null()
+                     : Handle<ScopeInfo>(evaluation_context_->scope_info())));
     scope_info->SetIsDebugEvaluateScope();
     evaluation_context_ = factory->NewDebugEvaluateContext(
         evaluation_context_, scope_info, element.materialized_object,
@@ -227,24 +256,62 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
 
 
 void DebugEvaluate::ContextBuilder::UpdateValues() {
-  scope_iterator_.Restart();
   for (ContextChainElement& element : context_chain_) {
     if (!element.materialized_object.is_null()) {
-      Handle<FixedArray> keys =
-          KeyAccumulator::GetKeys(element.materialized_object,
-                                  KeyCollectionMode::kOwnOnly,
-                                  ENUMERABLE_STRINGS)
-              .ToHandleChecked();
-
-      for (int i = 0; i < keys->length(); i++) {
-        DCHECK(keys->get(i)->IsString());
-        Handle<String> key(String::cast(keys->get(i)), isolate_);
-        Handle<Object> value =
-            JSReceiver::GetDataProperty(element.materialized_object, key);
-        scope_iterator_.SetVariableValue(key, value);
-      }
+      // Write back potential changes to materialized stack locals to the stack.
+      FrameInspector(frame_, inlined_jsframe_index_, isolate_)
+          .UpdateStackLocalsFromMaterializedObject(element.materialized_object,
+                                                   element.scope_info);
     }
-    scope_iterator_.Next();
+  }
+}
+
+
+void DebugEvaluate::ContextBuilder::MaterializeReceiver(
+    Handle<JSObject> target, Handle<Context> local_context,
+    Handle<JSFunction> local_function, Handle<StringSet> non_locals) {
+  Handle<Object> recv = isolate_->factory()->undefined_value();
+  Handle<String> name = isolate_->factory()->this_string();
+  if (non_locals->Has(name)) {
+    // '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();
+}
+
+void DebugEvaluate::ContextBuilder::MaterializeStackLocals(
+    Handle<JSObject> target, Handle<JSFunction> function,
+    FrameInspector* frame_inspector) {
+  bool materialize_arguments_object = true;
+
+  // Do not materialize the arguments object for eval or top-level code.
+  if (function->shared()->is_toplevel()) materialize_arguments_object = false;
+
+  // First materialize stack locals (modulo arguments object).
+  Handle<SharedFunctionInfo> shared(function->shared());
+  Handle<ScopeInfo> scope_info(shared->scope_info());
+  frame_inspector->MaterializeStackLocals(target, scope_info,
+                                          materialize_arguments_object);
+
+  // Then materialize the arguments object.
+  if (materialize_arguments_object) {
+    // Skip if "arguments" is already taken and wasn't optimized out (which
+    // causes {MaterializeStackLocals} above to skip the local variable).
+    Handle<String> arguments_str = isolate_->factory()->arguments_string();
+    Maybe<bool> maybe = JSReceiver::HasOwnProperty(target, arguments_str);
+    DCHECK(maybe.IsJust());
+    if (maybe.FromJust()) return;
+
+    // FunctionGetArguments can't throw an exception.
+    Handle<JSObject> arguments =
+        Accessors::FunctionGetArguments(frame_, inlined_jsframe_index_);
+    JSObject::SetOwnPropertyIgnoreAttributes(target, arguments_str, arguments,
+                                             NONE)
+        .Check();
   }
 }
 

src/debug/debug-evaluate.h

@@ -7,8 +7,7 @@
 
 #include <vector>
 
-#include "src/debug/debug-frames.h"
-#include "src/debug/debug-scopes.h"
+#include "src/frames.h"
 #include "src/objects.h"
 #include "src/objects/shared-function-info.h"
 #include "src/objects/string-table.h"
@@ -71,20 +70,31 @@ class DebugEvaluate : public AllStatic {
     void UpdateValues();
 
     Handle<Context> evaluation_context() const { return evaluation_context_; }
-    Handle<SharedFunctionInfo> outer_info() const;
+    Handle<SharedFunctionInfo> outer_info() const { return outer_info_; }
 
    private:
     struct ContextChainElement {
+      Handle<ScopeInfo> scope_info;
       Handle<Context> wrapped_context;
       Handle<JSObject> materialized_object;
       Handle<StringSet> whitelist;
     };
 
+    void MaterializeReceiver(Handle<JSObject> target,
+                             Handle<Context> local_context,
+                             Handle<JSFunction> local_function,
+                             Handle<StringSet> non_locals);
+
+    void MaterializeStackLocals(Handle<JSObject> target,
+                                Handle<JSFunction> function,
+                                FrameInspector* frame_inspector);
+
+    Handle<SharedFunctionInfo> outer_info_;
     Handle<Context> evaluation_context_;
     std::vector<ContextChainElement> context_chain_;
     Isolate* isolate_;
-    FrameInspector frame_inspector_;
-    ScopeIterator scope_iterator_;
+    JavaScriptFrame* frame_;
+    int inlined_jsframe_index_;
   };
 
   static MaybeHandle<Object> Evaluate(Isolate* isolate,

src/debug/debug-frames.cc

@@ -15,7 +15,6 @@ namespace internal {
 FrameInspector::FrameInspector(StandardFrame* frame, int inlined_frame_index,
                                Isolate* isolate)
     : frame_(frame),
-      inlined_frame_index_(inlined_frame_index),
       isolate_(isolate) {
   // Extract the relevant information from the frame summary and discard it.
   FrameSummary summary = FrameSummary::Get(frame, inlined_frame_index);
@@ -83,6 +82,97 @@ bool FrameInspector::IsWasm() { return frame_->is_wasm(); }
 
 bool FrameInspector::IsJavaScript() { return frame_->is_java_script(); }
 
+// To inspect all the provided arguments the frame might need to be
+// replaced with the arguments frame.
+void FrameInspector::SetArgumentsFrame(StandardFrame* frame) {
+  DCHECK(has_adapted_arguments_);
+  DCHECK(frame->is_arguments_adaptor());
+  frame_ = frame;
+  is_optimized_ = frame_->is_optimized();
+  is_interpreted_ = frame_->is_interpreted();
+  DCHECK(!is_optimized_);
+}
+
+
+// Create a plain JSObject which materializes the local scope for the specified
+// frame.
+void FrameInspector::MaterializeStackLocals(Handle<JSObject> target,
+                                            Handle<ScopeInfo> scope_info,
+                                            bool materialize_arguments_object) {
+  HandleScope scope(isolate_);
+  // First fill all parameters.
+  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
+    // Do not materialize the parameter if it is shadowed by a context local.
+    // TODO(yangguo): check whether this is necessary, now that we materialize
+    //                context locals as well.
+    Handle<String> name(scope_info->ParameterName(i));
+    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+    if (ParameterIsShadowedByContextLocal(scope_info, name)) continue;
+
+    Handle<Object> value =
+        i < GetParametersCount()
+            ? GetParameter(i)
+            : Handle<Object>::cast(isolate_->factory()->undefined_value());
+    DCHECK(!value->IsTheHole(isolate_));
+
+    JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
+  }
+
+  // Second fill all stack locals.
+  for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+    Handle<String> name(scope_info->StackLocalName(i));
+    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+    Handle<Object> value = GetExpression(scope_info->StackLocalIndex(i));
+    // TODO(yangguo): We convert optimized out values to {undefined} when they
+    // are passed to the debugger. Eventually we should handle them somehow.
+    if (value->IsTheHole(isolate_)) {
+      value = isolate_->factory()->undefined_value();
+    }
+    if (value->IsOptimizedOut(isolate_)) {
+      if (materialize_arguments_object) {
+        Handle<String> arguments_str = isolate_->factory()->arguments_string();
+        if (String::Equals(name, arguments_str)) continue;
+      }
+      value = isolate_->factory()->undefined_value();
+    }
+    JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
+  }
+}
+
+
+void FrameInspector::UpdateStackLocalsFromMaterializedObject(
+    Handle<JSObject> target, Handle<ScopeInfo> scope_info) {
+  // Optimized frames and wasm frames are not supported. Simply give up.
+  if (is_optimized_ || frame_->is_wasm()) return;
+
+  HandleScope scope(isolate_);
+
+  // Parameters.
+  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
+    // Shadowed parameters were not materialized.
+    Handle<String> name(scope_info->ParameterName(i));
+    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+    if (ParameterIsShadowedByContextLocal(scope_info, name)) continue;
+
+    DCHECK(!javascript_frame()->GetParameter(i)->IsTheHole(isolate_));
+    Handle<Object> value =
+        Object::GetPropertyOrElement(target, name).ToHandleChecked();
+    javascript_frame()->SetParameterValue(i, *value);
+  }
+
+  // Stack locals.
+  for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+    Handle<String> name(scope_info->StackLocalName(i));
+    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+    int index = scope_info->StackLocalIndex(i);
+    if (frame_->GetExpression(index)->IsTheHole(isolate_)) continue;
+    Handle<Object> value =
+        Object::GetPropertyOrElement(target, name).ToHandleChecked();
+    frame_->SetExpression(index, *value);
+  }
+}
+
+
 bool FrameInspector::ParameterIsShadowedByContextLocal(
     Handle<ScopeInfo> info, Handle<String> parameter_name) {
   VariableMode mode;

src/debug/debug-frames.h

@@ -22,7 +22,7 @@ class FrameInspector {
   ~FrameInspector();
 
   int GetParametersCount();
-  Handle<JSFunction> GetFunction() const { return function_; }
+  Handle<JSFunction> GetFunction() { return function_; }
   Handle<Script> GetScript() { return script_; }
   Handle<Object> GetParameter(int index);
   Handle<Object> GetExpression(int index);
@@ -41,14 +41,21 @@ class FrameInspector {
                                           : JavaScriptFrame::cast(frame_);
   }
 
-  int inlined_frame_index() const { return inlined_frame_index_; }
+  JavaScriptFrame* GetArgumentsFrame() { return javascript_frame(); }
+  void SetArgumentsFrame(StandardFrame* frame);
+
+  void MaterializeStackLocals(Handle<JSObject> target,
+                              Handle<ScopeInfo> scope_info,
+                              bool materialize_arguments_object = false);
+
+  void UpdateStackLocalsFromMaterializedObject(Handle<JSObject> object,
+                                               Handle<ScopeInfo> scope_info);
 
  private:
   bool ParameterIsShadowedByContextLocal(Handle<ScopeInfo> info,
                                          Handle<String> parameter_name);
 
   StandardFrame* frame_;
-  int inlined_frame_index_;
   std::unique_ptr<DeoptimizedFrameInfo> deoptimized_frame_;
   wasm::WasmInterpreter::FramePtr wasm_interpreted_frame_;
   Isolate* isolate_;

src/debug/debug-scope-iterator.cc

@@ -75,8 +75,26 @@ void DebugScopeIterator::Advance() {
 }
 
 bool DebugScopeIterator::ShouldIgnore() {
-  if (GetType() == debug::ScopeIterator::ScopeTypeLocal) return false;
-  return !iterator_.DeclaresLocals(i::ScopeIterator::Mode::ALL);
+  // Almost always Script scope will be empty, so just filter out that noise.
+  // Also drop empty Block, Eval and Script scopes, should we get any.
+  DCHECK(!Done());
+  debug::ScopeIterator::ScopeType type = GetType();
+  if (type != debug::ScopeIterator::ScopeTypeBlock &&
+      type != debug::ScopeIterator::ScopeTypeScript &&
+      type != debug::ScopeIterator::ScopeTypeEval &&
+      type != debug::ScopeIterator::ScopeTypeModule) {
+    return false;
+  }
+
+  // TODO(kozyatinskiy): make this function faster.
+  Handle<JSObject> value;
+  if (!iterator_.ScopeObject().ToHandle(&value)) return false;
+  Handle<FixedArray> keys =
+      KeyAccumulator::GetKeys(value, KeyCollectionMode::kOwnOnly,
+                              ENUMERABLE_STRINGS,
+                              GetKeysConversion::kConvertToString)
+          .ToHandleChecked();
+  return keys->length() == 0;
 }
 
 v8::debug::ScopeIterator::ScopeType DebugScopeIterator::GetType() {
@@ -86,8 +104,11 @@ v8::debug::ScopeIterator::ScopeType DebugScopeIterator::GetType() {
 
 v8::Local<v8::Object> DebugScopeIterator::GetObject() {
   DCHECK(!Done());
-  Handle<JSObject> value = iterator_.ScopeObject(i::ScopeIterator::Mode::ALL);
-  return Utils::ToLocal(value);
+  Handle<JSObject> value;
+  if (iterator_.ScopeObject().ToHandle(&value)) {
+    return Utils::ToLocal(value);
+  }
+  return v8::Local<v8::Object>();
 }
 
 v8::Local<v8::Function> DebugScopeIterator::GetFunction() {

src/debug/debug-scopes.h

@@ -48,35 +48,32 @@ class ScopeIterator {
 
   ScopeIterator(Isolate* isolate, Handle<JSFunction> function);
   ScopeIterator(Isolate* isolate, Handle<JSGeneratorObject> generator);
-  ~ScopeIterator();
 
-  Handle<JSObject> MaterializeScopeDetails();
+  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeScopeDetails();
 
   // More scopes?
-  bool Done() const { return context_.is_null(); }
+  bool Done() { return context_.is_null(); }
 
   // Move to the next scope.
   void Next();
 
-  // Restart to the first scope and context.
-  void Restart();
-
   // Return the type of the current scope.
-  ScopeType Type() const;
-
-  // Indicates which variables should be visited. Either only variables from the
-  // scope that are available on the stack, or all variables.
-  enum class Mode { STACK, ALL };
+  ScopeType Type();
 
   // Return the JavaScript object with the content of the current scope.
-  Handle<JSObject> ScopeObject(Mode mode);
+  MaybeHandle<JSObject> ScopeObject();
 
-  // Returns whether the current scope declares any variables.
-  bool DeclaresLocals(Mode mode) const;
+  bool HasContext();
 
   // Set variable value and return true on success.
   bool SetVariableValue(Handle<String> variable_name, Handle<Object> new_value);
 
+  Handle<ScopeInfo> CurrentScopeInfo();
+
+  // Return the context for this scope. For the local context there might not
+  // be an actual context.
+  Handle<Context> CurrentContext();
+
   // Populate the set with collected non-local variable names.
   Handle<StringSet> GetNonLocals();
 
@@ -97,63 +94,96 @@ class ScopeIterator {
   void DebugPrint();
 #endif
 
-  bool InInnerScope() const { return !function_.is_null(); }
-  bool HasContext() const;
-  Handle<Context> CurrentContext() const {
-    DCHECK(HasContext());
-    return context_;
-  }
-
  private:
+  struct ExtendedScopeInfo {
+    ExtendedScopeInfo(Handle<ScopeInfo> info, int start, int end)
+        : scope_info(info), start_position(start), end_position(end) {}
+    explicit ExtendedScopeInfo(Handle<ScopeInfo> info)
+        : scope_info(info), start_position(-1), end_position(-1) {}
+    Handle<ScopeInfo> scope_info;
+    int start_position;
+    int end_position;
+    bool is_hidden() { return start_position == -1 && end_position == -1; }
+  };
+
   Isolate* isolate_;
-  ParseInfo* info_;
   FrameInspector* const frame_inspector_ = nullptr;
   Handle<JSGeneratorObject> generator_;
   Handle<JSFunction> function_;
+  Handle<ScopeInfo> function_scope_info_;
   Handle<Context> context_;
   Handle<Script> script_;
+  std::vector<ExtendedScopeInfo> nested_scope_chain_;
   Handle<StringSet> non_locals_;
-  DeclarationScope* closure_scope_;
-  Scope* start_scope_;
-  Scope* current_scope_;
   bool seen_script_scope_;
 
-  inline JavaScriptFrame* GetFrame() const {
-    return frame_inspector_->javascript_frame();
+  inline JavaScriptFrame* GetFrame() {
+    return frame_inspector_->GetArgumentsFrame();
   }
 
+  Handle<Context> GetContext();
   int GetSourcePosition();
 
+  void MaterializeStackLocals(Handle<JSObject> local_scope,
+                              Handle<ScopeInfo> scope_info);
+
   void TryParseAndRetrieveScopes(ScopeIterator::Option option);
 
   void RetrieveScopeChain(DeclarationScope* scope);
 
-  void UnwrapEvaluationContext();
+  void CollectNonLocals(ParseInfo* info, DeclarationScope* scope);
 
-  typedef std::function<bool(Handle<String> name, Handle<Object> value)>
-      Visitor;
+  void UnwrapEvaluationContext();
 
+  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeScriptScope();
+  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeLocalScope();
+  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeModuleScope();
+  Handle<JSObject> MaterializeClosure();
+  Handle<JSObject> MaterializeInnerScope();
   Handle<JSObject> WithContextExtension();
 
   bool SetLocalVariableValue(Handle<String> variable_name,
                              Handle<Object> new_value);
-  bool SetContextVariableValue(Handle<String> variable_name,
+  bool SetInnerScopeVariableValue(Handle<String> variable_name,
+                                  Handle<Object> new_value);
+  bool SetClosureVariableValue(Handle<String> variable_name,
                                Handle<Object> new_value);
-  bool SetContextExtensionValue(Handle<String> variable_name,
-                                Handle<Object> new_value);
   bool SetScriptVariableValue(Handle<String> variable_name,
                               Handle<Object> new_value);
   bool SetModuleVariableValue(Handle<String> variable_name,
                               Handle<Object> new_value);
 
   // Helper functions.
-  void VisitScope(const Visitor& visitor, Mode mode) const;
-  void VisitLocalScope(const Visitor& visitor, Mode mode) const;
-  void VisitScriptScope(const Visitor& visitor) const;
-  void VisitModuleScope(const Visitor& visitor) const;
-  bool VisitLocals(const Visitor& visitor, Mode mode) const;
-  bool VisitContextLocals(const Visitor& visitor, Handle<ScopeInfo> scope_info,
-                          Handle<Context> context) const;
+  bool SetParameterValue(Handle<ScopeInfo> scope_info,
+                         Handle<String> parameter_name,
+                         Handle<Object> new_value);
+  bool SetStackVariableValue(Handle<ScopeInfo> scope_info,
+                             Handle<String> variable_name,
+                             Handle<Object> new_value);
+  bool SetContextVariableValue(Handle<ScopeInfo> scope_info,
+                               Handle<Context> context,
+                               Handle<String> variable_name,
+                               Handle<Object> new_value);
+
+  void CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
+                                      Handle<Context> context,
+                                      Handle<JSObject> scope_object);
+  void CopyModuleVarsToScopeObject(Handle<ScopeInfo> scope_info,
+                                   Handle<Context> context,
+                                   Handle<JSObject> scope_object);
+  void CopyContextExtensionToScopeObject(Handle<Context> context,
+                                         Handle<JSObject> scope_object,
+                                         KeyCollectionMode mode);
+
+  // Get the chain of nested scopes within this scope for the source statement
+  // position. The scopes will be added to the list from the outermost scope to
+  // the innermost scope. Only nested block, catch or with scopes are tracked
+  // and will be returned, but no inner function scopes.
+  void GetNestedScopeChain(Isolate* isolate, Scope* scope,
+                           int statement_position);
+
+  bool HasNestedScopeChain() const;
+  ExtendedScopeInfo& LastNestedScopeChain();
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(ScopeIterator);
 };

src/deoptimizer.cc

@@ -2198,28 +2198,52 @@ Handle<Object> GetValueForDebugger(TranslatedFrame::iterator it,
 DeoptimizedFrameInfo::DeoptimizedFrameInfo(TranslatedState* state,
                                            TranslatedState::iterator frame_it,
                                            Isolate* isolate) {
-  int parameter_count =
-      frame_it->shared_info()->internal_formal_parameter_count();
-  TranslatedFrame::iterator stack_it = frame_it->begin();
+  // If the previous frame is an adaptor frame, we will take the parameters
+  // from there.
+  TranslatedState::iterator parameter_frame = frame_it;
+  if (parameter_frame != state->begin()) {
+    parameter_frame--;
+  }
+  int parameter_count;
+  if (parameter_frame->kind() == TranslatedFrame::kArgumentsAdaptor) {
+    parameter_count = parameter_frame->height() - 1;  // Ignore the receiver.
+  } else {
+    parameter_frame = frame_it;
+    parameter_count =
+        frame_it->shared_info()->internal_formal_parameter_count();
+  }
+  TranslatedFrame::iterator parameter_it = parameter_frame->begin();
+  parameter_it++;  // Skip the function.
+  parameter_it++;  // Skip the receiver.
 
-  // Get the function. Note that this might materialize the function.
-  // In case the debugger mutates this value, we should deoptimize
-  // the function and remember the value in the materialized value store.
-  function_ = Handle<JSFunction>::cast(stack_it->GetValue());
-  stack_it++;  // Skip the function.
-  stack_it++;  // Skip the receiver.
+  // Figure out whether there is a construct stub frame on top of
+  // the parameter frame.
+  has_construct_stub_ =
+      parameter_frame != state->begin() &&
+      (parameter_frame - 1)->kind() == TranslatedFrame::kConstructStub;
 
   DCHECK_EQ(TranslatedFrame::kInterpretedFunction, frame_it->kind());
   source_position_ = Deoptimizer::ComputeSourcePositionFromBytecodeArray(
       *frame_it->shared_info(), frame_it->node_id());
 
-  DCHECK_EQ(parameter_count,
-            function_->shared()->internal_formal_parameter_count());
+  TranslatedFrame::iterator value_it = frame_it->begin();
+  // Get the function. Note that this might materialize the function.
+  // In case the debugger mutates this value, we should deoptimize
+  // the function and remember the value in the materialized value store.
+  function_ = Handle<JSFunction>::cast(value_it->GetValue());
 
   parameters_.resize(static_cast<size_t>(parameter_count));
   for (int i = 0; i < parameter_count; i++) {
-    Handle<Object> parameter = GetValueForDebugger(stack_it, isolate);
+    Handle<Object> parameter = GetValueForDebugger(parameter_it, isolate);
     SetParameter(i, parameter);
+    parameter_it++;
+  }
+
+  // Skip the function, the receiver and the arguments.
+  int skip_count =
+      frame_it->shared_info()->internal_formal_parameter_count() + 2;
+  TranslatedFrame::iterator stack_it = frame_it->begin();
+  for (int i = 0; i < skip_count; i++) {
     stack_it++;
   }
 

src/deoptimizer.h

@@ -1025,6 +1025,12 @@ class DeoptimizedFrameInfo : public Malloced {
   // Get the frame context.
   Handle<Object> GetContext() { return context_; }
 
+  // Check if this frame is preceded by construct stub frame.  The bottom-most
+  // inlined frame might still be called by an uninlined construct stub.
+  bool HasConstructStub() {
+    return has_construct_stub_;
+  }
+
   // Get an incoming argument.
   Handle<Object> GetParameter(int index) {
     DCHECK(0 <= index && index < parameters_count());
@@ -1056,6 +1062,7 @@ class DeoptimizedFrameInfo : public Malloced {
 
   Handle<JSFunction> function_;
   Handle<Object> context_;
+  bool has_construct_stub_;
   std::vector<Handle<Object> > parameters_;
   std::vector<Handle<Object> > expression_stack_;
   int source_position_;

src/objects/scope-info.cc

@@ -740,7 +740,6 @@ int ScopeInfo::StackSlotIndex(String* name) const {
 int ScopeInfo::ModuleIndex(Handle<String> name, VariableMode* mode,
                            InitializationFlag* init_flag,
                            MaybeAssignedFlag* maybe_assigned_flag) {
-  DCHECK(name->IsInternalizedString());
   DCHECK_EQ(scope_type(), MODULE_SCOPE);
   DCHECK_NOT_NULL(mode);
   DCHECK_NOT_NULL(init_flag);

src/runtime/runtime-debug.cc

@@ -344,7 +344,7 @@ RUNTIME_FUNCTION(Runtime_GetGeneratorScopeDetails) {
     return isolate->heap()->undefined_value();
   }
 
-  return *it.MaterializeScopeDetails();
+  RETURN_RESULT_OR_FAILURE(isolate, it.MaterializeScopeDetails());
 }
 
 static bool SetScopeVariableValue(ScopeIterator* it, int index,

test/cctest/test-parsing.cc

@@ -648,7 +648,8 @@ TEST(ScopeUsesArgumentsSuperThis) {
       CHECK(i::Rewriter::Rewrite(&info));
       info.ast_value_factory()->Internalize(isolate);
       CHECK(i::DeclarationScope::Analyze(&info));
-      i::DeclarationScope::AllocateScopeInfos(&info, isolate);
+      i::DeclarationScope::AllocateScopeInfos(&info, isolate,
+                                              i::AnalyzeMode::kRegular);
       CHECK_NOT_NULL(info.literal());
 
       i::DeclarationScope* script_scope = info.literal()->scope();
@@ -10125,7 +10126,8 @@ TEST(LexicalLoopVariable) {
     CHECK(i::parsing::ParseProgram(&info, isolate));
     CHECK(i::Rewriter::Rewrite(&info));
     CHECK(i::DeclarationScope::Analyze(&info));
-    i::DeclarationScope::AllocateScopeInfos(&info, isolate);
+    i::DeclarationScope::AllocateScopeInfos(&info, isolate,
+                                            i::AnalyzeMode::kRegular);
     CHECK_NOT_NULL(info.literal());
 
     i::DeclarationScope* script_scope = info.literal()->scope();

test/debugger/debug/harmony/modules-debug-scopes2.js

@@ -142,7 +142,7 @@ listener_delegate = function(exec_state) {
       {exported_var: undefined, imported_var: undefined},
       0, exec_state);
   CheckScopeDoesNotHave(
-      ["doesntexist", "exported_let", "imported_let"],
+      ["local_var", "doesntexist", "local_let", "exported_let", "imported_let"],
       0, exec_state);
 };
 debugger;
@@ -163,7 +163,7 @@ listener_delegate = function(exec_state) {
   CheckScopeContent(
       {exported_let: 3, exported_var: 4,
        imported_let: 3, imported_var: 4}, 0, exec_state);
-  CheckScopeDoesNotHave([], 0, exec_state);
+  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
 };
 debugger;
 EndTest();
@@ -181,7 +181,7 @@ listener_delegate = function(exec_state) {
   CheckScopeContent(
       {exported_let: 13, exported_var: 14,
        imported_let: 13, imported_var: 14}, 0, exec_state);
-  CheckScopeDoesNotHave([], 0, exec_state);
+  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
 };
 debugger;
 EndTest();

test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt

@@ -4,7 +4,7 @@ Running test: testTotal
 foo1 (module1:7:2)
 foo2 (module2:6:9)
 (anonymous) (module3:4:0)
-local
+local:foo1
 [
     [0] : c1 = 12
     [1] : g1 = 2
@@ -94,7 +94,7 @@ foo1 =
     objectId : <objectId>
     type : function
 }
-local
+local:foo2
 [
     [0] : c2 = 22
 ]
@@ -180,8 +180,7 @@ foo2 =
 module
 [
     [0] : foo2 = function foo2() {   let c2 = 22;   return foo1() + a2 + b2 + c2; }
-    [1] : a3 = 30
-    [2] : b3 = 31
+    [1] : b3 = 31
 ]
 global
 [
@@ -206,20 +205,6 @@ foo2 =
     objectId : <objectId>
     type : function
 }
-a3 = 
-{
-    description : 30
-    type : number
-    value : 30
-}
-Evaluating: ++a3
-updated a3 = 
-{
-    description : 31
-    type : number
-    value : 31
-}
-Evaluating: --a3
 b3 = 
 {
     description : 31
@@ -239,7 +224,7 @@ Running test: testAnother
 (anonymous) (module4:5:13)
 bar (module4:5:24)
 (anonymous) (module4:7:0)
-local:bar
+local
 [
 ]
 closure:bar
@@ -310,12 +295,6 @@ updated a =
     value : 1
 }
 Evaluating: --a
-module
-[
-    [0] : a = 1
-    [1] : b = 2
-    [2] : bar = function bar() {   let a = 0;   (() => {a; debugger;})(); }
-]
 global
 [
   ...
@@ -331,41 +310,6 @@ Array =
     objectId : <objectId>
     type : function
 }
-a = 
-{
-    description : 1
-    type : number
-    value : 1
-}
-Evaluating: ++a
-updated a = 
-{
-    description : 2
-    type : number
-    value : 2
-}
-Evaluating: --a
-b = 
-{
-    description : 2
-    type : number
-    value : 2
-}
-Evaluating: ++b
-updated b = 
-{
-    description : 3
-    type : number
-    value : 3
-}
-Evaluating: --b
-bar = 
-{
-    className : Function
-    description : function bar() {   let a = 0;   (() => {a; debugger;})(); }
-    objectId : <objectId>
-    type : function
-}
 
 Running test: testDifferentModuleVariables
 (anonymous) (module5:5:0)
@@ -541,10 +485,6 @@ Evaluating: --x
 
 Running test: testLocalVariableToplevel
 (anonymous) (module7:2:0)
-module
-[
-    [0] : x = 5
-]
 global
 [
   ...
@@ -561,17 +501,3 @@ Array =
     objectId : <objectId>
     type : function
 }
-x = 
-{
-    description : 5
-    type : number
-    value : 5
-}
-Evaluating: ++x
-updated x = 
-{
-    description : 6
-    type : number
-    value : 6
-}
-Evaluating: --x