Stack allocate lexical locals + hoist stack slots

Review URL: https://codereview.chromium.org/981203003

Cr-Commit-Position: refs/heads/master@{#28008}

src/compiler/ast-graph-builder.cc

@@ -1022,10 +1022,15 @@ void AstGraphBuilder::VisitBlock(Block* stmt) {
     VisitStatements(stmt->statements());
   } else {
     // Visit declarations and statements in a block scope.
+    if (stmt->scope()->ContextLocalCount() > 0) {
       Node* context = BuildLocalBlockContext(stmt->scope());
       ContextScope scope(this, stmt->scope(), context);
       VisitDeclarations(stmt->scope()->declarations());
       VisitStatements(stmt->statements());
+    } else {
+      VisitDeclarations(stmt->scope()->declarations());
+      VisitStatements(stmt->statements());
+    }
   }
   if (stmt->labels() != NULL) block.EndBlock();
 }
@@ -1480,10 +1485,15 @@ void AstGraphBuilder::VisitClassLiteral(ClassLiteral* expr) {
     VisitClassLiteralContents(expr);
   } else {
     // Visit declarations and class literal in a block scope.
+    if (expr->scope()->ContextLocalCount() > 0) {
       Node* context = BuildLocalBlockContext(expr->scope());
       ContextScope scope(this, expr->scope(), context);
       VisitDeclarations(expr->scope()->declarations());
       VisitClassLiteralContents(expr);
+    } else {
+      VisitDeclarations(expr->scope()->declarations());
+      VisitClassLiteralContents(expr);
+    }
   }
 }
 

src/full-codegen.cc

