[Deflake] Fix data-race relating to accessing FLAG_* on worker thread during background compile

The parser object can now be created on a worker thread, therefore we shouldn't access
global FLAGs during the constructor. Instead move them to the ParseInfo constructor
and set the parser fields based on these. Also avoid accessing always_opt flags in
bytecode-flags - instead accessing it in ParseInfo and propagating to the bytecode
generator.

Also gets rid of unused kUntrustedCodeMitigations flag in UnoptimizedCompilationInfo

BUG=v8:8582

Change-Id: I6e6fdc8cc7865803cb5f334f652abc0e3e4cb3ce
Reviewed-on: https://chromium-review.googlesource.com/c/1375918Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58316}

src/interpreter/bytecode-flags.cc

@@ -30,10 +30,10 @@ uint8_t CreateObjectLiteralFlags::Encode(int runtime_flags,
 }
 
 // static
-uint8_t CreateClosureFlags::Encode(bool pretenure, bool is_function_scope) {
+uint8_t CreateClosureFlags::Encode(bool pretenure, bool is_function_scope,
+                                   bool might_always_opt) {
   uint8_t result = PretenuredBit::encode(pretenure);
-  if (!FLAG_always_opt && !FLAG_prepare_always_opt &&
-      pretenure == NOT_TENURED && is_function_scope) {
+  if (!might_always_opt && pretenure == NOT_TENURED && is_function_scope) {
     result |= FastNewClosureBit::encode(true);
   }
   return result;

src/interpreter/bytecode-flags.h

@@ -43,7 +43,8 @@ class CreateClosureFlags {
   class PretenuredBit : public BitField8<bool, 0, 1> {};
   class FastNewClosureBit : public BitField8<bool, PretenuredBit::kNext, 1> {};
 
-  static uint8_t Encode(bool pretenure, bool is_function_scope);
+  static uint8_t Encode(bool pretenure, bool is_function_scope,
+                        bool might_always_opt);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(CreateClosureFlags);

src/interpreter/bytecode-generator.cc

@@ -1806,7 +1806,8 @@ void BytecodeGenerator::VisitDebuggerStatement(DebuggerStatement* stmt) {
 void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
   DCHECK(expr->scope()->outer_scope() == current_scope());
   uint8_t flags = CreateClosureFlags::Encode(
-      expr->pretenure(), closure_scope()->is_function_scope());
+      expr->pretenure(), closure_scope()->is_function_scope(),
+      info()->might_always_opt());
   size_t entry = builder()->AllocateDeferredConstantPoolEntry();
   FeedbackSlot slot = GetCachedCreateClosureSlot(expr);
   builder()->CreateClosure(entry, feedback_index(slot), flags);

src/parsing/parse-info.cc

@@ -52,6 +52,16 @@ ParseInfo::ParseInfo(Isolate* isolate, AccountingAllocator* zone_allocator)
   if (isolate->compiler_dispatcher()->IsEnabled()) {
     parallel_tasks_.reset(new ParallelTasks(isolate->compiler_dispatcher()));
   }
+  set_might_always_opt(FLAG_always_opt || FLAG_prepare_always_opt);
+  set_allow_lazy_compile(FLAG_lazy);
+  set_allow_natives_syntax(FLAG_allow_natives_syntax);
+  set_allow_harmony_public_fields(FLAG_harmony_public_fields);
+  set_allow_harmony_static_fields(FLAG_harmony_static_fields);
+  set_allow_harmony_dynamic_import(FLAG_harmony_dynamic_import);
+  set_allow_harmony_import_meta(FLAG_harmony_import_meta);
+  set_allow_harmony_numeric_separator(FLAG_harmony_numeric_separator);
+  set_allow_harmony_private_fields(FLAG_harmony_private_fields);
+  set_allow_harmony_private_methods(FLAG_harmony_private_methods);
 }
 
 ParseInfo::ParseInfo(Isolate* isolate)

src/parsing/parse-info.h

@@ -94,6 +94,26 @@ class V8_EXPORT_PRIVATE ParseInfo {
   FLAG_ACCESSOR(kRequiresInstanceMembersInitializer,
                 requires_instance_members_initializer,
                 set_requires_instance_members_initializer);
+  FLAG_ACCESSOR(kMightAlwaysOpt, might_always_opt, set_might_always_opt)
+  FLAG_ACCESSOR(kAllowNativeSyntax, allow_natives_syntax,
+                set_allow_natives_syntax)
+  FLAG_ACCESSOR(kAllowLazyCompile, allow_lazy_compile, set_allow_lazy_compile)
+  FLAG_ACCESSOR(kAllowNativeSyntax, allow_native_syntax,
+                set_allow_native_syntax);
+  FLAG_ACCESSOR(kAllowHarmonyPublicFields, allow_harmony_public_fields,
+                set_allow_harmony_public_fields);
+  FLAG_ACCESSOR(kAllowHarmonyStaticFields, allow_harmony_static_fields,
+                set_allow_harmony_static_fields);
+  FLAG_ACCESSOR(kAllowHarmonyDynamicImport, allow_harmony_dynamic_import,
+                set_allow_harmony_dynamic_import);
+  FLAG_ACCESSOR(kAllowHarmonyImportMeta, allow_harmony_import_meta,
+                set_allow_harmony_import_meta);
+  FLAG_ACCESSOR(kAllowHarmonyNumericSeparator, allow_harmony_numeric_separator,
+                set_allow_harmony_numeric_separator);
+  FLAG_ACCESSOR(kAllowHarmonyPrivateFields, allow_harmony_private_fields,
+                set_allow_harmony_private_fields);
+  FLAG_ACCESSOR(kAllowHarmonyPrivateMethods, allow_harmony_private_methods,
+                set_allow_harmony_private_methods);
 #undef FLAG_ACCESSOR
 
   void set_parse_restriction(ParseRestriction restriction) {
@@ -281,6 +301,16 @@ class V8_EXPORT_PRIVATE ParseInfo {
     kIsDeclaration = 1 << 16,
     kRequiresInstanceMembersInitializer = 1 << 17,
     kContainsAsmModule = 1 << 18,
+    kMightAlwaysOpt = 1 << 19,
+    kAllowLazyCompile = 1 << 20,
+    kAllowNativeSyntax = 1 << 21,
+    kAllowHarmonyPublicFields = 1 << 22,
+    kAllowHarmonyStaticFields = 1 << 23,
+    kAllowHarmonyDynamicImport = 1 << 24,
+    kAllowHarmonyImportMeta = 1 << 25,
+    kAllowHarmonyNumericSeparator = 1 << 26,
+    kAllowHarmonyPrivateFields = 1 << 27,
+    kAllowHarmonyPrivateMethods = 1 << 28
   };
 
   //------------- Inputs to parsing and scope analysis -----------------------

src/parsing/parser.cc

@@ -407,21 +407,22 @@ Parser::Parser(ParseInfo* info)
   // of functions without an outer context when setting a breakpoint through
   // Debug::FindSharedFunctionInfoInScript
   // We also compile eagerly for kProduceExhaustiveCodeCache.
-  bool can_compile_lazily = FLAG_lazy && !info->is_eager();
+  bool can_compile_lazily = info->allow_lazy_compile() && !info->is_eager();
 
   set_default_eager_compile_hint(can_compile_lazily
                                      ? FunctionLiteral::kShouldLazyCompile
                                      : FunctionLiteral::kShouldEagerCompile);
-  allow_lazy_ = FLAG_lazy && info->allow_lazy_parsing() && !info->is_native() &&
-                info->extension() == nullptr && can_compile_lazily;
-  set_allow_natives(FLAG_allow_natives_syntax || info->is_native());
-  set_allow_harmony_public_fields(FLAG_harmony_public_fields);
-  set_allow_harmony_static_fields(FLAG_harmony_static_fields);
-  set_allow_harmony_dynamic_import(FLAG_harmony_dynamic_import);
-  set_allow_harmony_import_meta(FLAG_harmony_import_meta);
-  set_allow_harmony_numeric_separator(FLAG_harmony_numeric_separator);
-  set_allow_harmony_private_fields(FLAG_harmony_private_fields);
-  set_allow_harmony_private_methods(FLAG_harmony_private_methods);
+  allow_lazy_ = info->allow_lazy_compile() && info->allow_lazy_parsing() &&
+                !info->is_native() && info->extension() == nullptr &&
+                can_compile_lazily;
+  set_allow_natives(info->allow_natives_syntax() || info->is_native());
+  set_allow_harmony_public_fields(info->allow_harmony_public_fields());
+  set_allow_harmony_static_fields(info->allow_harmony_static_fields());
+  set_allow_harmony_dynamic_import(info->allow_harmony_dynamic_import());
+  set_allow_harmony_import_meta(info->allow_harmony_import_meta());
+  set_allow_harmony_numeric_separator(info->allow_harmony_numeric_separator());
+  set_allow_harmony_private_fields(info->allow_harmony_private_fields());
+  set_allow_harmony_private_methods(info->allow_harmony_private_methods());
   for (int feature = 0; feature < v8::Isolate::kUseCounterFeatureCount;
        ++feature) {
     use_counts_[feature] = 0;
@@ -2503,7 +2504,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   // parenthesis before the function means that it will be called
   // immediately). bar can be parsed lazily, but we need to parse it in a mode
   // that tracks unresolved variables.
-  DCHECK_IMPLIES(parse_lazily(), FLAG_lazy);
+  DCHECK_IMPLIES(parse_lazily(), info()->allow_lazy_compile());
   DCHECK_IMPLIES(parse_lazily(), has_error() || allow_lazy_);
   DCHECK_IMPLIES(parse_lazily(), extension_ == nullptr);
 

src/unoptimized-compilation-info.cc

@@ -18,9 +18,7 @@ namespace internal {
 UnoptimizedCompilationInfo::UnoptimizedCompilationInfo(Zone* zone,
                                                        ParseInfo* parse_info,
                                                        FunctionLiteral* literal)
-    : flags_(FLAG_untrusted_code_mitigations ? kUntrustedCodeMitigations : 0),
-      zone_(zone),
-      feedback_vector_spec_(zone) {
+    : flags_(0), zone_(zone), feedback_vector_spec_(zone) {
   // NOTE: The parse_info passed here represents the global information gathered
   // during parsing, but does not represent specific details of the actual
   // function literal being compiled for this OptimizedCompilationInfo. As such,
@@ -34,6 +32,7 @@ UnoptimizedCompilationInfo::UnoptimizedCompilationInfo(Zone* zone,
   if (parse_info->is_eval()) MarkAsEval();
   if (parse_info->is_native()) MarkAsNative();
   if (parse_info->collect_type_profile()) MarkAsCollectTypeProfile();
+  if (parse_info->might_always_opt()) MarkAsMightAlwaysOpt();
 }
 
 DeclarationScope* UnoptimizedCompilationInfo::scope() const {

src/unoptimized-compilation-info.h

@@ -46,6 +46,9 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
   void MarkAsCollectTypeProfile() { SetFlag(kCollectTypeProfile); }
   bool collect_type_profile() const { return GetFlag(kCollectTypeProfile); }
 
+  void MarkAsMightAlwaysOpt() { SetFlag(kMightAlwaysOpt); }
+  bool might_always_opt() const { return GetFlag(kMightAlwaysOpt); }
+
   // Accessors for the input data of the function being compiled.
 
   FunctionLiteral* literal() const { return literal_; }
@@ -98,7 +101,7 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
     kIsEval = 1 << 0,
     kIsNative = 1 << 1,
     kCollectTypeProfile = 1 << 2,
-    kUntrustedCodeMitigations = 1 << 3,
+    kMightAlwaysOpt = 1 << 3,
   };
 
   void SetFlag(Flag flag) { flags_ |= flag; }

test/cctest/test-parsing.cc

@@ -1613,10 +1613,10 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
   // Parse the data
   i::FunctionLiteral* function;
   {
+    SetGlobalFlags(flags);
     i::Handle<i::Script> script = factory->NewScript(source);
     i::ParseInfo info(isolate, script);
     info.set_allow_lazy_parsing(flags.Contains(kAllowLazy));
-    SetGlobalFlags(flags);
     if (is_module) info.set_module();
     i::parsing::ParseProgram(&info, isolate);
     function = info.literal();
@@ -11050,6 +11050,7 @@ TEST(PrivateNamesSyntaxError) {
   LocalContext env;
 
   auto test = [isolate](const char* program, bool is_lazy) {
+    i::FLAG_harmony_private_fields = true;
     i::Factory* const factory = isolate->factory();
     i::Handle<i::String> source =
         factory->NewStringFromUtf8(i::CStrVector(program)).ToHandleChecked();
@@ -11057,7 +11058,6 @@ TEST(PrivateNamesSyntaxError) {
     i::ParseInfo info(isolate, script);
 
     info.set_allow_lazy_parsing(is_lazy);
-    i::FLAG_harmony_private_fields = true;
     CHECK(i::parsing::ParseProgram(&info, isolate));
     CHECK(i::Rewriter::Rewrite(&info));
     CHECK(!i::DeclarationScope::Analyze(&info));