Make non-Module generators only context allocate parameters.

src/debug/debug-scopes.cc

 // 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.
 
 #include "src/debug/debug-scopes.h"
 
 #include <memory>
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
@@ skipping 12 matching lines @@
                              ScopeIterator::Option option)
     : isolate_(isolate),
       frame_inspector_(frame_inspector),
       nested_scope_chain_(4),
       seen_script_scope_(false) {
   if (!frame_inspector->GetContext()->IsContext()) {
     // Optimized frame, context or function cannot be materialized. Give up.
     return;
   }
 
-  context_ = Handle<Context>::cast(frame_inspector->GetContext());
-
   // We should not instantiate a ScopeIterator for wasm frames.
   DCHECK(frame_inspector->GetScript()->type() != Script::TYPE_WASM);
 
+  TryParseAndRetrieveScopes(option);
+}
+
+void ScopeIterator::TryParseAndRetrieveScopes(ScopeIterator::Option option) {
+  context_ = GetFrameContext();
+
   // Catch the case when the debugger stops in an internal function.
   Handle<JSFunction> function = GetFunction();
   Handle<SharedFunctionInfo> shared_info(function->shared());
   Handle<ScopeInfo> scope_info(shared_info->scope_info());
-  if (shared_info->script()->IsUndefined(isolate)) {
+  if (shared_info->script()->IsUndefined(isolate_)) {
     while (context_->closure() == *function) {
       context_ = Handle<Context>(context_->previous(), isolate_);
     }
     return;
   }
 
   // Currently it takes too much time to find nested scopes due to script
   // parsing. Sometimes we want to run the ScopeIterator as fast as possible
   // (for example, while collecting async call stacks on every
   // addEventListener call), even if we drop some nested scopes.
   // Later we may optimize getting the nested scopes (cache the result?)
   // and include nested scopes into the "fast" iteration case as well.
   bool ignore_nested_scopes = (option == IGNORE_NESTED_SCOPES);
   bool collect_non_locals = (option == COLLECT_NON_LOCALS);
-  if (!ignore_nested_scopes && shared_info->HasDebugInfo()) {
+  if (!ignore_nested_scopes && shared_info->HasDebugInfo() &&
+      frame_inspector_ != nullptr) {
     // The source position at return is always the end of the function,
     // which is not consistent with the current scope chain. Therefore all
     // nested with, catch and block contexts are skipped, and we can only
     // inspect the function scope.
     // This can only happen if we set a break point inside right before the
     // return, which requires a debug info to be available.
     Handle<DebugInfo> debug_info(shared_info->GetDebugInfo());
 
     // Find the break point where execution has stopped.
     BreakLocation location = BreakLocation::FromFrame(debug_info, GetFrame());
@@ skipping 34 matching lines @@
       info->set_language_mode(shared_info->language_mode());
     } else if (scope_info->scope_type() == MODULE_SCOPE) {
       info->set_module();
     } else {
       DCHECK(scope_info->scope_type() == SCRIPT_SCOPE);
     }
   } else {
     // Inner function.
     info.reset(new ParseInfo(shared_info));
   }
-  if (parsing::ParseAny(info.get(), isolate) &&
-      Rewriter::Rewrite(info.get(), isolate)) {
+  if (parsing::ParseAny(info.get(), isolate_) &&
+      Rewriter::Rewrite(info.get(), isolate_)) {
     DeclarationScope* scope = info->literal()->scope();
     if (!ignore_nested_scopes || collect_non_locals) {
       CollectNonLocals(info.get(), scope);
     }
     if (!ignore_nested_scopes) {
       DeclarationScope::Analyze(info.get(), isolate_, AnalyzeMode::kDebugger);
       RetrieveScopeChain(scope);
     }
   } else {
     // A failed reparse indicates that the preparser has diverged from the
     // parser or that the preparse data given to the initial parse has been
     // faulty. We fail in debug mode but in release mode we only provide the
     // information we get from the context chain but nothing about
     // completely stack allocated scopes or stack allocated locals.
     // Or it could be due to stack overflow.
     // Silently fail by presenting an empty context chain.
     CHECK(isolate_->has_pending_exception());
     isolate_->clear_pending_exception();
     context_ = Handle<Context>();
   }
   UnwrapEvaluationContext();
 }
 
 ScopeIterator::ScopeIterator(Isolate* isolate, Handle<JSFunction> function)
     : isolate_(isolate),
-      frame_inspector_(NULL),
       context_(function->context()),
       seen_script_scope_(false) {
   if (!function->shared()->IsSubjectToDebugging()) context_ = Handle<Context>();
   UnwrapEvaluationContext();
 }
 
 ScopeIterator::ScopeIterator(Isolate* isolate,
                              Handle<JSGeneratorObject> generator)
     : isolate_(isolate),
-      frame_inspector_(NULL),
+      generator_(generator),
       context_(generator->context()),
       seen_script_scope_(false) {
   if (!generator->function()->shared()->IsSubjectToDebugging()) {
     context_ = Handle<Context>();
+    return;
   }
-  UnwrapEvaluationContext();
+  TryParseAndRetrieveScopes(DEFAULT);
 }
 
 void ScopeIterator::UnwrapEvaluationContext() {
   while (true) {
     if (context_.is_null()) return;
     if (!context_->IsDebugEvaluateContext()) return;
     Handle<Object> wrapped(context_->get(Context::WRAPPED_CONTEXT_INDEX),
                            isolate_);
     if (wrapped->IsContext()) {
       context_ = Handle<Context>::cast(wrapped);
@@ skipping 286 matching lines @@
           ->Print(os);
       break;
 
     default:
       UNREACHABLE();
   }
   PrintF("\n");
 }
 #endif
 
+inline Handle<Context> ScopeIterator::GetFrameContext() {
+  if (frame_inspector_) {
+    return Handle<Context>::cast(frame_inspector_->GetContext());
+  } else {
+    DCHECK(!generator_.is_null());

[neis, 2017/05/24 10:39:37]

Could you move these DCHECKs into the ScopeIterator constructor that takes a
generator?

[Jarin, 2017/05/24 12:32:21]

I can add this to the constructor, but here I want to say that we have either
generator or frame_inspector.

+    return handle(generator_->context());
+  }
+}
+
+Handle<JSFunction> ScopeIterator::GetFunction() {
+  if (frame_inspector_) {
+    return frame_inspector_->GetFunction();
+  } else {
+    DCHECK(!generator_.is_null());
+    return handle(generator_->function());
+  }
+}
+
+int ScopeIterator::GetSourcePosition() {
+  if (frame_inspector_) {
+    return frame_inspector_->GetSourcePosition();
+  } else {
+    DCHECK(!generator_.is_null());
+    return generator_->source_position();
+  }
+}
+
 void ScopeIterator::RetrieveScopeChain(DeclarationScope* scope) {
   DCHECK_NOT_NULL(scope);
-  int source_position = frame_inspector_->GetSourcePosition();
-  GetNestedScopeChain(isolate_, scope, source_position);
+  GetNestedScopeChain(isolate_, scope, GetSourcePosition());
 }
 
 void ScopeIterator::CollectNonLocals(ParseInfo* info, DeclarationScope* scope) {
   DCHECK_NOT_NULL(scope);
   DCHECK(non_locals_.is_null());
   non_locals_ = scope->CollectNonLocals(info, StringSet::New(isolate_));
 }
 
 
 MaybeHandle<JSObject> ScopeIterator::MaterializeScriptScope() {
   Handle<JSGlobalObject> global(CurrentContext()->global_object());
   Handle<ScriptContextTable> script_contexts(
       global->native_context()->script_context_table());
 
   Handle<JSObject> script_scope =
       isolate_->factory()->NewJSObjectWithNullProto();
 
   for (int context_index = 0; context_index < script_contexts->used();
        context_index++) {
     Handle<Context> context =
         ScriptContextTable::GetContext(script_contexts, context_index);
     Handle<ScopeInfo> scope_info(context->scope_info());
     CopyContextLocalsToScopeObject(scope_info, context, script_scope);
   }
   return script_scope;
 }
 
+void ScopeIterator::MaterializeStackLocals(Handle<JSObject> local_scope,
+                                           Handle<ScopeInfo> scope_info) {
+  if (frame_inspector_) {
+    return frame_inspector_->MaterializeStackLocals(local_scope, scope_info);
+  }
+
+  DCHECK(!generator_.is_null());
+  // Fill all stack locals.
+  Handle<FixedArray> register_file(generator_->register_file());
+  for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
+    Handle<String> name = handle(scope_info->StackLocalName(i));
+    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+    Handle<Object> value(register_file->get(scope_info->StackLocalIndex(i)),
+                         isolate_);
+    // 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->IsOptimizedOut(isolate_)) {

[neis, 2017/05/24 10:39:37]

Maybe add a TODO that we might need to convert StaleRegisterValue once we
optimize away the storing of unused stack locals at suspend.

+      value = isolate_->factory()->undefined_value();
+    }
+    JSObject::SetOwnPropertyIgnoreAttributes(local_scope, name, value, NONE)
+        .Check();
+  }
+}
 
 MaybeHandle<JSObject> ScopeIterator::MaterializeLocalScope() {
-  Handle<JSFunction> function = GetFunction();
+  Handle<JSFunction> function(GetFunction());
+  Handle<SharedFunctionInfo> shared(function->shared());
+  Handle<ScopeInfo> scope_info(shared->scope_info());
 
   Handle<JSObject> local_scope =
       isolate_->factory()->NewJSObjectWithNullProto();
-  frame_inspector_->MaterializeStackLocals(local_scope, function);
+  MaterializeStackLocals(local_scope, scope_info);
 
-  Handle<Context> frame_context =
-      Handle<Context>::cast(frame_inspector_->GetContext());
-
-  HandleScope scope(isolate_);
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
+  Handle<Context> frame_context = GetFrameContext();
 
   if (!scope_info->HasContext()) return local_scope;
 
   // Fill all context locals.
   Handle<Context> function_context(frame_context->closure_context());
   CopyContextLocalsToScopeObject(scope_info, function_context, local_scope);
 
   // Finally copy any properties from the function context extension.
   // These will be variables introduced by eval.
   if (function_context->closure() == *function &&
@@ skipping 61 matching lines @@
 
 // Create a plain JSObject which materializes the block scope for the specified
 // block context.
 Handle<JSObject> ScopeIterator::MaterializeInnerScope() {
   Handle<JSObject> inner_scope =
       isolate_->factory()->NewJSObjectWithNullProto();
 
   Handle<Context> context = Handle<Context>::null();
   if (!nested_scope_chain_.is_empty()) {
     Handle<ScopeInfo> scope_info = nested_scope_chain_.last().scope_info;
-    frame_inspector_->MaterializeStackLocals(inner_scope, scope_info);
+    MaterializeStackLocals(inner_scope, scope_info);
     if (scope_info->HasContext()) context = CurrentContext();
   } else {
     context = CurrentContext();
   }
 
   if (!context.is_null()) {
     // Fill all context locals.
     CopyContextLocalsToScopeObject(CurrentScopeInfo(), context, inner_scope);
     CopyContextExtensionToScopeObject(context, inner_scope,
                                       KeyCollectionMode::kOwnOnly);
   }
   return inner_scope;
 }
 
 
 // Create a plain JSObject which materializes the module scope for the specified
 // module context.
 MaybeHandle<JSObject> ScopeIterator::MaterializeModuleScope() {
   Handle<Context> context = CurrentContext();
   DCHECK(context->IsModuleContext());
   Handle<ScopeInfo> scope_info(context->scope_info());
   Handle<JSObject> module_scope =
       isolate_->factory()->NewJSObjectWithNullProto();
   CopyContextLocalsToScopeObject(scope_info, context, module_scope);
   CopyModuleVarsToScopeObject(scope_info, context, module_scope);
   return module_scope;
 }
 
 bool ScopeIterator::SetParameterValue(Handle<ScopeInfo> scope_info,
-                                      JavaScriptFrame* frame,
                                       Handle<String> parameter_name,
                                       Handle<Object> new_value) {
   // Setting stack locals of optimized frames is not supported.
-  if (frame->is_optimized()) return false;
   HandleScope scope(isolate_);
   for (int i = 0; i < scope_info->ParameterCount(); ++i) {
     if (String::Equals(handle(scope_info->ParameterName(i)), parameter_name)) {
-      frame->SetParameterValue(i, *new_value);
+      DCHECK_NOT_NULL(frame_inspector_);

[neis, 2017/05/24 10:39:37]

Please add a comment here explaining that it cannot be a generator because we
force context allocation for generator parameters.

[Jarin, 2017/05/24 12:32:21]

Done.

+      JavaScriptFrame* frame = GetFrame();
+      if (frame->is_optimized()) {
+        return false;
+      }
+      GetFrame()->SetParameterValue(i, *new_value);

[jgruber, 2017/05/24 08:01:48]

Nit: frame->SetParameterValue

[Jarin, 2017/05/24 12:32:21]

Done.

       return true;
     }
   }
   return false;
 }
 
 bool ScopeIterator::SetStackVariableValue(Handle<ScopeInfo> scope_info,
                                           Handle<String> variable_name,
                                           Handle<Object> new_value) {
-  if (frame_inspector_ == nullptr) return false;
-  JavaScriptFrame* frame = GetFrame();
   // Setting stack locals of optimized frames is not supported.

[neis, 2017/05/24 10:39:37]

Maybe add that it's supported for optimized suspended generators.

[Jarin, 2017/05/24 12:32:21]

Done.

-  if (frame->is_optimized()) return false;
   HandleScope scope(isolate_);
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     if (String::Equals(handle(scope_info->StackLocalName(i)), variable_name)) {
-      frame->SetExpression(scope_info->StackLocalIndex(i), *new_value);
+      int stack_local_index = scope_info->StackLocalIndex(i);
+      if (frame_inspector_ != nullptr) {
+        // Set the variable on the stack.
+        JavaScriptFrame* frame = GetFrame();
+        if (frame->is_optimized()) return false;
+        frame->SetExpression(scope_info->StackLocalIndex(i), *new_value);
+      } else {

[neis, 2017/05/24 10:39:37]

...SetExpression(stack_local_index, ...)

[Jarin, 2017/05/24 12:32:21]

Done.

+        // Set the variable in the suspended generator.
+        DCHECK(!generator_.is_null());
+        Handle<FixedArray> register_file(generator_->register_file());
+        DCHECK_LT(stack_local_index, register_file->length());
+        register_file->set(stack_local_index, *new_value);
+      }
       return true;
     }
   }
   return false;
 }
 
 bool ScopeIterator::SetContextVariableValue(Handle<ScopeInfo> scope_info,
                                             Handle<Context> context,
                                             Handle<String> variable_name,
                                             Handle<Object> new_value) {
@@ skipping 22 matching lines @@
           .Check();
       return true;
     }
   }
 
   return false;
 }
 
 bool ScopeIterator::SetLocalVariableValue(Handle<String> variable_name,
                                           Handle<Object> new_value) {
-  JavaScriptFrame* frame = GetFrame();
-  Handle<ScopeInfo> scope_info(frame->function()->shared()->scope_info());
+  Handle<ScopeInfo> scope_info(GetFunction()->shared()->scope_info());
 
   // Parameter might be shadowed in context. Don't stop here.
-  bool result = SetParameterValue(scope_info, frame, variable_name, new_value);
+  bool result = SetParameterValue(scope_info, variable_name, new_value);
 
   // Stack locals.
   if (SetStackVariableValue(scope_info, variable_name, new_value)) {
     return true;
   }
 
   if (scope_info->HasContext() &&
       SetContextVariableValue(scope_info, CurrentContext(), variable_name,
                               new_value)) {
     return true;
@@ skipping 131 matching lines @@
     JSObject::SetOwnPropertyIgnoreAttributes(scope_object, key, value, NONE)
         .Check();
   }
 }
 
 void ScopeIterator::GetNestedScopeChain(Isolate* isolate, Scope* scope,
                                         int position) {
   if (scope->is_function_scope()) {
     // Do not collect scopes of nested inner functions inside the current one.
     // Nested arrow functions could have the same end positions.
-    Handle<JSFunction> function = frame_inspector_->GetFunction();
+    Handle<JSFunction> function = GetFunction();
     if (scope->start_position() > function->shared()->start_position() &&
         scope->end_position() <= function->shared()->end_position()) {
       return;
     }
   }
   if (scope->is_hidden()) {
     // We need to add this chain element in case the scope has a context
     // associated. We need to keep the scope chain and context chain in sync.
     nested_scope_chain_.Add(ExtendedScopeInfo(scope->scope_info()));
   } else {
     nested_scope_chain_.Add(ExtendedScopeInfo(
         scope->scope_info(), scope->start_position(), scope->end_position()));
   }
   for (Scope* inner_scope = scope->inner_scope(); inner_scope != nullptr;
        inner_scope = inner_scope->sibling()) {
     int beg_pos = inner_scope->start_position();
     int end_pos = inner_scope->end_position();
     DCHECK((beg_pos >= 0 && end_pos >= 0) || inner_scope->is_hidden());
     if (beg_pos <= position && position < end_pos) {
       GetNestedScopeChain(isolate, inner_scope, position);
       return;
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8