Add a mode to completely deserialize scope chains

This will allow for the background parser to parse inner functions

BUG=v8:5215
R=marja@chromium.org,verwaest@chromium.org

Review-Url: https://codereview.chromium.org/2198043002
Cr-Commit-Position: refs/heads/master@{#38291}

src/DEPS

@@ -15,6 +15,7 @@ include_rules = [
   "+src/interpreter/bytecode-register.h",
   "+src/interpreter/bytecodes.h",
   "+src/interpreter/interpreter.h",
+  "+testing/gtest/include/gtest/gtest_prod.h",
   "-src/libplatform",
   "-include/libplatform"
 ]

src/ast/scopes.cc

@@ -4,6 +4,8 @@
 
 #include "src/ast/scopes.h"
 
+#include <set>
+
 #include "src/accessors.h"
 #include "src/bootstrapper.h"
 #include "src/messages.h"
@@ -204,7 +206,8 @@ void Scope::SetDefaults() {
 
 Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
                                     Context* context, Scope* script_scope,
-                                    AstValueFactory* ast_value_factory) {
+                                    AstValueFactory* ast_value_factory,
+                                    DeserializationMode deserialization_mode) {
   // Reconstruct the outer scope chain from a closure's context chain.
   Scope* current_scope = NULL;
   Scope* innermost_scope = NULL;
@@ -239,6 +242,9 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
           new (zone) Scope(zone, current_scope,
                            ast_value_factory->GetString(handle(name, isolate)));
     }
+    if (deserialization_mode == DeserializationMode::kDeserializeOffHeap) {
+      current_scope->DeserializeScopeInfo(isolate, ast_value_factory);
+    }
     if (innermost_scope == NULL) innermost_scope = current_scope;
     context = context->previous();
   }
@@ -248,6 +254,78 @@ Scope* Scope::DeserializeScopeChain(Isolate* isolate, Zone* zone,
   return (innermost_scope == NULL) ? script_scope : innermost_scope;
 }
 
