[parser] Remove ParseInfo flag setting after script creation

Previously, ParseInfo would create a script (with CreateScript) based on
its flags, and then set its own flags based on that created script. This
created a weird circular dependency for some of those flags, and
sometimes we would have valid flags before script creation (main thread
compile), while other times not (streaming compile).

Now we set the ParseInfo flags manually and uniformly before script
creation, and check that they match the created script after it has been
created.

Bug: v8:10314
Change-Id: Ife886c77727cd228c944a4f97369a3e6365d8219
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2093433
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66648}

src/codegen/compiler.cc

@@ -720,6 +720,7 @@ bool FinalizeUnoptimizedCode(
   }
 
   Handle<Script> script(Script::cast(shared_info->script()), isolate);
+  parse_info->CheckFlagsForFunctionFromScript(*script);
 
   // Finalize the inner functions' compilation jobs.
   for (auto&& inner_job : *inner_function_jobs) {
@@ -1149,14 +1150,9 @@ BackgroundCompileTask::BackgroundCompileTask(ScriptStreamingData* streamed_data,
   // will happen in the main thread after parsing.
   LOG(isolate, ScriptEvent(Logger::ScriptEventType::kStreamingCompile,
                            info_->script_id()));
-  info_->set_toplevel();
-  info_->set_allow_lazy_parsing();
-  if (V8_UNLIKELY(info_->block_coverage_enabled())) {
-    info_->AllocateSourceRangeMap();
-  }
-  LanguageMode language_mode = construct_language_mode(FLAG_use_strict);
-  info_->set_language_mode(
-      stricter_language_mode(info_->language_mode(), language_mode));
+  info_->SetFlagsForToplevelCompile(isolate->is_collecting_type_profile(), true,
+                                    construct_language_mode(FLAG_use_strict),
+                                    REPLMode::kNo);
   language_mode_ = info_->language_mode();
 
   std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(
@@ -1277,10 +1273,10 @@ void BackgroundCompileTask::Run() {
       // We don't have the script source or the script origin yet, so use a few
       // default values for them. These will be fixed up during the main-thread
       // merge.
-      Handle<Script> script = info_->CreateScript(
-          off_thread_isolate_.get(),
-          off_thread_isolate_->factory()->empty_string(), ScriptOriginOptions(),
-          REPLMode::kNo, NOT_NATIVES_CODE);
+      Handle<Script> script =
+          info_->CreateScript(off_thread_isolate_.get(),
+                              off_thread_isolate_->factory()->empty_string(),
+                              ScriptOriginOptions(), NOT_NATIVES_CODE);
 
       Handle<SharedFunctionInfo> outer_function_sfi =
           FinalizeTopLevel(info_.get(), script, off_thread_isolate_.get(),
@@ -1594,9 +1590,7 @@ bool Compiler::FinalizeBackgroundCompileTask(
   DCHECK(!shared_info->is_compiled());
 
   Handle<Script> script(Script::cast(shared_info->script()), isolate);
-  // TODO(leszeks): We can probably remove this, the parse_info flags should
-  // already match the script's.
-  parse_info->SetFlagsForFunctionFromScript(*script);
+  parse_info->CheckFlagsForFunctionFromScript(*script);
 
   task->parser()->UpdateStatistics(isolate, script);
   task->parser()->HandleSourceURLComments(isolate, script);
@@ -1703,9 +1697,19 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     allow_eval_cache = true;
   } else {
     ParseInfo parse_info(isolate);
+    parse_info.SetFlagsForToplevelCompile(isolate->is_collecting_type_profile(),
+                                          true, language_mode, REPLMode::kNo);
+
+    parse_info.set_eval();
+    parse_info.set_parse_restriction(restriction);
+    parse_info.set_parameters_end_pos(parameters_end_pos);
+    if (!context->IsNativeContext()) {
+      parse_info.set_outer_scope_info(handle(context->scope_info(), isolate));
+    }
+    DCHECK(!parse_info.is_module());
+
     script = parse_info.CreateScript(
         isolate, source, OriginOptionsForEval(outer_info->script()));
-    script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
     script->set_eval_from_shared(*outer_info);
     if (eval_position == kNoSourcePosition) {
       // If the position is missing, attempt to get the code offset by
@@ -1724,15 +1728,6 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     }
     script->set_eval_from_position(eval_position);
 
-    parse_info.set_eval();
-    parse_info.set_language_mode(language_mode);
-    parse_info.set_parse_restriction(restriction);
-    parse_info.set_parameters_end_pos(parameters_end_pos);
-    if (!context->IsNativeContext()) {
-      parse_info.set_outer_scope_info(handle(context->scope_info(), isolate));
-    }
-    DCHECK(!parse_info.is_module());
-
     if (!CompileToplevel(&parse_info, script, isolate, &is_compiled_scope)
              .ToHandle(&shared_info)) {
       return MaybeHandle<JSFunction>();
@@ -2143,8 +2138,8 @@ Handle<Script> NewScript(Isolate* isolate, ParseInfo* parse_info,
                          ScriptOriginOptions origin_options,
                          NativesFlag natives) {
   // Create a script object describing the script to be compiled.
-  Handle<Script> script = parse_info->CreateScript(
-      isolate, source, origin_options, script_details.repl_mode, natives);
+  Handle<Script> script =
+      parse_info->CreateScript(isolate, source, origin_options, natives);
   SetScriptFieldsFromDetails(*script, script_details);
   LOG(isolate, ScriptDetails(*script));
   return script;
@@ -2234,19 +2229,24 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
   }
 
   if (maybe_result.is_null()) {
-    ParseInfo parse_info(isolate);
     // No cache entry found compile the script.
+    ParseInfo parse_info(isolate);
+
+    parse_info.SetFlagsForToplevelCompile(
+        isolate->is_collecting_type_profile(), natives == NOT_NATIVES_CODE,
+        language_mode, script_details.repl_mode);
+
+    parse_info.set_module(origin_options.IsModule());
+    parse_info.set_extension(extension);
+    parse_info.set_eager(compile_options == ScriptCompiler::kEagerCompile);
+
     Handle<Script> script = NewScript(isolate, &parse_info, source,
                                       script_details, origin_options, natives);
+    DCHECK_IMPLIES(parse_info.collect_type_profile(),
+                   script->IsUserJavaScript());
     DCHECK_EQ(parse_info.is_repl_mode(), script->is_repl_mode());
 
     // Compile the function and add it to the isolate cache.
-    if (origin_options.IsModule()) parse_info.set_module();
-    parse_info.set_extension(extension);
-    parse_info.set_eager(compile_options == ScriptCompiler::kEagerCompile);
-
-    parse_info.set_language_mode(
-        stricter_language_mode(parse_info.language_mode(), language_mode));
     maybe_result =
         CompileToplevel(&parse_info, script, isolate, &is_compiled_scope);
     Handle<SharedFunctionInfo> result;
@@ -2308,9 +2308,9 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
   IsCompiledScope is_compiled_scope;
   if (!maybe_result.ToHandle(&wrapped)) {
     ParseInfo parse_info(isolate);
-    script = NewScript(isolate, &parse_info, source, script_details,
-                       origin_options, NOT_NATIVES_CODE);
-    script->set_wrapped_arguments(*arguments);
+    parse_info.SetFlagsForToplevelCompile(isolate->is_collecting_type_profile(),
+                                          true, language_mode,
+                                          script_details.repl_mode);
 
     parse_info.set_eval();  // Use an eval scope as declaration scope.
     parse_info.set_function_syntax_kind(FunctionSyntaxKind::kWrapped);
@@ -2323,8 +2323,9 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
     if (!context->IsNativeContext()) {
       parse_info.set_outer_scope_info(handle(context->scope_info(), isolate));
     }
-    parse_info.set_language_mode(
-        stricter_language_mode(parse_info.language_mode(), language_mode));
+
+    script = NewScript(isolate, &parse_info, source, script_details,
+                       origin_options, NOT_NATIVES_CODE);
 
     Handle<SharedFunctionInfo> top_level;
     maybe_result =

src/common/globals.h

@@ -828,6 +828,10 @@ enum class REPLMode {
   kNo,
 };
 
+inline REPLMode construct_repl_mode(bool is_repl_mode) {
+  return is_repl_mode ? REPLMode::kYes : REPLMode::kNo;
+}
+
 // Flag indicating whether code is built into the VM (one of the natives files).
 enum NativesFlag { NOT_NATIVES_CODE, EXTENSION_CODE, INSPECTOR_CODE };
 

src/parsing/parse-info.cc

@@ -7,6 +7,7 @@
 #include "src/ast/ast-source-ranges.h"
 #include "src/ast/ast-value-factory.h"
 #include "src/ast/ast.h"
+#include "src/common/globals.h"
 #include "src/compiler-dispatcher/compiler-dispatcher.h"
 #include "src/heap/heap-inl.h"
 #include "src/logging/counters.h"
@@ -175,7 +176,6 @@ template <typename LocalIsolate>
 Handle<Script> ParseInfo::CreateScript(LocalIsolate* isolate,
                                        Handle<String> source,
                                        ScriptOriginOptions origin_options,
-                                       REPLMode repl_mode,
                                        NativesFlag natives) {
   // Create a script object describing the script to be compiled.
   Handle<Script> script;
@@ -198,10 +198,15 @@ Handle<Script> ParseInfo::CreateScript(LocalIsolate* isolate,
       break;
   }
   script->set_origin_options(origin_options);
-  script->set_is_repl_mode(repl_mode == REPLMode::kYes);
+  script->set_is_repl_mode(is_repl_mode());
+  if (is_wrapped_as_function()) {
+    script->set_wrapped_arguments(*wrapped_arguments());
+  } else if (is_eval()) {
+    script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
+  }
 
-  SetFlagsForToplevelCompileFromScript(isolate, *script,
-                                       isolate->is_collecting_type_profile());
+  CheckFlagsForToplevelCompileFromScript(*script,
+                                         isolate->is_collecting_type_profile());
   return script;
 }
 
@@ -209,13 +214,11 @@ template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE)
     Handle<Script> ParseInfo::CreateScript(Isolate* isolate,
                                            Handle<String> source,
                                            ScriptOriginOptions origin_options,
-                                           REPLMode repl_mode,
                                            NativesFlag natives);
 template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE)
     Handle<Script> ParseInfo::CreateScript(OffThreadIsolate* isolate,
                                            Handle<String> source,
                                            ScriptOriginOptions origin_options,
-                                           REPLMode repl_mode,
                                            NativesFlag natives);
 
 AstValueFactory* ParseInfo::GetOrCreateAstValueFactory() {
@@ -228,6 +231,7 @@ AstValueFactory* ParseInfo::GetOrCreateAstValueFactory() {
 
 void ParseInfo::AllocateSourceRangeMap() {
   DCHECK(block_coverage_enabled());
+  DCHECK_NULL(source_range_map());
   set_source_range_map(new (zone()) SourceRangeMap(zone()));
 }
 
@@ -239,15 +243,29 @@ void ParseInfo::set_character_stream(
   character_stream_.swap(character_stream);
 }
 
+void ParseInfo::SetFlagsForToplevelCompile(bool is_collecting_type_profile,
+                                           bool is_user_javascript,
+                                           LanguageMode language_mode,
+                                           REPLMode repl_mode) {
+  set_allow_lazy_parsing();
+  set_toplevel();
+  set_collect_type_profile(is_user_javascript && is_collecting_type_profile);
+  set_language_mode(
+      stricter_language_mode(this->language_mode(), language_mode));
+  set_repl_mode(repl_mode == REPLMode::kYes);
+
+  if (V8_UNLIKELY(is_user_javascript && block_coverage_enabled())) {
+    AllocateSourceRangeMap();
+  }
+}
+
 template <typename LocalIsolate>
 void ParseInfo::SetFlagsForToplevelCompileFromScript(
     LocalIsolate* isolate, Script script, bool is_collecting_type_profile) {
   SetFlagsForFunctionFromScript(script);
-  set_allow_lazy_parsing();
-  set_toplevel();
-  set_collect_type_profile(is_collecting_type_profile &&
-                           script.IsUserJavaScript());
-  set_repl_mode(script.is_repl_mode());
+  SetFlagsForToplevelCompile(is_collecting_type_profile,
+                             script.IsUserJavaScript(), language_mode(),
+                             construct_repl_mode(script.is_repl_mode()));
 
   if (script.is_wrapped()) {
     set_function_syntax_kind(FunctionSyntaxKind::kWrapped);
@@ -255,6 +273,21 @@ void ParseInfo::SetFlagsForToplevelCompileFromScript(
   }
 }
 
+void ParseInfo::CheckFlagsForToplevelCompileFromScript(
+    Script script, bool is_collecting_type_profile) {
+  CheckFlagsForFunctionFromScript(script);
+  DCHECK(allow_lazy_parsing());
+  DCHECK(is_toplevel());
+  DCHECK_EQ(collect_type_profile(),
+            is_collecting_type_profile && script.IsUserJavaScript());
+  DCHECK_EQ(is_repl_mode(), script.is_repl_mode());
+
+  if (script.is_wrapped()) {
+    DCHECK_EQ(function_syntax_kind(), FunctionSyntaxKind::kWrapped);
+    DCHECK_EQ(*wrapped_arguments(), script.wrapped_arguments());
+  }
+}
+
 void ParseInfo::SetFlagsForFunctionFromScript(Script script) {
   DCHECK(script_id_ == -1 || script_id_ == script.id());
   script_id_ = script.id();
@@ -268,6 +301,17 @@ void ParseInfo::SetFlagsForFunctionFromScript(Script script) {
   }
 }
 
+void ParseInfo::CheckFlagsForFunctionFromScript(Script script) {
+  DCHECK_EQ(script_id_, script.id());
+  // We set "is_eval" for wrapped functions to get an outer declaration scope.
+  // This is a bit hacky, but ok since we can't be both eval and wrapped.
+  DCHECK_EQ(is_eval() && !is_wrapped_as_function(),
+            script.compilation_type() == Script::COMPILATION_TYPE_EVAL);
+  DCHECK_EQ(is_module(), script.origin_options().IsModule());
+  DCHECK_IMPLIES(block_coverage_enabled() && script.IsUserJavaScript(),
+                 source_range_map() != nullptr);
+}
+
 void ParseInfo::ParallelTasks::Enqueue(ParseInfo* outer_parse_info,
                                        const AstRawString* function_name,
                                        FunctionLiteral* literal) {

src/parsing/parse-info.h

@@ -59,7 +59,6 @@ class V8_EXPORT_PRIVATE ParseInfo {
   EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
   Handle<Script> CreateScript(LocalIsolate* isolate, Handle<String> source,
                               ScriptOriginOptions origin_options,
-                              REPLMode repl_mode = REPLMode::kNo,
                               NativesFlag natives = NOT_NATIVES_CODE);
 
   // Either returns the ast-value-factory associcated with this ParseInfo, or
@@ -264,7 +263,12 @@ class V8_EXPORT_PRIVATE ParseInfo {
 
   ParallelTasks* parallel_tasks() { return parallel_tasks_.get(); }
 
-  void SetFlagsForFunctionFromScript(Script script);
+  void SetFlagsForToplevelCompile(bool is_collecting_type_profile,
+                                  bool is_user_javascript,
+                                  LanguageMode language_mode,
+                                  REPLMode repl_mode);
+
+  void CheckFlagsForFunctionFromScript(Script script);
 
   //--------------------------------------------------------------------------
   // TODO(titzer): these should not be part of ParseInfo.
@@ -293,10 +297,14 @@ class V8_EXPORT_PRIVATE ParseInfo {
   }
 
  private:
+  void SetFlagsForFunctionFromScript(Script script);
+
   template <typename LocalIsolate>
   void SetFlagsForToplevelCompileFromScript(LocalIsolate* isolate,
                                             Script script,
                                             bool is_collecting_type_profile);
+  void CheckFlagsForToplevelCompileFromScript(Script script,
+                                              bool is_collecting_type_profile);
 
   // Set function info flags based on those in either FunctionLiteral or
   // SharedFunctionInfo |function|