[Compiler] Split CompileUnoptimizedCode into main and non-main thread phases

Splits CompileUnoptimizedCode into a non-main thread GenerateUnoptimizedCode and
a main thread FinalizeUnoptimizedCode phase. Adds Disallow<HeapAccess> scopes in
CompileUnoptimizedCode to ensure no access to the heap during this phase.

Also cleans up a few heap accesses in CompilationInfo's constructor to avoid
violating the disallowed heap access.

Currently we reallow heap access during asm.js compilation as a temporary
measure until the script streamer uses an off-heap script buffer.

BUG=v8:5203
TBR=titzer@chromium.org

Change-Id: I7f6140f19938a10a85f1cd89501812dd59dbf6d4
Reviewed-on: https://chromium-review.googlesource.com/605949
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47337}

src/asmjs/asm-js.cc

@@ -213,6 +213,14 @@ class AsmJsCompilationJob final : public CompilationJob {
 };
 
 CompilationJob::Status AsmJsCompilationJob::PrepareJobImpl() {
+  // TODO(rmcilroy): Temporarily allow heap access here until we use a
+  // off-heap ScannerStream.
+  DCHECK(
+      ThreadId::Current().Equals(compilation_info()->isolate()->thread_id()));
+  AllowHeapAllocation allow_allocation;
+  AllowHandleAllocation allow_handles;
+  AllowHandleDereference allow_deref;
+
   // Step 1: Translate asm.js module to WebAssembly module.
   HistogramTimerScope translate_time_scope(
       compilation_info()->isolate()->counters()->asm_wasm_translation_time());

src/ast/scopes.cc

@@ -8,6 +8,7 @@
 
 #include "src/accessors.h"
 #include "src/ast/ast.h"
+#include "src/base/optional.h"
 #include "src/bootstrapper.h"
 #include "src/counters.h"
 #include "src/messages.h"
@@ -651,6 +652,12 @@ void DeclarationScope::Analyze(ParseInfo* info) {
   DCHECK(info->literal() != NULL);
   DeclarationScope* scope = info->literal()->scope();
 
+  base::Optional<AllowHandleDereference> allow_deref;
+  if (!info->maybe_outer_scope_info().is_null()) {
+    // Allow dereferences to the scope info if there is one.
+    allow_deref.emplace();
+  }
+
   if (scope->is_eval_scope() && is_sloppy(scope->language_mode())) {
     AstNodeFactory factory(info->ast_value_factory(), info->zone());
     scope->HoistSloppyBlockFunctions(&factory);
@@ -678,8 +685,7 @@ void DeclarationScope::Analyze(ParseInfo* info) {
   scope->AllocateVariables(info);
 
 #ifdef DEBUG
-  if (info->script_is_native() ? FLAG_print_builtin_scopes
-                               : FLAG_print_scopes) {
+  if (info->is_native() ? FLAG_print_builtin_scopes : FLAG_print_scopes) {
     PrintF("Global scope:\n");
     scope->Print();
   }
@@ -1978,7 +1984,7 @@ void UpdateNeedsHoleCheck(Variable* var, VariableProxy* proxy, Scope* scope) {
 
 void Scope::ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var) {
 #ifdef DEBUG
-  if (info->script_is_native()) {
+  if (info->is_native()) {
     // To avoid polluting the global object in native scripts
     //  - Variables must not be allocated to the global scope.
     CHECK_NOT_NULL(outer_scope());

src/background-parsing-task.cc

@@ -43,6 +43,9 @@ BackgroundParsingTask::BackgroundParsingTask(
   if (V8_UNLIKELY(FLAG_runtime_stats)) {
     info->set_runtime_call_stats(new (info->zone()) RuntimeCallStats());
   }
+  if (V8_UNLIKELY(info->block_coverage_enabled())) {
+    info->AllocateSourceRangeMap();
+  }
 
   source_->info->set_cached_data(&script_data_);
   // Parser needs to stay alive for finalizing the parsing on the main

src/codegen.cc

@@ -99,9 +99,13 @@ void CodeGenerator::MakeCodePrologue(ParseInfo* parse_info,
 
   if (!FLAG_trace_codegen && !print_ast) return;
 
-  // Requires internalizing the AST, so make sure we are on the main thread.
+  // Requires internalizing the AST, so make sure we are on the main thread and
+  // allow handle dereference and allocations.
+  // TODO(rmcilroy): Make ast-printer print ast raw strings instead of
+  // internalized strings to avoid internalizing here.
   DCHECK(ThreadId::Current().Equals(info->isolate()->thread_id()));
-  AllowDeferredHandleDereference allow_deref;
+  AllowHandleDereference allow_deref;
+  AllowHandleAllocation allow_handles;
   AllowHeapAllocation allow_gc;
   parse_info->ast_value_factory()->Internalize(info->isolate());
 

src/compilation-info.cc

@@ -30,26 +30,10 @@ CompilationInfo::CompilationInfo(Zone* zone, Isolate* isolate,
   literal_ = literal;
   source_range_map_ = parse_info->source_range_map();
 
-  // Collect source positions for optimized code when profiling or if debugger
-  // is active, to be able to get more precise source positions at the price of
-  // more memory consumption.
-  if (isolate_->NeedsSourcePositionsForProfiling()) {
-    MarkAsSourcePositionsEnabled();
-  }
-
-  if (FLAG_block_coverage && isolate->is_block_code_coverage() &&
-      script_->IsUserJavaScript()) {
-    MarkAsBlockCoverageEnabled();
-  }
-
   if (parse_info->is_debug()) MarkAsDebug();
   if (parse_info->is_eval()) MarkAsEval();
   if (parse_info->is_native()) MarkAsNative();
   if (parse_info->will_serialize()) MarkAsSerializing();
-  if (script_->type() == Script::TYPE_NATIVE) MarkAsNative();
-  if (script_->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
-    MarkAsEval();
-  }
 }
 
 CompilationInfo::CompilationInfo(Zone* zone, Isolate* isolate,

src/compilation-info.h

@@ -40,15 +40,14 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
     kIsEval = 1 << 1,
     kIsNative = 1 << 2,
     kSerializing = 1 << 3,
-    kBlockCoverageEnabled = 1 << 4,
-    kAccessorInliningEnabled = 1 << 5,
-    kFunctionContextSpecializing = 1 << 6,
-    kInliningEnabled = 1 << 7,
-    kDisableFutureOptimization = 1 << 8,
-    kSplittingEnabled = 1 << 9,
-    kSourcePositionsEnabled = 1 << 10,
-    kBailoutOnUninitialized = 1 << 11,
-    kLoopPeelingEnabled = 1 << 12,
+    kAccessorInliningEnabled = 1 << 4,
+    kFunctionContextSpecializing = 1 << 5,
+    kInliningEnabled = 1 << 6,
+    kDisableFutureOptimization = 1 << 7,
+    kSplittingEnabled = 1 << 8,
+    kSourcePositionsEnabled = 1 << 9,
+    kBailoutOnUninitialized = 1 << 10,
+    kLoopPeelingEnabled = 1 << 11,
   };
 
   // Construct a compilation info for unoptimized compilation.
@@ -71,6 +70,7 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
     literal_ = literal;
   }
 
+  bool has_source_range_map() const { return source_range_map_ != nullptr; }
   SourceRangeMap* source_range_map() const { return source_range_map_; }
   void set_source_range_map(SourceRangeMap* source_range_map) {
     source_range_map_ = source_range_map;
@@ -122,11 +122,6 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
   void MarkAsNative() { SetFlag(kIsNative); }
   bool is_native() const { return GetFlag(kIsNative); }
 
-  void MarkAsBlockCoverageEnabled() { SetFlag(kBlockCoverageEnabled); }
-  bool is_block_coverage_enabled() const {
-    return GetFlag(kBlockCoverageEnabled);
-  }
-
   // Flags used by optimized compilation.
 
   void MarkAsFunctionContextSpecializing() {

src/compiler.cc

@@ -255,14 +255,13 @@ void EnsureFeedbackMetadata(CompilationInfo* compilation_info) {
       compilation_info->literal()->feedback_vector_spec()));
 }
 
-bool UseAsmWasm(FunctionLiteral* literal,
-                Handle<SharedFunctionInfo> shared_info, bool is_debug) {
+bool UseAsmWasm(FunctionLiteral* literal, bool asm_wasm_broken, bool is_debug) {
   // Check whether asm.js validation is enabled.
   if (!FLAG_validate_asm) return false;
 
   // Modules that have validated successfully, but were subsequently broken by
   // invalid module instantiation attempts are off limit forever.
-  if (!shared_info.is_null() && shared_info->is_asm_wasm_broken()) return false;
+  if (asm_wasm_broken) return false;
 
   // Compiling for debugging is not supported, fall back.
   if (is_debug) return false;
@@ -316,7 +315,8 @@ void InstallUnoptimizedCode(CompilationInfo* compilation_info) {
 
   // Install coverage info on the shared function info.
   if (compilation_info->has_coverage_info()) {
-    DCHECK(compilation_info->is_block_coverage_enabled());
+    DCHECK(FLAG_block_coverage &&
+           compilation_info->isolate()->is_block_code_coverage());
     compilation_info->isolate()->debug()->InstallCoverageInfo(
         compilation_info->shared_info(), compilation_info->coverage_info());
   }
@@ -384,9 +384,9 @@ bool Renumber(ParseInfo* parse_info,
 }
 
 std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
-    Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
-  if (UseAsmWasm(literal, shared_info, parse_info->is_debug())) {
+    ParseInfo* parse_info, FunctionLiteral* literal, Isolate* isolate) {
+  if (UseAsmWasm(literal, parse_info->is_asm_wasm_broken(),
+                 parse_info->is_debug())) {
     std::unique_ptr<CompilationJob> asm_job(
         AsmJs::NewCompilationJob(parse_info, literal, isolate));
     if (asm_job->PrepareJob() == CompilationJob::SUCCEEDED &&
@@ -409,41 +409,45 @@ std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJob(
   return std::unique_ptr<CompilationJob>();  // Compilation failed, return null.
 }
 
-Handle<SharedFunctionInfo> CompileUnoptimizedCode(
-    ParseInfo* parse_info, Handle<SharedFunctionInfo> shared_info,
-    Isolate* isolate) {
-  DCHECK(AllowCompilation::IsAllowed(isolate));
+// TODO(rmcilroy): Remove |isolate| once CompilationJob doesn't need it.
+std::unique_ptr<CompilationJob> GenerateUnoptimizedCode(
+    ParseInfo* parse_info, Isolate* isolate,
+    std::forward_list<std::unique_ptr<CompilationJob>>* inner_function_jobs) {
+  DisallowHeapAllocation no_allocation;
+  DisallowHandleAllocation no_handles;
+  DisallowHandleDereference no_deref;
+  DCHECK(inner_function_jobs->empty());
 
   Compiler::EagerInnerFunctionLiterals inner_literals;
   if (!Compiler::Analyze(parse_info, &inner_literals)) {
-    if (!isolate->has_pending_exception()) isolate->StackOverflow();
-    return Handle<SharedFunctionInfo>::null();
+    return std::unique_ptr<CompilationJob>();
   }
 
   // Prepare and execute compilation of the outer-most function.
-  std::unique_ptr<CompilationJob> outer_job(
+  std::unique_ptr<CompilationJob> outer_function_job(
       PrepareAndExecuteUnoptimizedCompileJob(parse_info, parse_info->literal(),
-                                             shared_info, isolate));
-  if (!outer_job) {
-    if (!isolate->has_pending_exception()) isolate->StackOverflow();
-    return Handle<SharedFunctionInfo>::null();
-  }
+                                             isolate));
+  if (!outer_function_job) return std::unique_ptr<CompilationJob>();
 
   // Prepare and execute compilation jobs for eager inner functions.
-  std::forward_list<std::unique_ptr<CompilationJob>> inner_jobs;
   for (auto it : inner_literals) {
     FunctionLiteral* inner_literal = it->value();
     std::unique_ptr<CompilationJob> inner_job(
-        PrepareAndExecuteUnoptimizedCompileJob(
-            parse_info, inner_literal, Handle<SharedFunctionInfo>::null(),
-            isolate));
-    if (!inner_job) {
-      if (!isolate->has_pending_exception()) isolate->StackOverflow();
-      return Handle<SharedFunctionInfo>::null();
-    }
-    inner_jobs.emplace_front(std::move(inner_job));
+        PrepareAndExecuteUnoptimizedCompileJob(parse_info, inner_literal,
+                                               isolate));
+    if (!inner_job) return std::unique_ptr<CompilationJob>();
+    inner_function_jobs->emplace_front(std::move(inner_job));
   }
 
+  return outer_function_job;
+}
+
+bool FinalizeUnoptimizedCode(
+    ParseInfo* parse_info, Isolate* isolate,
+    Handle<SharedFunctionInfo> shared_info, CompilationJob* outer_function_job,
+    std::forward_list<std::unique_ptr<CompilationJob>>* inner_function_jobs) {
+  DCHECK(AllowCompilation::IsAllowed(isolate));
+
   // Internalize ast values onto the heap.
   parse_info->ast_value_factory()->Internalize(isolate);
 
@@ -465,15 +469,14 @@ Handle<SharedFunctionInfo> CompileUnoptimizedCode(
   }
 
   // Finalize the outer-most function's compilation job.
-  outer_job->compilation_info()->set_shared_info(shared_info);
-  if (FinalizeUnoptimizedCompilationJob(outer_job.get()) !=
+  outer_function_job->compilation_info()->set_shared_info(shared_info);
+  if (FinalizeUnoptimizedCompilationJob(outer_function_job) !=
       CompilationJob::SUCCEEDED) {
-    if (!isolate->has_pending_exception()) isolate->StackOverflow();
-    return Handle<SharedFunctionInfo>::null();
+    return false;
   }
 
   // Finalize the inner functions' compilation jobs.
-  for (auto&& inner_job : inner_jobs) {
+  for (auto&& inner_job : *inner_function_jobs) {
     Handle<SharedFunctionInfo> inner_shared_info =
         Compiler::GetSharedFunctionInfo(
             inner_job->compilation_info()->literal(), parse_info->script(),
@@ -484,13 +487,10 @@ Handle<SharedFunctionInfo> CompileUnoptimizedCode(
     inner_job->compilation_info()->set_shared_info(inner_shared_info);
     if (FinalizeUnoptimizedCompilationJob(inner_job.get()) !=
         CompilationJob::SUCCEEDED) {
-      if (!isolate->has_pending_exception()) isolate->StackOverflow();
-      return Handle<SharedFunctionInfo>::null();
+      return false;
     }
   }
-
-  // Compilation succeeded, return result.
-  return shared_info;
+  return true;
 }
 
 MUST_USE_RESULT MaybeHandle<Code> CompileUnoptimizedFunction(
@@ -506,15 +506,24 @@ MUST_USE_RESULT MaybeHandle<Code> CompileUnoptimizedFunction(
     return MaybeHandle<Code>();
   }
 
-  // Compile either unoptimized code or bytecode for the interpreter.
-  Handle<SharedFunctionInfo> result =
-      CompileUnoptimizedCode(parse_info, shared_info, isolate);
-  if (result.is_null()) {
+  // Generate the unoptimized bytecode or asm-js data.
+  std::forward_list<std::unique_ptr<CompilationJob>> inner_function_jobs;
+  std::unique_ptr<CompilationJob> outer_function_job(
+      GenerateUnoptimizedCode(parse_info, isolate, &inner_function_jobs));
+  if (!outer_function_job) {
+    if (!isolate->has_pending_exception()) isolate->StackOverflow();
+    return MaybeHandle<Code>();
+  }
+
+  // Finalize compilation of the unoptimized bytecode or asm-js data.
+  if (!FinalizeUnoptimizedCode(parse_info, isolate, shared_info,
+                               outer_function_job.get(),
+                               &inner_function_jobs)) {
+    if (!isolate->has_pending_exception()) isolate->StackOverflow();
     return MaybeHandle<Code>();
   }
-  DCHECK(shared_info.is_identical_to(result));
 
-  return handle(result->code(), isolate);
+  return handle(shared_info->code(), isolate);
 }
 
 MUST_USE_RESULT MaybeHandle<Code> GetCodeFromOptimizedCodeCache(
@@ -906,11 +915,23 @@ Handle<SharedFunctionInfo> CompileToplevel(
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                  parse_info->is_eval() ? "V8.CompileEval" : "V8.Compile");
 
-    // Compile the code.
-    result = CompileUnoptimizedCode(parse_info, shared_info, isolate);
-    if (result.is_null()) {
+    // Generate the unoptimized bytecode or asm-js data.
+    std::forward_list<std::unique_ptr<CompilationJob>> inner_function_jobs;
+    std::unique_ptr<CompilationJob> outer_function_job(
+        GenerateUnoptimizedCode(parse_info, isolate, &inner_function_jobs));
+    if (!outer_function_job) {
+      if (!isolate->has_pending_exception()) isolate->StackOverflow();
+      return Handle<SharedFunctionInfo>::null();
+    }
+
+    // Finalize compilation of the unoptimized bytecode or asm-js data.
+    if (!FinalizeUnoptimizedCode(parse_info, isolate, shared_info,
+                                 outer_function_job.get(),
+                                 &inner_function_jobs)) {
+      if (!isolate->has_pending_exception()) isolate->StackOverflow();
       return Handle<SharedFunctionInfo>::null();
     }
+    result = outer_function_job->compilation_info()->shared_info();
     DCHECK_IMPLIES(!shared_info.is_null(), shared_info.is_identical_to(result));
 
     if (!script.is_null()) {

src/interpreter/bytecode-generator.cc

@@ -772,7 +772,7 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
       loop_depth_(0),
       catch_prediction_(HandlerTable::UNCAUGHT) {
   DCHECK_EQ(closure_scope(), closure_scope()->GetClosureScope());
-  if (info->is_block_coverage_enabled()) {
+  if (info->has_source_range_map()) {
     DCHECK(FLAG_block_coverage);
     block_coverage_builder_ = new (zone())
         BlockCoverageBuilder(zone(), builder(), info->source_range_map());
@@ -784,7 +784,7 @@ Handle<BytecodeArray> BytecodeGenerator::FinalizeBytecode(Isolate* isolate) {
 
   AllocateDeferredConstants(isolate);
 
-  if (info()->is_block_coverage_enabled()) {
+  if (block_coverage_builder_) {
     info()->set_coverage_info(
         isolate->factory()->NewCoverageInfo(block_coverage_builder_->slots()));
     if (FLAG_trace_block_coverage) {

src/parsing/parse-info.cc

@@ -57,6 +57,7 @@ ParseInfo::ParseInfo(Handle<SharedFunctionInfo> shared)
   function_literal_id_ = shared->function_literal_id();
   set_language_mode(shared->language_mode());
   set_module(shared->kind() == FunctionKind::kModule);
+  set_asm_wasm_broken(shared->is_asm_wasm_broken());
 
   Handle<Script> script(Script::cast(shared->script()));
   set_script(script);
@@ -76,6 +77,9 @@ ParseInfo::ParseInfo(Handle<SharedFunctionInfo> shared)
       shared->feedback_metadata()->length() == 0
           ? FLAG_type_profile && script->IsUserJavaScript()
           : shared->feedback_metadata()->HasTypeProfileSlot());
+  if (block_coverage_enabled() && script->IsUserJavaScript()) {
+    AllocateSourceRangeMap();
+  }
 }
 
 ParseInfo::ParseInfo(Handle<Script> script)
@@ -90,6 +94,9 @@ ParseInfo::ParseInfo(Handle<Script> script)
   set_eval(script->compilation_type() == Script::COMPILATION_TYPE_EVAL);
 
   set_collect_type_profile(FLAG_type_profile && script->IsUserJavaScript());
+  if (block_coverage_enabled() && script->IsUserJavaScript()) {
+    AllocateSourceRangeMap();
+  }
 }
 
 // static
@@ -145,7 +152,7 @@ void ParseInfo::InitFromIsolate(Isolate* isolate) {
   set_runtime_call_stats(isolate->counters()->runtime_call_stats());
   set_ast_string_constants(isolate->ast_string_constants());
   if (FLAG_block_coverage && isolate->is_block_code_coverage()) {
-    set_source_range_map(new (zone()) SourceRangeMap(zone()));
+    set_block_coverage_enabled();
   }
 }
 
@@ -199,11 +206,10 @@ void ParseInfo::ShareAstValueFactory(ParseInfo* other) {
   ast_value_factory_ = other->ast_value_factory_;
 }
 
-#ifdef DEBUG
-bool ParseInfo::script_is_native() const {
-  return script_->type() == Script::TYPE_NATIVE;
+void ParseInfo::AllocateSourceRangeMap() {
+  DCHECK(block_coverage_enabled());
+  set_source_range_map(new (zone()) SourceRangeMap(zone()));
 }
-#endif  // DEBUG
 
 }  // namespace internal
 }  // namespace v8

src/parsing/parse-info.h

@@ -78,6 +78,9 @@ class V8_EXPORT_PRIVATE ParseInfo : public UnoptimizedCompileJobFinishCallback {
   FLAG_ACCESSOR(kLazyCompile, lazy_compile, set_lazy_compile)
   FLAG_ACCESSOR(kCollectTypeProfile, collect_type_profile,
                 set_collect_type_profile)
+  FLAG_ACCESSOR(kIsAsmWasmBroken, is_asm_wasm_broken, set_asm_wasm_broken)
+  FLAG_ACCESSOR(kBlockCoverageEnabled, block_coverage_enabled,
+                set_block_coverage_enabled)
 #undef FLAG_ACCESSOR
 
   void set_parse_restriction(ParseRestriction restriction) {
@@ -203,6 +206,7 @@ class V8_EXPORT_PRIVATE ParseInfo : public UnoptimizedCompileJobFinishCallback {
     runtime_call_stats_ = runtime_call_stats;
   }
 
+  void AllocateSourceRangeMap();
   SourceRangeMap* source_range_map() const { return source_range_map_; }
   void set_source_range_map(SourceRangeMap* source_range_map) {
     source_range_map_ = source_range_map;
@@ -251,10 +255,6 @@ class V8_EXPORT_PRIVATE ParseInfo : public UnoptimizedCompileJobFinishCallback {
 
   void ParseFinished(std::unique_ptr<ParseInfo> info) override;
 
-#ifdef DEBUG
-  bool script_is_native() const;
-#endif  // DEBUG
-
  private:
   // Various configuration flags for parsing.
   enum Flag {
@@ -272,6 +272,8 @@ class V8_EXPORT_PRIVATE ParseInfo : public UnoptimizedCompileJobFinishCallback {
     kSerializing = 1 << 10,
     kLazyCompile = 1 << 11,
     kCollectTypeProfile = 1 << 12,
+    kBlockCoverageEnabled = 1 << 13,
+    kIsAsmWasmBroken = 1 << 14,
   };
 
   //------------- Inputs to parsing and scope analysis -----------------------