+void Scope::DeserializeScopeInfo(Isolate* isolate,
+                                 AstValueFactory* ast_value_factory) {
+  if (scope_info_.is_null()) return;
+
+  DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
+
+  std::set<const AstRawString*> names_seen;
+  // Internalize context local & globals variables.
+  for (int var = 0; var < scope_info_->ContextLocalCount() +
+                              scope_info_->ContextGlobalCount();
+       ++var) {
+    Handle<String> name_handle(scope_info_->ContextLocalName(var), isolate);
+    const AstRawString* name = ast_value_factory->GetString(name_handle);
+    if (!names_seen.insert(name).second) continue;
+    int index = Context::MIN_CONTEXT_SLOTS + var;
+    VariableMode mode = scope_info_->ContextLocalMode(var);
+    InitializationFlag init_flag = scope_info_->ContextLocalInitFlag(var);
+    MaybeAssignedFlag maybe_assigned_flag =
+        scope_info_->ContextLocalMaybeAssignedFlag(var);
+    VariableLocation location = var < scope_info_->ContextLocalCount()
+                                    ? VariableLocation::CONTEXT
+                                    : VariableLocation::GLOBAL;
+    Variable::Kind kind = Variable::NORMAL;
+    if (index == scope_info_->ReceiverContextSlotIndex()) {
+      kind = Variable::THIS;
+    }
+
+    Variable* result = variables_.Declare(this, name, mode, kind, init_flag,
+                                          maybe_assigned_flag);
+    result->AllocateTo(location, index);
+  }
+
+  // We must read parameters from the end since for multiply declared
+  // parameters the value of the last declaration of that parameter is used
+  // inside a function (and thus we need to look at the last index). Was bug#
+  // 1110337.
+  for (int index = scope_info_->ParameterCount() - 1; index >= 0; --index) {
+    Handle<String> name_handle(scope_info_->ParameterName(index), isolate);
+    const AstRawString* name = ast_value_factory->GetString(name_handle);
+    if (!names_seen.insert(name).second) continue;
+
+    VariableMode mode = DYNAMIC;
+    InitializationFlag init_flag = kCreatedInitialized;
+    MaybeAssignedFlag maybe_assigned_flag = kMaybeAssigned;
+    VariableLocation location = VariableLocation::LOOKUP;
+    Variable::Kind kind = Variable::NORMAL;
+
+    Variable* result = variables_.Declare(this, name, mode, kind, init_flag,
+                                          maybe_assigned_flag);
+    result->AllocateTo(location, index);
+  }
+
+  // Internalize function proxy for this scope.
+  if (scope_info_->HasFunctionName()) {
+    AstNodeFactory factory(ast_value_factory);
+    Handle<String> name_handle(scope_info_->FunctionName(), isolate);
+    const AstRawString* name = ast_value_factory->GetString(name_handle);
+    VariableMode mode;
+    int index = scope_info_->FunctionContextSlotIndex(*name_handle, &mode);
+    if (index >= 0) {
+      Variable* result = new (zone())
+          Variable(this, name, mode, Variable::NORMAL, kCreatedInitialized);
+      VariableProxy* proxy = factory.NewVariableProxy(result);
+      VariableDeclaration* declaration =
+          factory.NewVariableDeclaration(proxy, mode, this, kNoSourcePosition);
+      DeclareFunctionVar(declaration);
+      result->AllocateTo(VariableLocation::CONTEXT, index);
+    }
+  }
+
+  scope_info_ = Handle<ScopeInfo>::null();
+}
 
 bool Scope::Analyze(ParseInfo* info) {
   DCHECK(info->literal() != NULL);

src/ast/scopes.h

@@ -114,9 +114,12 @@ class Scope: public ZoneObject {
   // doesn't re-allocate variables repeatedly.
   static bool Analyze(ParseInfo* info);
 
+  enum class DeserializationMode { kDeserializeOffHeap, kKeepScopeInfo };
+
   static Scope* DeserializeScopeChain(Isolate* isolate, Zone* zone,
                                       Context* context, Scope* script_scope,
-                                      AstValueFactory* ast_value_factory);
+                                      AstValueFactory* ast_value_factory,
+                                      DeserializationMode deserialization_mode);
 
 #ifdef DEBUG
   // The scope name is only used for printing/debugging.
@@ -853,6 +856,9 @@ class Scope: public ZoneObject {
 
   void SetDefaults();
 
+  void DeserializeScopeInfo(Isolate* isolate,
+                            AstValueFactory* ast_value_factory);
+
   PendingCompilationErrorHandler pending_error_handler_;
 };
 

src/background-parsing-task.cc

@@ -61,6 +61,9 @@ void BackgroundParsingTask::Run() {
   Isolate* isolate = source_->info->isolate();
   source_->info->set_isolate(nullptr);
 
+  source_->parser->DeserializeScopeChain(
+      source_->info.get(), Handle<Context>::null(),
+      Scope::DeserializationMode::kDeserializeOffHeap);
   source_->parser->ParseOnBackground(source_->info.get());
 
   if (script_data_ != nullptr) {

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

@@ -69,6 +69,9 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   parse_info_->set_hash_seed(isolate_->heap()->HashSeed());
   parse_info_->set_unicode_cache(unicode_cache_.get());
   parser_.reset(new Parser(parse_info_.get()));
+  parser_->DeserializeScopeChain(
+      parse_info_.get(), handle(function_->context(), isolate_),
+      Scope::DeserializationMode::kDeserializeOffHeap);
   status_ = CompileJobStatus::kReadyToParse;
 }
 

src/compiler-dispatcher/compiler-dispatcher-job.h

@@ -9,6 +9,7 @@
 
 #include "src/base/macros.h"
 #include "src/handles.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 
 namespace v8 {
 namespace internal {
@@ -59,6 +60,8 @@ class CompilerDispatcherJob {
   void ResetOnMainThread();
 
  private:
+  FRIEND_TEST(CompilerDispatcherJobTest, ScopeChain);
+
   void InternalizeParsingResult();
 
   CompileJobStatus status_ = CompileJobStatus::kInitial;

src/parsing/parser.cc

@@ -855,6 +855,31 @@ Parser::Parser(ParseInfo* info)
   }
 }
 
+void Parser::DeserializeScopeChain(
+    ParseInfo* info, Handle<Context> context,
+    Scope::DeserializationMode deserialization_mode) {
+  // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
+  // context, which will have the "this" binding for script scopes.
+  Scope* scope = NewScriptScope();
+  info->set_script_scope(scope);
+  if (!context.is_null() && !context->IsNativeContext()) {
+    scope =
+        Scope::DeserializeScopeChain(info->isolate(), zone(), *context, scope,
+                                     ast_value_factory(), deserialization_mode);
+    if (info->context().is_null()) {
+      DCHECK(deserialization_mode ==
+             Scope::DeserializationMode::kDeserializeOffHeap);
+    } else {
+      // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
+      // means the Parser cannot operate independent of the V8 heap. Tell the
+      // string table to internalize strings and values right after they're
+      // created. This kind of parsing can only be done in the main thread.
+      DCHECK(parsing_on_main_thread_);
+      ast_value_factory()->Internalize(info->isolate());
+    }
+  }
+  original_scope_ = scope;
+}
 
 FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
   // TODO(bmeurer): We temporarily need to pass allow_nesting = true here,
@@ -884,6 +909,9 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
     cached_parse_data_->Initialize();
   }
 
+  DeserializeScopeChain(info, info->context(),
+                        Scope::DeserializationMode::kKeepScopeInfo);
+
   source = String::Flatten(source);
   FunctionLiteral* result;
 
@@ -940,22 +968,8 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
 
   FunctionLiteral* result = NULL;
   {
-    // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
-    // context, which will have the "this" binding for script scopes.
-    Scope* scope = NewScriptScope();
-    info->set_script_scope(scope);
-    if (!info->context().is_null() && !info->context()->IsNativeContext()) {
-      scope = Scope::DeserializeScopeChain(info->isolate(), zone(),
-                                           *info->context(), scope,
-                                           ast_value_factory());
-      // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
-      // means the Parser cannot operate independent of the V8 heap. Tell the
-      // string table to internalize strings and values right after they're
-      // created. This kind of parsing can only be done in the main thread.
-      DCHECK(parsing_on_main_thread_);
-      ast_value_factory()->Internalize(info->isolate());
-    }
-    original_scope_ = scope;
+    Scope* scope = original_scope_;
+    DCHECK(scope);
     if (info->is_eval()) {
       if (!scope->is_script_scope() || is_strict(info->language_mode())) {
         parsing_mode = PARSE_EAGERLY;
@@ -1046,6 +1060,8 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info) {
     timer.Start();
   }
   Handle<SharedFunctionInfo> shared_info = info->shared_info();
+  DeserializeScopeChain(info, info->context(),
+                        Scope::DeserializationMode::kKeepScopeInfo);
 
   // Initialize parser state.
   source = String::Flatten(source);
@@ -1108,16 +1124,8 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
 
   {
     // Parse the function literal.
-    Scope* scope = NewScriptScope();
-    info->set_script_scope(scope);
-    if (!info->context().is_null()) {
-      // Ok to use Isolate here, since lazy function parsing is only done in the
-      // main thread.
-      DCHECK(parsing_on_main_thread_);
-      scope = Scope::DeserializeScopeChain(isolate, zone(), *info->context(),
-                                           scope, ast_value_factory());
-    }
-    original_scope_ = scope;
+    Scope* scope = original_scope_;
+    DCHECK(scope);
     FunctionState function_state(&function_state_, &scope_state_, scope,
                                  shared_info->kind());
     DCHECK(is_sloppy(scope->language_mode()) ||
@@ -5470,6 +5478,8 @@ void Parser::ParseOnBackground(ParseInfo* info) {
   scanner_.Initialize(stream_ptr);
   DCHECK(info->context().is_null() || info->context()->IsNativeContext());
 
+  DCHECK(original_scope_);
+
   // When streaming, we don't know the length of the source until we have parsed
   // it. The raw data can be UTF-8, so we wouldn't know the source length until
   // we have decoded it anyway even if we knew the raw data length (which we

src/parsing/parser.h

@@ -717,6 +717,9 @@ class Parser : public ParserBase<ParserTraits> {
   bool Parse(ParseInfo* info);
   void ParseOnBackground(ParseInfo* info);
 
+  void DeserializeScopeChain(ParseInfo* info, Handle<Context> context,
+                             Scope::DeserializationMode deserialization_mode);
+
   // Handle errors detected during parsing, move statistics to Isolate,
   // internalize strings (move them to the heap).
   void Internalize(Isolate* isolate, Handle<Script> script, bool error);

test/cctest/test-parsing.cc

@@ -3393,8 +3393,9 @@ TEST(SerializationOfMaybeAssignmentFlag) {
   CHECK(str->IsInternalizedString());
   i::Scope* script_scope =
       new (&zone) i::Scope(&zone, nullptr, i::SCRIPT_SCOPE);
-  i::Scope* s = i::Scope::DeserializeScopeChain(isolate, &zone, context,
-                                                script_scope, &avf);
+  i::Scope* s = i::Scope::DeserializeScopeChain(
+      isolate, &zone, context, script_scope, &avf,
+      i::Scope::DeserializationMode::kKeepScopeInfo);
   CHECK(s != script_scope);
   CHECK(name != NULL);
 
@@ -3440,8 +3441,9 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
 
   i::Scope* script_scope =
       new (&zone) i::Scope(&zone, nullptr, i::SCRIPT_SCOPE);
-  i::Scope* s = i::Scope::DeserializeScopeChain(isolate, &zone, context,
-                                                script_scope, &avf);
+  i::Scope* s = i::Scope::DeserializeScopeChain(
+      isolate, &zone, context, script_scope, &avf,
+      i::Scope::DeserializationMode::kKeepScopeInfo);
   CHECK(s != script_scope);
   const i::AstRawString* name_x = avf.GetOneByteString("x");
 

test/unittests/compiler-dispatcher/compiler-dispatcher-job-unittest.cc

@@ -17,6 +17,8 @@ typedef TestWithContext CompilerDispatcherJobTest;
 
 namespace {
 
+const char test_script[] = "x*x";
+
 class ScriptResource : public v8::String::ExternalOneByteStringResource {
  public:
   ScriptResource(const char* data, size_t length)
@@ -42,14 +44,14 @@ Handle<JSFunction> CreateFunction(
                  ->NewExternalStringFromOneByte(maybe_resource)
                  .ToHandleChecked();
   } else {
-    source = isolate->factory()->NewStringFromStaticChars("source");
+    source = isolate->factory()->NewStringFromAsciiChecked(test_script);
   }
   Handle<Script> script = isolate->factory()->NewScript(source);
   Handle<SharedFunctionInfo> shared = isolate->factory()->NewSharedFunctionInfo(
-      isolate->factory()->NewStringFromStaticChars("f"), MaybeHandle<Code>(),
+      isolate->factory()->NewStringFromAsciiChecked("f"), MaybeHandle<Code>(),
       false);
   SharedFunctionInfo::SetScript(shared, script);
-  shared->set_end_position(source->length() - 1);
+  shared->set_end_position(source->length());
   Handle<JSFunction> function =
       isolate->factory()->NewFunctionFromSharedFunctionInfo(
           shared, handle(isolate->context(), isolate));
@@ -70,7 +72,7 @@ TEST_F(CompilerDispatcherJobTest, CanParseOnBackgroundThread) {
     ASSERT_FALSE(job->can_parse_on_background_thread());
   }
   {
-    ScriptResource script("script", strlen("script"));
+    ScriptResource script(test_script, strlen(test_script));
     std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
         i_isolate(), CreateFunction(i_isolate(), &script), FLAG_stack_size));
     ASSERT_TRUE(job->can_parse_on_background_thread());

tools/gcmole/run-gcmole.isolate

@@ -16,6 +16,7 @@
       '../../include/',
       '../../src/',
       '../../test/cctest/',
+      '../../testing/gtest/include/gtest/gtest_prod.h',
       '../../third_party/icu/source/',
     ],
   },