@@ -1814,14 +1814,17 @@ bool BackEdgeTable::Verify(Isolate* isolate, Code* unoptimized) {
 FullCodeGenerator::EnterBlockScopeIfNeeded::EnterBlockScopeIfNeeded(
     FullCodeGenerator* codegen, Scope* scope, BailoutId entry_id,
     BailoutId declarations_id, BailoutId exit_id)
-    : codegen_(codegen), scope_(scope), exit_id_(exit_id) {
+    : codegen_(codegen), exit_id_(exit_id) {
   saved_scope_ = codegen_->scope();
 
   if (scope == NULL) {
     codegen_->PrepareForBailoutForId(entry_id, NO_REGISTERS);
+    needs_block_context_ = false;
   } else {
+    needs_block_context_ = scope->ContextLocalCount() > 0;
     codegen_->scope_ = scope;
     {
+      if (needs_block_context_) {
         Comment cmnt(masm(), "[ Extend block context");
         __ Push(scope->GetScopeInfo(codegen->isolate()));
         codegen_->PushFunctionArgumentForContextAllocation();
@@ -1830,6 +1833,8 @@ FullCodeGenerator::EnterBlockScopeIfNeeded::EnterBlockScopeIfNeeded(
         // Replace the context stored in the frame.
         codegen_->StoreToFrameField(StandardFrameConstants::kContextOffset,
                                     codegen_->context_register());
+      }
+      CHECK_EQ(0, scope->num_stack_slots());
       codegen_->PrepareForBailoutForId(entry_id, NO_REGISTERS);
     }
     {
@@ -1842,7 +1847,7 @@ FullCodeGenerator::EnterBlockScopeIfNeeded::EnterBlockScopeIfNeeded(
 
 
 FullCodeGenerator::EnterBlockScopeIfNeeded::~EnterBlockScopeIfNeeded() {
-  if (scope_ != NULL) {
+  if (needs_block_context_) {
     codegen_->LoadContextField(codegen_->context_register(),
                                Context::PREVIOUS_INDEX);
     // Update local stack frame context field.

src/full-codegen.h

@@ -16,6 +16,7 @@
 #include "src/compiler.h"
 #include "src/globals.h"
 #include "src/objects.h"
+#include "src/scopes.h"
 
 namespace v8 {
 namespace internal {
@@ -220,8 +221,9 @@ class FullCodeGenerator: public AstVisitor {
     virtual ~NestedBlock() {}
 
     virtual NestedStatement* Exit(int* stack_depth, int* context_length) {
-      if (statement()->AsBlock()->scope() != NULL) {
-        ++(*context_length);
+      auto block_scope = statement()->AsBlock()->scope();
+      if (block_scope != nullptr) {
+        if (block_scope->ContextLocalCount() > 0) ++(*context_length);
       }
       return previous_;
     }
@@ -968,9 +970,9 @@ class FullCodeGenerator: public AstVisitor {
     MacroAssembler* masm() const { return codegen_->masm(); }
 
     FullCodeGenerator* codegen_;
-    Scope* scope_;
     Scope* saved_scope_;
     BailoutId exit_id_;
+    bool needs_block_context_;
   };
 
   MacroAssembler* masm_;

src/hydrogen.cc

@@ -4512,14 +4512,16 @@ void HOptimizedGraphBuilder::VisitBlock(Block* stmt) {
 
   { BreakAndContinueScope push(&break_info, this);
     if (scope != NULL) {
+      if (scope->ContextLocalCount() > 0) {
         // Load the function object.
         Scope* declaration_scope = scope->DeclarationScope();
         HInstruction* function;
         HValue* outer_context = environment()->context();
         if (declaration_scope->is_script_scope() ||
             declaration_scope->is_eval_scope()) {
-        function = new(zone()) HLoadContextSlot(
-            outer_context, Context::CLOSURE_INDEX, HLoadContextSlot::kNoCheck);
+          function = new (zone())
+              HLoadContextSlot(outer_context, Context::CLOSURE_INDEX,
+                               HLoadContextSlot::kNoCheck);
         } else {
           function = New<HThisFunction>();
         }
@@ -4533,13 +4535,15 @@ void HOptimizedGraphBuilder::VisitBlock(Block* stmt) {
         if (instr->HasObservableSideEffects()) {
           AddSimulate(stmt->EntryId(), REMOVABLE_SIMULATE);
         }
+      }
       VisitDeclarations(scope->declarations());
       AddSimulate(stmt->DeclsId(), REMOVABLE_SIMULATE);
     }
     CHECK_BAILOUT(VisitStatements(stmt->statements()));
   }
   set_scope(outer_scope);
-  if (scope != NULL && current_block() != NULL) {
+  if (scope != NULL && current_block() != NULL &&
+      scope->ContextLocalCount() > 0) {
     HValue* inner_context = environment()->context();
     HValue* outer_context = Add<HLoadNamedField>(
         inner_context, nullptr,

src/objects.h

@@ -4288,6 +4288,9 @@ class ScopeInfo : public FixedArray {
   // Return the name of the given stack local.
   String* StackLocalName(int var);
 
+  // Return the name of the given stack local.
+  int StackLocalIndex(int var);
+
   // Return the name of the given context local.
   String* ContextLocalName(int var);
 
@@ -4396,33 +4399,38 @@ class ScopeInfo : public FixedArray {
   //    slot is used per parameter, so in total this part occupies
   //    ParameterCount() slots in the array. For other scopes than function
   //    scopes ParameterCount() is 0.
-  // 2. StackLocalEntries:
+  // 2. StackLocalFirstSlot:
+  //    Index of a first stack slot for stack local. Stack locals belonging to
+  //    this scope are located on a stack at slots starting from this index.
+  // 3. StackLocalEntries:
   //    Contains the names of local variables that are allocated on the stack,
-  //    in increasing order of the stack slot index. One slot is used per stack
-  //    local, so in total this part occupies StackLocalCount() slots in the
-  //    array.
-  // 3. ContextLocalNameEntries:
+  //    in increasing order of the stack slot index. First local variable has
+  //    a stack slot index defined in StackLocalFirstSlot (point 2 above).
+  //    One slot is used per stack local, so in total this part occupies
+  //    StackLocalCount() slots in the array.
+  // 4. ContextLocalNameEntries:
   //    Contains the names of local variables and parameters that are allocated
   //    in the context. They are stored in increasing order of the context slot
   //    index starting with Context::MIN_CONTEXT_SLOTS. One slot is used per
   //    context local, so in total this part occupies ContextLocalCount() slots
   //    in the array.
-  // 4. ContextLocalInfoEntries:
+  // 5. ContextLocalInfoEntries:
   //    Contains the variable modes and initialization flags corresponding to
   //    the context locals in ContextLocalNameEntries. One slot is used per
   //    context local, so in total this part occupies ContextLocalCount()
   //    slots in the array.
-  // 5. StrongModeFreeVariableNameEntries:
+  // 6. StrongModeFreeVariableNameEntries:
   //    Stores the names of strong mode free variables.
-  // 6. StrongModeFreeVariablePositionEntries:
+  // 7. StrongModeFreeVariablePositionEntries:
   //    Stores the locations (start and end position) of strong mode free
   //    variables.
-  // 7. FunctionNameEntryIndex:
+  // 8. FunctionNameEntryIndex:
   //    If the scope belongs to a named function expression this part contains
   //    information about the function variable. It always occupies two array
   //    slots:  a. The name of the function variable.
   //            b. The context or stack slot index for the variable.
   int ParameterEntriesIndex();
+  int StackLocalFirstSlotIndex();
   int StackLocalEntriesIndex();
   int ContextLocalNameEntriesIndex();
   int ContextLocalInfoEntriesIndex();

src/parser.cc

@@ -519,6 +519,7 @@ void ParserTraits::CheckPossibleEvalCall(Expression* expression,
   if (callee != NULL &&
       callee->raw_name() == parser_->ast_value_factory()->eval_string()) {
     scope->DeclarationScope()->RecordEvalCall();
+    scope->RecordEvalCall();
   }
 }
 
@@ -3339,8 +3340,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
       clear_first =
           factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
     }
-    Statement* clear_first_or_next =
-        factory()->NewIfStatement(compare, clear_first, next, next->position());
+    Statement* clear_first_or_next = factory()->NewIfStatement(
+        compare, clear_first, next, RelocInfo::kNoPosition);
     inner_block->AddStatement(clear_first_or_next, zone());
   }
 
@@ -4048,6 +4049,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     parenthesized_function_ = false;  // The bit was set for this function only.
 
     if (is_lazily_parsed) {
+      for (Scope* s = scope_->outer_scope();
+           s != nullptr && (s != s->DeclarationScope()); s = s->outer_scope()) {
+        s->ForceContextAllocation();
+      }
       SkipLazyFunctionBody(&materialized_literal_count,
                            &expected_property_count, CHECK_OK);
     } else {

src/runtime/runtime-debug.cc

@@ -686,17 +686,16 @@ static bool ParameterIsShadowedByContextLocal(Handle<ScopeInfo> info,
 // frame.
 MUST_USE_RESULT
 static MaybeHandle<JSObject> MaterializeStackLocalsWithFrameInspector(
-    Isolate* isolate, Handle<JSObject> target, Handle<JSFunction> function,
+    Isolate* isolate, Handle<JSObject> target, Handle<ScopeInfo> scope_info,
     FrameInspector* frame_inspector) {
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
-
   // 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.
     Handle<String> name(scope_info->ParameterName(i));
     if (ParameterIsShadowedByContextLocal(scope_info, name)) continue;
 
+    DCHECK_NOT_NULL(frame_inspector);
+
     HandleScope scope(isolate);
     Handle<Object> value(i < frame_inspector->GetParametersCount()
                              ? frame_inspector->GetParameter(i)
@@ -713,8 +712,12 @@ static MaybeHandle<JSObject> MaterializeStackLocalsWithFrameInspector(
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     if (scope_info->LocalIsSynthetic(i)) continue;
     Handle<String> name(scope_info->StackLocalName(i));
-    Handle<Object> value(frame_inspector->GetExpression(i), isolate);
-    if (value->IsTheHole()) continue;
+    Handle<Object> value(
+        frame_inspector->GetExpression(scope_info->StackLocalIndex(i)),
+        isolate);
+    if (value->IsTheHole()) {
+      value = isolate->factory()->undefined_value();
+    }
 
     RETURN_ON_EXCEPTION(isolate, Runtime::SetObjectProperty(
                                      isolate, target, name, value, SLOPPY),
@@ -724,12 +727,21 @@ static MaybeHandle<JSObject> MaterializeStackLocalsWithFrameInspector(
   return target;
 }
 
+MUST_USE_RESULT
+static MaybeHandle<JSObject> MaterializeStackLocalsWithFrameInspector(
+    Isolate* isolate, Handle<JSObject> target, Handle<JSFunction> function,
+    FrameInspector* frame_inspector) {
+  Handle<SharedFunctionInfo> shared(function->shared());
+  Handle<ScopeInfo> scope_info(shared->scope_info());
+
+  return MaterializeStackLocalsWithFrameInspector(isolate, target, scope_info,
+                                                  frame_inspector);
+}
 
-static void UpdateStackLocalsFromMaterializedObject(Isolate* isolate,
-                                                    Handle<JSObject> target,
-                                                    Handle<JSFunction> function,
-                                                    JavaScriptFrame* frame,
-                                                    int inlined_jsframe_index) {
+
+static void UpdateStackLocalsFromMaterializedObject(
+    Isolate* isolate, Handle<JSObject> target, Handle<ScopeInfo> scope_info,
+    JavaScriptFrame* frame, int inlined_jsframe_index) {
   if (inlined_jsframe_index != 0 || frame->is_optimized()) {
     // Optimized frames are not supported.
     // TODO(yangguo): make sure all code deoptimized when debugger is active
@@ -737,9 +749,6 @@ static void UpdateStackLocalsFromMaterializedObject(Isolate* isolate,
     return;
   }
 
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
-
   // Parameters.
   for (int i = 0; i < scope_info->ParameterCount(); ++i) {
     // Shadowed parameters were not materialized.
@@ -756,12 +765,13 @@ static void UpdateStackLocalsFromMaterializedObject(Isolate* isolate,
   // Stack locals.
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     if (scope_info->LocalIsSynthetic(i)) continue;
-    if (frame->GetExpression(i)->IsTheHole()) continue;
+    int index = scope_info->StackLocalIndex(i);
+    if (frame->GetExpression(index)->IsTheHole()) continue;
     HandleScope scope(isolate);
     Handle<Object> value = Object::GetPropertyOrElement(
                                target, handle(scope_info->StackLocalName(i),
                                               isolate)).ToHandleChecked();
-    frame->SetExpression(i, *value);
+    frame->SetExpression(index, *value);
   }
 }
 
@@ -903,7 +913,7 @@ static bool SetLocalVariableValue(Isolate* isolate, JavaScriptFrame* frame,
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     HandleScope scope(isolate);
     if (String::Equals(handle(scope_info->StackLocalName(i)), variable_name)) {
-      frame->SetExpression(i, *new_value);
+      frame->SetExpression(scope_info->StackLocalIndex(i), *new_value);
       return true;
     }
   }
@@ -940,6 +950,30 @@ static bool SetLocalVariableValue(Isolate* isolate, JavaScriptFrame* frame,
 }
 
 
+static bool SetBlockVariableValue(Isolate* isolate,
+                                  Handle<Context> block_context,
+                                  Handle<ScopeInfo> scope_info,
+                                  JavaScriptFrame* frame,
+                                  Handle<String> variable_name,
+                                  Handle<Object> new_value) {
+  if (frame != nullptr) {
+    for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+      HandleScope scope(isolate);
+      if (String::Equals(handle(scope_info->StackLocalName(i)),
+                         variable_name)) {
+        frame->SetExpression(scope_info->StackLocalIndex(i), *new_value);
+        return true;
+      }
+    }
+  }
+  if (!block_context.is_null()) {
+    return SetContextLocalValue(block_context->GetIsolate(), scope_info,
+                                block_context, variable_name, new_value);
+  }
+  return false;
+}
+
+
 // Create a plain JSObject which materializes the closure content for the
 // context.
 MUST_USE_RESULT static MaybeHandle<JSObject> MaterializeClosure(
@@ -1020,17 +1054,6 @@ static bool SetClosureVariableValue(Isolate* isolate, Handle<Context> context,
 }
 
 
-static bool SetBlockContextVariableValue(Handle<Context> block_context,
-                                         Handle<String> variable_name,
-                                         Handle<Object> new_value) {
-  DCHECK(block_context->IsBlockContext());
-  Handle<ScopeInfo> scope_info(ScopeInfo::cast(block_context->extension()));
-
-  return SetContextLocalValue(block_context->GetIsolate(), scope_info,
-                              block_context, variable_name, new_value);
-}
-
-
 static bool SetScriptVariableValue(Handle<Context> context,
                                    Handle<String> variable_name,
                                    Handle<Object> new_value) {
@@ -1082,20 +1105,28 @@ static bool SetCatchVariableValue(Isolate* isolate, Handle<Context> context,
 // Create a plain JSObject which materializes the block scope for the specified
 // block context.
 MUST_USE_RESULT static MaybeHandle<JSObject> MaterializeBlockScope(
-    Isolate* isolate, Handle<Context> context) {
-  DCHECK(context->IsBlockContext());
-  Handle<ScopeInfo> scope_info(ScopeInfo::cast(context->extension()));
-
-  // Allocate and initialize a JSObject with all the arguments, stack locals
-  // heap locals and extension properties of the debugged function.
+    Isolate* isolate, Handle<ScopeInfo> scope_info, Handle<Context> context,
+    JavaScriptFrame* frame, int inlined_jsframe_index) {
   Handle<JSObject> block_scope =
       isolate->factory()->NewJSObject(isolate->object_function());
 
+  if (frame != nullptr) {
+    FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
+    RETURN_ON_EXCEPTION(isolate,
+                        MaterializeStackLocalsWithFrameInspector(
+                            isolate, block_scope, scope_info, &frame_inspector),
+                        JSObject);
+  }
+
+  if (!context.is_null()) {
+    Handle<ScopeInfo> scope_info_from_context(
+        ScopeInfo::cast(context->extension()));
     // Fill all context locals.
-  if (!ScopeInfo::CopyContextLocalsToScopeObject(scope_info, context,
-                                                 block_scope)) {
+    if (!ScopeInfo::CopyContextLocalsToScopeObject(scope_info_from_context,
+                                                   context, block_scope)) {
       return MaybeHandle<JSObject>();
     }
+  }
 
   return block_scope;
 }
@@ -1361,8 +1392,20 @@ class ScopeIterator {
       case ScopeIterator::ScopeTypeClosure:
         // Materialize the content of the closure scope into a JSObject.
         return MaterializeClosure(isolate_, CurrentContext());
-      case ScopeIterator::ScopeTypeBlock:
-        return MaterializeBlockScope(isolate_, CurrentContext());
+      case ScopeIterator::ScopeTypeBlock: {
+        if (!nested_scope_chain_.is_empty()) {
+          // this is a block scope on the stack.
+          Handle<ScopeInfo> scope_info = nested_scope_chain_.last();
+          Handle<Context> context = scope_info->HasContext()
+                                        ? CurrentContext()
+                                        : Handle<Context>::null();
+          return MaterializeBlockScope(isolate_, scope_info, context, frame_,
+                                       inlined_jsframe_index_);
+        } else {
+          return MaterializeBlockScope(isolate_, Handle<ScopeInfo>::null(),
+                                       CurrentContext(), nullptr, 0);
+        }
+      }
       case ScopeIterator::ScopeTypeModule:
         return MaterializeModuleScope(isolate_, CurrentContext());
     }
@@ -1370,6 +1413,16 @@ class ScopeIterator {
     return Handle<JSObject>();
   }
 
+  bool HasContext() {
+    ScopeType type = Type();
+    if (type == ScopeTypeBlock || type == ScopeTypeLocal) {
+      if (!nested_scope_chain_.is_empty()) {
+        return nested_scope_chain_.last()->HasContext();
+      }
+    }
+    return true;
+  }
+
   bool SetVariableValue(Handle<String> variable_name,
                         Handle<Object> new_value) {
     DCHECK(!failed_);
@@ -1391,8 +1444,9 @@ class ScopeIterator {
         return SetScriptVariableValue(CurrentContext(), variable_name,
                                       new_value);
       case ScopeIterator::ScopeTypeBlock:
-        return SetBlockContextVariableValue(CurrentContext(), variable_name,
-                                            new_value);
+        return SetBlockVariableValue(
+            isolate_, HasContext() ? CurrentContext() : Handle<Context>::null(),
+            CurrentScopeInfo(), frame_, variable_name, new_value);
       case ScopeIterator::ScopeTypeModule:
         // TODO(2399): should we implement it?
         break;
@@ -2188,6 +2242,179 @@ static Handle<JSObject> NewJSObjectWithNullProto(Isolate* isolate) {
 }
 
 
+namespace {
+
+// This class builds a context chain for evaluation of expressions
+// in debugger.
+// The scope chain leading up to a breakpoint where evaluation occurs
+// looks like:
+// - [a mix of with, catch and block scopes]
+//    - [function stack + context]
+//      - [outer context]
+// The builder materializes all stack variables into properties of objects;
+// the expression is then evaluated as if it is inside a series of 'with'
+// statements using those objects. To this end, the builder builds a new
+// context chain, based on a scope chain:
+//   - every With and Catch scope begets a cloned context
+//   - Block scope begets one or two contexts:
+//       - if a block has context-allocated varaibles, its context is cloned
+//       - stack locals are materizalized as a With context
+//   - Local scope begets a With context for materizalized locals, chained to
+//     original function context. Original function context is the end of
+//     the chain.
+class EvaluationContextBuilder {
+ public:
+  EvaluationContextBuilder(Isolate* isolate, JavaScriptFrame* frame,
+                           int inlined_jsframe_index)
+      : isolate_(isolate),
+        frame_(frame),
+        inlined_jsframe_index_(inlined_jsframe_index) {
+    FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
+    Handle<JSFunction> function =
+        handle(JSFunction::cast(frame_inspector.GetFunction()));
+    Handle<Context> outer_context = handle(function->context(), isolate);
+    outer_info_ = handle(function->shared());
+    Handle<Context> inner_context;
+
+    bool stop = false;
+    for (ScopeIterator it(isolate, frame, inlined_jsframe_index);
+         !it.Failed() && !it.Done() && !stop; it.Next()) {
+      ScopeIterator::ScopeType scope_type = it.Type();
+
+      if (scope_type == ScopeIterator::ScopeTypeLocal) {
+        Handle<JSObject> materialized_function =
+            NewJSObjectWithNullProto(isolate);
+
+        if (!MaterializeStackLocalsWithFrameInspector(
+                 isolate, materialized_function, function, &frame_inspector)
+                 .ToHandle(&materialized_function))
+          return;
+
+        if (!MaterializeArgumentsObject(isolate, materialized_function,
+                                        function)
+                 .ToHandle(&materialized_function))
+          return;
+
+        Handle<Context> parent_context =
+            it.HasContext() ? it.CurrentContext() : outer_context;
+        Handle<Context> with_context = isolate->factory()->NewWithContext(
+            function, parent_context, materialized_function);
+
+        ContextChainElement context_chain_element;
+        context_chain_element.original_context = it.CurrentContext();
+        context_chain_element.materialized_object = materialized_function;
+        context_chain_element.scope_info = it.CurrentScopeInfo();
+        context_chain_.Add(context_chain_element);
+
+        stop = true;
+        RecordContextsInChain(&inner_context, with_context, with_context);
+      } else if (scope_type == ScopeIterator::ScopeTypeCatch ||
+                 scope_type == ScopeIterator::ScopeTypeWith) {
+        Handle<Context> cloned_context =
+            Handle<Context>::cast(FixedArray::CopySize(
+                it.CurrentContext(), it.CurrentContext()->length()));
+
+        ContextChainElement context_chain_element;
+        context_chain_element.original_context = it.CurrentContext();
+        context_chain_element.cloned_context = cloned_context;
+        context_chain_.Add(context_chain_element);
+
+        RecordContextsInChain(&inner_context, cloned_context, cloned_context);
+      } else if (scope_type == ScopeIterator::ScopeTypeBlock) {
+        Handle<JSObject> materialized_object =
+            NewJSObjectWithNullProto(isolate);
+        if (!MaterializeStackLocalsWithFrameInspector(
+                 isolate, materialized_object, it.CurrentScopeInfo(),
+                 &frame_inspector).ToHandle(&materialized_object))
+          return;
+        if (it.HasContext()) {
+          Handle<Context> cloned_context =
+              Handle<Context>::cast(FixedArray::CopySize(
+                  it.CurrentContext(), it.CurrentContext()->length()));
+          Handle<Context> with_context = isolate->factory()->NewWithContext(
+              function, cloned_context, materialized_object);
+
+          ContextChainElement context_chain_element;
+          context_chain_element.original_context = it.CurrentContext();
+          context_chain_element.cloned_context = cloned_context;
+          context_chain_element.materialized_object = materialized_object;
+          context_chain_element.scope_info = it.CurrentScopeInfo();
+          context_chain_.Add(context_chain_element);
+
+          RecordContextsInChain(&inner_context, cloned_context, with_context);
+        } else {
+          Handle<Context> with_context = isolate->factory()->NewWithContext(
+              function, outer_context, materialized_object);
+
+          ContextChainElement context_chain_element;
+          context_chain_element.materialized_object = materialized_object;
+          context_chain_element.scope_info = it.CurrentScopeInfo();
+          context_chain_.Add(context_chain_element);
+
+          RecordContextsInChain(&inner_context, with_context, with_context);
+        }
+      } else {
+        stop = true;
+      }
+    }
+    if (innermost_context_.is_null()) {
+      innermost_context_ = outer_context;
+    }
+    DCHECK(!innermost_context_.is_null());
+  }
+
+  void UpdateVariables() {
+    for (int i = 0; i < context_chain_.length(); i++) {
+      ContextChainElement element = context_chain_[i];
+      if (!element.original_context.is_null() &&
+          !element.cloned_context.is_null()) {
+        Handle<Context> cloned_context = element.cloned_context;
+        cloned_context->CopyTo(
+            Context::MIN_CONTEXT_SLOTS, *element.original_context,
+            Context::MIN_CONTEXT_SLOTS,
+            cloned_context->length() - Context::MIN_CONTEXT_SLOTS);
+      }
+      if (!element.materialized_object.is_null()) {
+        // Write back potential changes to materialized stack locals to the
+        // stack.
+        UpdateStackLocalsFromMaterializedObject(
+            isolate_, element.materialized_object, element.scope_info, frame_,
+            inlined_jsframe_index_);
+      }
+    }
+  }
+
+  Handle<Context> innermost_context() const { return innermost_context_; }
+  Handle<SharedFunctionInfo> outer_info() const { return outer_info_; }
+
+ private:
+  struct ContextChainElement {
+    Handle<Context> original_context;
+    Handle<Context> cloned_context;
+    Handle<JSObject> materialized_object;
+    Handle<ScopeInfo> scope_info;
+  };
+
+  void RecordContextsInChain(Handle<Context>* inner_context,
+                             Handle<Context> first, Handle<Context> last) {
+    if (!inner_context->is_null()) {
+      (*inner_context)->set_previous(*last);
+    } else {
+      innermost_context_ = last;
+    }
+    *inner_context = first;
+  }
+
+  Handle<SharedFunctionInfo> outer_info_;
+  Handle<Context> innermost_context_;
+  List<ContextChainElement> context_chain_;
+  Isolate* isolate_;
+  JavaScriptFrame* frame_;
+  int inlined_jsframe_index_;
+};
+}
+
+
 // Evaluate a piece of JavaScript in the context of a stack frame for
 // debugging.  Things that need special attention are:
 // - Parameters and stack-allocated locals need to be materialized.  Altered
@@ -2224,100 +2451,22 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluate) {
   isolate->set_context(*(save->context()));
 
   // Materialize stack locals and the arguments object.
-  Handle<JSObject> materialized;
-  Handle<JSFunction> function;
-  Handle<SharedFunctionInfo> outer_info;
-  Handle<Context> eval_context;
 
-  // We need to limit the lifetime of the FrameInspector because evaluation can
-  // call arbitrary code and only one FrameInspector can be active at a time.
-  {
-    FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
-    materialized = NewJSObjectWithNullProto(isolate);
-    function = handle(JSFunction::cast(frame_inspector.GetFunction()));
-    outer_info = handle(function->shared());
-    eval_context = handle(Context::cast(frame_inspector.GetContext()));
-
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, materialized,
-        MaterializeStackLocalsWithFrameInspector(isolate, materialized,
-                                                 function, &frame_inspector));
-
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, materialized,
-        MaterializeArgumentsObject(isolate, materialized, function));
-  }
-
-  // At this point, the lookup chain may look like this:
-  // [inner context] -> [function stack]+[function context] -> [outer context]
-  // The function stack is not an actual context, it complements the function
-  // context. In order to have the same lookup chain when debug-evaluating,
-  // we:
-  //  - clone inner context
-  //  - materialize the stack and insert it into the context chain as a
-  //    with-context before the function context.
-  // [inner context clone] -> [with context] -> [function context] ->
-  //      [outer context]
-  // Ordering the with-context before the function context forces a dynamic
-  // lookup instead of a static lookup that could fail as the scope info is
-  // outdated and may expect variables to still be stack-allocated.
-  // Afterwards, we write changes to the with-context back to the stack, and
-  // write changes in cloned contexts back to original contexts.
-
-  DCHECK(!eval_context.is_null());
-  Handle<Context> function_context = eval_context;
-  Handle<Context> outer_context(function->context(), isolate);
-  Handle<Context> inner_context;
-  Handle<Context> innermost_context;
-
-  // We iterate to find the function's context, cloning until we hit it.
-  // If the function has no context-allocated variables, we iterate until
-  // we hit the outer context.
-  while (!function_context->IsFunctionContext() &&
-         !function_context->IsScriptContext() &&
-         !function_context.is_identical_to(outer_context)) {
-    Handle<Context> clone = Handle<Context>::cast(
-        FixedArray::CopySize(function_context, function_context->length()));
-    if (!inner_context.is_null()) {
-      inner_context->set_previous(*clone);
-    } else {
-      innermost_context = clone;
-    }
-    inner_context = clone;
-    function_context = Handle<Context>(function_context->previous(), isolate);
+  EvaluationContextBuilder context_builder(isolate, frame,
+                                           inlined_jsframe_index);
+  if (isolate->has_pending_exception()) {
+    return isolate->heap()->exception();
   }
 
-  Handle<Context> materialized_context = isolate->factory()->NewWithContext(
-      function, function_context, materialized);
-
-  if (inner_context.is_null()) {
-    // No inner context. The with-context is now inner-most.
-    innermost_context = materialized_context;
-  } else {
-    inner_context->set_previous(*materialized_context);
-  }
 
   Handle<Object> receiver(frame->receiver(), isolate);
-  MaybeHandle<Object> maybe_result =
-      DebugEvaluate(isolate, outer_info, innermost_context, context_extension,
-                    receiver, source);
+  MaybeHandle<Object> maybe_result = DebugEvaluate(
+      isolate, context_builder.outer_info(),
+      context_builder.innermost_context(), context_extension, receiver, source);
 
   Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result, maybe_result);
-
-  // Write back potential changes to materialized stack locals to the stack.
-  UpdateStackLocalsFromMaterializedObject(isolate, materialized, function,
-                                          frame, inlined_jsframe_index);
-
-  while (!innermost_context.is_identical_to(materialized_context)) {
-    DCHECK(eval_context->map() == innermost_context->map());
-    innermost_context->CopyTo(
-        Context::MIN_CONTEXT_SLOTS, *eval_context, Context::MIN_CONTEXT_SLOTS,
-        innermost_context->length() - Context::MIN_CONTEXT_SLOTS);
-    innermost_context = handle(innermost_context->previous(), isolate);
-    eval_context = handle(eval_context->previous(), isolate);
-  }
-
+  context_builder.UpdateVariables();
   return *result;
 }
 

src/scopeinfo.cc

@@ -27,7 +27,6 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
   const int strong_mode_free_variable_count =
       strong_mode_free_variables.length();
   // Make sure we allocate the correct amount.
-  DCHECK(scope->StackLocalCount() == stack_local_count);
   DCHECK(scope->ContextLocalCount() == context_local_count);
 
   bool simple_parameter_list =
@@ -54,8 +53,8 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
 
   const bool has_function_name = function_name_info != NONE;
   const int parameter_count = scope->num_parameters();
-  const int length = kVariablePartIndex + parameter_count + stack_local_count +
-                     2 * context_local_count +
+  const int length = kVariablePartIndex + parameter_count +
+                     (1 + stack_local_count) + 2 * context_local_count +
                      3 * strong_mode_free_variable_count +
                      (has_function_name ? 2 : 0);
 
@@ -89,9 +88,17 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
   // Add stack locals' names. We are assuming that the stack locals'
   // slots are allocated in increasing order, so we can simply add
   // them to the ScopeInfo object.
+  int first_slot_index;
+  if (stack_local_count > 0) {
+    first_slot_index = stack_locals[0]->index();
+  } else {
+    first_slot_index = 0;
+  }
+  DCHECK(index == scope_info->StackLocalFirstSlotIndex());
+  scope_info->set(index++, Smi::FromInt(first_slot_index));
   DCHECK(index == scope_info->StackLocalEntriesIndex());
   for (int i = 0; i < stack_local_count; ++i) {
-    DCHECK(stack_locals[i]->index() == i);
+    DCHECK(stack_locals[i]->index() == first_slot_index + i);
     scope_info->set(index++, *stack_locals[i]->name());
   }
 
@@ -145,16 +152,12 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
     int var_index = scope->function()->proxy()->var()->index();
     scope_info->set(index++, *scope->function()->proxy()->name());
     scope_info->set(index++, Smi::FromInt(var_index));
-    DCHECK(function_name_info != STACK ||
-           (var_index == scope_info->StackLocalCount() &&
-            var_index == scope_info->StackSlotCount() - 1));
     DCHECK(function_name_info != CONTEXT ||
            var_index == scope_info->ContextLength() - 1);
   }
 
   DCHECK(index == scope_info->length());
   DCHECK(scope->num_parameters() == scope_info->ParameterCount());
-  DCHECK(scope->num_stack_slots() == scope_info->StackSlotCount());
   DCHECK(scope->num_heap_slots() == scope_info->ContextLength() ||
          (scope->num_heap_slots() == kVariablePartIndex &&
           scope_info->ContextLength() == 0));
@@ -269,6 +272,13 @@ String* ScopeInfo::StackLocalName(int var) {
 }
 
 
+int ScopeInfo::StackLocalIndex(int var) {
+  DCHECK(0 <= var && var < StackLocalCount());
+  int first_slot_index = Smi::cast(get(StackLocalFirstSlotIndex()))->value();
+  return first_slot_index + var;
+}
+
+
 String* ScopeInfo::ContextLocalName(int var) {
   DCHECK(0 <= var && var < ContextLocalCount());
   int info_index = ContextLocalNameEntriesIndex() + var;
@@ -343,11 +353,12 @@ int ScopeInfo::StrongModeFreeVariableEndPosition(int var) {
 int ScopeInfo::StackSlotIndex(String* name) {
   DCHECK(name->IsInternalizedString());
   if (length() > 0) {
+    int first_slot_index = Smi::cast(get(StackLocalFirstSlotIndex()))->value();
     int start = StackLocalEntriesIndex();
     int end = StackLocalEntriesIndex() + StackLocalCount();
     for (int i = start; i < end; ++i) {
       if (name == get(i)) {
-        return i - start;
+        return i - start + first_slot_index;
       }
     }
   }
@@ -453,7 +464,7 @@ bool ScopeInfo::CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
     if (scope_info->LocalIsSynthetic(first_context_var + i)) continue;
     int context_index = Context::MIN_CONTEXT_SLOTS + i;
     Handle<Object> value = Handle<Object>(context->get(context_index), isolate);
-    // Do not reflect variables under TDZ in scope object.
+    // Reflect variables under TDZ as undefined in scope object.
     if (value->IsTheHole()) continue;
     RETURN_ON_EXCEPTION_VALUE(
         isolate, Runtime::DefineObjectProperty(
@@ -472,11 +483,16 @@ int ScopeInfo::ParameterEntriesIndex() {
 }
 
 
-int ScopeInfo::StackLocalEntriesIndex() {
+int ScopeInfo::StackLocalFirstSlotIndex() {
   return ParameterEntriesIndex() + ParameterCount();
 }
 
 
+int ScopeInfo::StackLocalEntriesIndex() {
+  return StackLocalFirstSlotIndex() + 1;
+}
+
+
 int ScopeInfo::ContextLocalNameEntriesIndex() {
   return StackLocalEntriesIndex() + StackLocalCount();
 }

src/scopes.cc

@@ -371,6 +371,7 @@ Scope* Scope::FinalizeBlockScope() {
   if (uses_arguments()) outer_scope_->RecordArgumentsUsage();
   if (uses_super_property()) outer_scope_->RecordSuperPropertyUsage();
   if (uses_this()) outer_scope_->RecordThisUsage();
+  if (scope_calls_eval_) outer_scope_->RecordEvalCall();
 
   return NULL;
 }
@@ -381,13 +382,13 @@ Variable* Scope::LookupLocal(const AstRawString* name) {
   if (result != NULL || scope_info_.is_null()) {
     return result;
   }
+  Handle<String> name_handle = name->string();
   // The Scope is backed up by ScopeInfo. This means it cannot operate in a
   // heap-independent mode, and all strings must be internalized immediately. So
   // it's ok to get the Handle<String> here.
-  Handle<String> name_handle = name->string();
   // If we have a serialized scope info, we might find the variable there.
   // There should be no local slot with the given name.
-  DCHECK(scope_info_->StackSlotIndex(*name_handle) < 0);
+  DCHECK(scope_info_->StackSlotIndex(*name_handle) < 0 || is_block_scope());
 
   // Check context slot lookup.
   VariableMode mode;
@@ -706,6 +707,7 @@ bool Scope::HasLazyCompilableOuterContext() const {
   bool found_non_trivial_declarations = false;
   for (const Scope* scope = outer; scope != NULL; scope = scope->outer_scope_) {
     if (scope->is_with_scope() && !found_non_trivial_declarations) return false;
+    if (scope->is_block_scope() && !scope->decls_.is_empty()) return false;
     if (scope->is_declaration_scope() && scope->num_heap_slots() > 0) {
       found_non_trivial_declarations = true;
     }
@@ -1288,7 +1290,7 @@ bool Scope::MustAllocateInContext(Variable* var) {
   if (has_forced_context_allocation()) return true;
   if (var->mode() == TEMPORARY) return false;
   if (var->mode() == INTERNAL) return true;
-  if (is_catch_scope() || is_block_scope() || is_module_scope()) return true;
+  if (is_catch_scope() || is_module_scope()) return true;
   if (is_script_scope() && IsLexicalVariableMode(var->mode())) return true;
   return var->has_forced_context_allocation() ||
       scope_calls_eval_ ||
@@ -1309,7 +1311,11 @@ bool Scope::HasArgumentsParameter(Isolate* isolate) {
 
 
 void Scope::AllocateStackSlot(Variable* var) {
+  if (is_block_scope()) {
+    DeclarationScope()->AllocateStackSlot(var);
+  } else {
     var->AllocateTo(Variable::LOCAL, num_stack_slots_++);
+  }
 }
 
 
@@ -1434,6 +1440,9 @@ void Scope::AllocateNonParameterLocals(Isolate* isolate) {
 
 
 void Scope::AllocateVariablesRecursively(Isolate* isolate) {
+  if (!already_resolved()) {
+    num_stack_slots_ = 0;
+  }
   // Allocate variables for inner scopes.
   for (int i = 0; i < inner_scopes_.length(); i++) {
     inner_scopes_[i]->AllocateVariablesRecursively(isolate);
@@ -1443,7 +1452,6 @@ void Scope::AllocateVariablesRecursively(Isolate* isolate) {
   // variables in inner scopes which might not had been resolved yet.
   if (already_resolved()) return;
   // The number of slots required for variables.
-  num_stack_slots_ = 0;
   num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
 
   // Allocate variables for this scope.

test/mjsunit/bugs/harmony/debug-blockscopes.js

@@ -52,6 +52,7 @@ function listener(event, exec_state, event_data, data) {
     }
   } catch (e) {
     exception = e;
+    print(e + e.stack);
   }
 }
 

test/mjsunit/debug-scopes.js

@@ -1054,10 +1054,9 @@ BeginTest("Classes and methods 1");
 listener_delegate = function(exec_state) {
   "use strict"
   CheckScopeChain([debug.ScopeType.Local,
-                   debug.ScopeType.Block,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
-  CheckScopeContent({C1: class { m() { debugger; }} }, 1, exec_state);
+  CheckScopeContent({}, 1, exec_state);
 };
 
 (function() {

test/mjsunit/es6/block-scoping-top-level.js

+// Copyright 2015 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.
+
+// Flags: --min-preparse-length=0
+'use strict';
+
+let xxx = 1;
+let f = undefined;
+{
+  let inner_x = xxx;
+  f = function() { return inner_x; };
+}
+
+assertSame(1, f());
+
+xxx = 42;
+{
+  f = function() { return inner_x1; };
+  let inner_x1 = xxx;
+}
+
+assertSame(42, f());
+
+xxx = 31;
+{
+  let inner_x1 = xxx;
+  try {
+    throw new Error();
+  } catch (e) {
+    f = function() { return inner_x1; };
+  }
+}
+assertSame(31, f());

test/mjsunit/es6/debug-blockscopes.js

@@ -498,7 +498,47 @@ listener_delegate = function(exec_state) {
                    debug.ScopeType.Local,
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
-  CheckScopeContent({}, 0, exec_state);
+  CheckScopeContent({x:undefined}, 0, exec_state);
 };
 uninitialized_1();
 EndTest();
+
+
+// Block scopes shadowing
+BeginTest("Block scopes shadowing 1");
+function shadowing_1() {
+  let i = 0;
+  {
+    let i = 5;
+    debugger;
+    assertEquals(27, i);
+  }
+  assertEquals(0, i);
+  debugger;
+  assertEquals(27, i);
+}
+
+listener_delegate = function (exec_state) {
+  exec_state.frame(0).evaluate("i = 27");
+}
+shadowing_1();
+EndTest();
+
+
+// Block scopes shadowing
+BeginTest("Block scopes shadowing 2");
+function shadowing_2() {
+  let i = 0;
+  {
+    let j = 5;
+    debugger;
+    assertEquals(27, j);
+  }
+  assertEquals(0, i);
+}
+
+listener_delegate = function (exec_state) {
+  exec_state.frame(0).evaluate("j = 27");
+}
+shadowing_2();
+EndTest();

test/mjsunit/es6/debug-function-scopes.js

@@ -107,13 +107,10 @@ var f2 = (function() {
 
 var mirror = Debug.MakeMirror(f2);
 
-assertEquals(6, mirror.scopeCount());
-
-// Implementation artifact: l4 isn't used in closure, but still it is saved.
-CheckScope(mirror.scope(0), { l4: 11 }, ScopeType.Block);
+assertEquals(5, mirror.scopeCount());
 
-CheckScope(mirror.scope(1), { l3: 9 }, ScopeType.Block);
-CheckScope(mirror.scope(2), { l1: 6, l2: 7 }, ScopeType.Block);
-CheckScope(mirror.scope(3), { v1:3, l0: 0, v3: 5, v6: 11 }, ScopeType.Closure);
-CheckScope(mirror.scope(4), { top_level_let: 255 }, ScopeType.Script);
-CheckScope(mirror.scope(5), {}, ScopeType.Global);
+CheckScope(mirror.scope(0), { l3: 9 }, ScopeType.Block);
+CheckScope(mirror.scope(1), { l2: 7 }, ScopeType.Block);
+CheckScope(mirror.scope(2), { v1:3, l0: 0, v3: 5, v6: 11 }, ScopeType.Closure);
+CheckScope(mirror.scope(3), { top_level_let: 255 }, ScopeType.Script);
+CheckScope(mirror.scope(4), {}, ScopeType.Global);

test/mjsunit/es6/regress/regress-468661.js

@@ -9,7 +9,7 @@ var exception = null;
 var break_count = 0;
 
 var expected_values =
-  [ReferenceError, ReferenceError, 0, 0, 0, 0, 0, 1, ReferenceError, ReferenceError];
+  [ReferenceError, undefined, 0, 0, 0, 0, 1, ReferenceError, ReferenceError];
 
 function listener(event, exec_state, event_data, data) {
   try {
@@ -39,6 +39,7 @@ function listener(event, exec_state, event_data, data) {
         assertTrue(v instanceof ReferenceError);
       } else {
         assertSame(expected_values[break_count], v);
+
       }
       ++break_count;
 
@@ -62,14 +63,14 @@ var sum = 0;
   debugger; // Break 0.
 
   for (let i=0; // Break 1.
-       i < 1;   // Break 2. // Break 3. // Break 6. // Break 7.
+       i < 1;   // Break 2. // Break 5. // Break 6.
        i++) {
-    let key = i; // Break 4.
-    sum += key;   // Break 5.
+    let key = i; // Break 3.
+    sum += key;   // Break 4.
   }
-}()); // Break 8.
+}()); // Break 7.
 
-assertNull(exception); // Break 9.
+assertNull(exception); // Break 8.
 assertEquals(expected_breaks, break_count);
 
 Debug.setListener(null);

test/mjsunit/mjsunit.status

@@ -114,6 +114,14 @@
   'debug-listbreakpoints': [PASS, NO_VARIANTS],  # arm64 nosnap with turbofan
   'debug-enable-disable-breakpoints': [PASS, NO_VARIANTS],  #arm64 nosnap with turbofan.
 
+  # Issue 4035: unexpected frame->context() in debugger
+  'regress/regress-crbug-107996': [PASS, NO_VARIANTS],
+  'regress/regress-crbug-171715': [PASS, NO_VARIANTS],
+  'regress/regress-crbug-222893': [PASS, NO_VARIANTS],
+  'regress/regress-325676': [PASS, NO_VARIANTS],
+  'debug-evaluate-closure': [PASS, NO_VARIANTS],
+  'debug-evaluate-with': [PASS, NO_VARIANTS],
+
   # TODO(mstarzinger): Optimizing top-level code flushed out some correctness
   # issues on ARM and ARM64.
   'es6/math-log2-log10': [PASS, NO_VARIANTS],  # on ARM and ARM64.