[compiler] Make source position collection lazier

Previously when lazy source positions were enabled, source positions
were immediately collected whenever an exception was thrown for every
frame in the stack trace.

This change makes source position collection trigger only when the
source positions of a stack frame are actually accessed with the
exception of the top frame which is still eagerly collected for now.

Additionally when stack overflows occur during source position
collection, the bytecode is marked with exception in the
source_position_table field so it can be distinguished from the case
where source position collection has never been attempted (undefined)
or is not desired because the bytecode is for natives
(empty_byte_array).

Bug: v8:8510
Change-Id: If7ee68edbacc9e2adadf00fe5ec822a8dbe1c79a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1520721Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60504}

src/builtins/builtins-function.cc

@@ -150,7 +150,7 @@ BUILTIN(AsyncFunctionConstructor) {
   Handle<JSFunction> func = Handle<JSFunction>::cast(maybe_func);
   Handle<Script> script =
       handle(Script::cast(func->shared()->script()), isolate);
-  int position = script->GetEvalPosition();
+  int position = Script::GetEvalPosition(isolate, script);
   USE(position);
 
   return *func;
@@ -169,7 +169,7 @@ BUILTIN(AsyncGeneratorFunctionConstructor) {
   Handle<JSFunction> func = Handle<JSFunction>::cast(maybe_func);
   Handle<Script> script =
       handle(Script::cast(func->shared()->script()), isolate);
-  int position = script->GetEvalPosition();
+  int position = Script::GetEvalPosition(isolate, script);
   USE(position);
 
   return *func;

src/compiler.cc

@@ -1132,12 +1132,22 @@ bool Compiler::CollectSourcePositions(Isolate* isolate,
   DCHECK(shared_info->HasBytecodeArray());
   DCHECK(!shared_info->GetBytecodeArray()->HasSourcePositionTable());
 
+  // Collecting source positions requires allocating a new source position
+  // table.
+  DCHECK(AllowHeapAllocation::IsAllowed());
+
+  Handle<BytecodeArray> bytecode =
+      handle(shared_info->GetBytecodeArray(), isolate);
+
   // TODO(v8:8510): Push the CLEAR_EXCEPTION flag or something like it down into
   // the parser so it aborts without setting a pending exception, which then
   // gets thrown. This would avoid the situation where potentially we'd reparse
   // several times (running out of stack each time) before hitting this limit.
-  if (GetCurrentStackPosition() < isolate->stack_guard()->real_climit())
+  if (GetCurrentStackPosition() < isolate->stack_guard()->real_climit()) {
+    // Stack is already exhausted.
+    bytecode->SetSourcePositionsFailedToCollect();
     return false;
+  }
 
   DCHECK(AllowCompilation::IsAllowed(isolate));
   DCHECK_EQ(ThreadId::Current(), isolate->thread_id());
@@ -1158,6 +1168,8 @@ bool Compiler::CollectSourcePositions(Isolate* isolate,
 
   // Parse and update ParseInfo with the results.
   if (!parsing::ParseAny(&parse_info, shared_info, isolate)) {
+    // Parsing failed probably as a result of stack exhaustion.
+    bytecode->SetSourcePositionsFailedToCollect();
     return FailWithPendingException(
         isolate, &parse_info, Compiler::ClearExceptionFlag::CLEAR_EXCEPTION);
   }
@@ -1170,6 +1182,8 @@ bool Compiler::CollectSourcePositions(Isolate* isolate,
       GenerateUnoptimizedCode(&parse_info, isolate->allocator(),
                               &inner_function_jobs));
   if (!outer_function_job) {
+    // Recompiling failed probably as a result of stack exhaustion.
+    bytecode->SetSourcePositionsFailedToCollect();
     return FailWithPendingException(
         isolate, &parse_info, Compiler::ClearExceptionFlag::CLEAR_EXCEPTION);
   }
@@ -1194,14 +1208,20 @@ bool Compiler::CollectSourcePositions(Isolate* isolate,
   }
 
   // Update the source position table on the original bytecode.
-  Handle<BytecodeArray> bytecode =
-      handle(shared_info->GetBytecodeArray(), isolate);
   DCHECK(bytecode->IsBytecodeEqual(
       *outer_function_job->compilation_info()->bytecode_array()));
   DCHECK(outer_function_job->compilation_info()->has_bytecode_array());
-  bytecode->set_source_position_table(outer_function_job->compilation_info()
-                                          ->bytecode_array()
-                                          ->SourcePositionTable());
+  ByteArray source_position_table = outer_function_job->compilation_info()
+                                        ->bytecode_array()
+                                        ->SourcePositionTable();
+  bytecode->set_source_position_table(source_position_table);
+  // If debugging, make sure that instrumented bytecode has the source position
+  // table set on it as well.
+  if (shared_info->HasDebugInfo() &&
+      shared_info->GetDebugInfo()->HasInstrumentedBytecodeArray()) {
+    shared_info->GetDebugBytecodeArray()->set_source_position_table(
+        source_position_table);
+  }
 
   DCHECK(!isolate->has_pending_exception());
   DCHECK(shared_info->is_compiled_scope().is_compiled());

src/compiler/bytecode-graph-builder.cc

@@ -886,7 +886,7 @@ void BytecodeGraphBuilder::VisitBytecodes() {
   interpreter::BytecodeArrayIterator iterator(bytecode_array());
   set_bytecode_iterator(&iterator);
   SourcePositionTableIterator source_position_iterator(
-      handle(bytecode_array()->SourcePositionTable(), isolate()));
+      handle(bytecode_array()->SourcePositionTableIfCollected(), isolate()));
 
   if (analyze_environment_liveness() && FLAG_trace_environment_liveness) {
     StdoutStream of;

src/debug/debug-frames.cc

@@ -19,6 +19,7 @@ FrameInspector::FrameInspector(StandardFrame* frame, int inlined_frame_index,
       isolate_(isolate) {
   // Extract the relevant information from the frame summary and discard it.
   FrameSummary summary = FrameSummary::Get(frame, inlined_frame_index);
+  summary.EnsureSourcePositionsAvailable();
 
   is_constructor_ = summary.is_constructor();
   source_position_ = summary.SourcePosition();

src/debug/liveedit.cc

@@ -1114,6 +1114,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
       Handle<DebugInfo> debug_info(sfi->GetDebugInfo(), isolate);
       isolate->debug()->RemoveBreakInfoAndMaybeFree(debug_info);
     }
+    SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, sfi);
     UpdatePositions(isolate, sfi, diffs);
 
     sfi->set_script(*new_script);

src/frames.cc

@@ -1274,13 +1274,19 @@ FrameSummary::JavaScriptFrameSummary::JavaScriptFrameSummary(
       parameters_(parameters, isolate) {
   DCHECK(abstract_code->IsBytecodeArray() ||
          Code::cast(abstract_code)->kind() != Code::OPTIMIZED_FUNCTION);
-  // TODO(v8:8510): Move this to the SourcePosition getter.
-  if (FLAG_enable_lazy_source_positions && abstract_code->IsBytecodeArray()) {
-    SharedFunctionInfo::EnsureSourcePositionsAvailable(
-        isolate, handle(function->shared(), isolate));
+}
+
+void FrameSummary::EnsureSourcePositionsAvailable() {
+  if (IsJavaScript()) {
+    java_script_summary_.EnsureSourcePositionsAvailable();
   }
 }
 
+void FrameSummary::JavaScriptFrameSummary::EnsureSourcePositionsAvailable() {
+  Handle<SharedFunctionInfo> shared(function()->shared(), isolate());
+  SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate(), shared);
+}
+
 bool FrameSummary::JavaScriptFrameSummary::is_subject_to_debugging() const {
   return function()->shared()->IsSubjectToDebugging();
 }
@@ -1972,6 +1978,9 @@ void PrintFunctionSource(StringStream* accumulator, SharedFunctionInfo shared,
 void JavaScriptFrame::Print(StringStream* accumulator,
                             PrintMode mode,
                             int index) const {
+  Handle<SharedFunctionInfo> shared = handle(function()->shared(), isolate());
+  SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate(), shared);
+
   DisallowHeapAllocation no_gc;
   Object receiver = this->receiver();
   JSFunction function = this->function();
@@ -1988,7 +1997,6 @@ void JavaScriptFrame::Print(StringStream* accumulator,
   // doesn't contain scope info, scope_info will return 0 for the number of
   // parameters, stack local variables, context local variables, stack slots,
   // or context slots.
-  SharedFunctionInfo shared = function->shared();
   ScopeInfo scope_info = shared->scope_info();
   Object script_obj = shared->script();
   if (script_obj->IsScript()) {
@@ -2028,7 +2036,7 @@ void JavaScriptFrame::Print(StringStream* accumulator,
   }
   if (is_optimized()) {
     accumulator->Add(" {\n// optimized frame\n");
-    PrintFunctionSource(accumulator, shared, code);
+    PrintFunctionSource(accumulator, *shared, code);
     accumulator->Add("}\n");
     return;
   }
@@ -2078,7 +2086,7 @@ void JavaScriptFrame::Print(StringStream* accumulator,
     accumulator->Add("  [%02d] : %o\n", i, GetExpression(i));
   }
 
-  PrintFunctionSource(accumulator, shared, code);
+  PrintFunctionSource(accumulator, *shared, code);
 
   accumulator->Add("}\n\n");
 }

src/frames.h

@@ -482,6 +482,8 @@ class FrameSummary {
                            int code_offset, bool is_constructor,
                            FixedArray parameters);
 
+    void EnsureSourcePositionsAvailable();
+
     Handle<Object> receiver() const { return receiver_; }
     Handle<JSFunction> function() const { return function_; }
     Handle<AbstractCode> abstract_code() const { return abstract_code_; }
@@ -569,6 +571,8 @@ class FrameSummary {
   static FrameSummary GetSingle(const StandardFrame* frame);
   static FrameSummary Get(const StandardFrame* frame, int index);
 
+  void EnsureSourcePositionsAvailable();
+
   // Dispatched accessors.
   Handle<Object> receiver() const;
   int code_offset() const;

src/interpreter/bytecode-array-writer.cc

@@ -50,11 +50,11 @@ Handle<BytecodeArray> BytecodeArrayWriter::ToBytecodeArray(
       bytecode_size, &bytecodes()->front(), frame_size, parameter_count,
       constant_pool);
   bytecode_array->set_handler_table(*handler_table);
-  // TODO(v8:8510): Need to support native functions that should always have
-  // source positions suppressed and should write empty_byte_array here.
-  if (!source_position_table_builder_.Omit()) {
+  if (!source_position_table_builder_.Lazy()) {
     Handle<ByteArray> source_position_table =
-        source_position_table_builder()->ToSourcePositionTable(isolate);
+        source_position_table_builder_.Omit()
+            ? ReadOnlyRoots(isolate).empty_byte_array_handle()
+            : source_position_table_builder()->ToSourcePositionTable(isolate);
     bytecode_array->set_source_position_table(*source_position_table);
     LOG_CODE_EVENT(isolate, CodeLinePosInfoRecordEvent(
                                 bytecode_array->GetFirstBytecodeAddress(),

src/isolate.cc

@@ -1003,11 +1003,12 @@ Handle<Object> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
         std::vector<FrameSummary> frames;
         StandardFrame::cast(frame)->Summarize(&frames);
         for (size_t i = frames.size(); i-- != 0 && !builder.full();) {
-          const auto& summary = frames[i];
+          auto& summary = frames[i];
           if (options.capture_only_frames_subject_to_debugging &&
               !summary.is_subject_to_debugging()) {
             continue;
           }
+          summary.EnsureSourcePositionsAvailable();
 
           if (summary.IsJavaScript()) {
             //=========================================================
@@ -1181,6 +1182,9 @@ Address Isolate::GetAbstractPC(int* line, int* column) {
   }
   JavaScriptFrame* frame = it.frame();
   DCHECK(!frame->is_builtin());
+
+  Handle<SharedFunctionInfo> shared = handle(frame->function()->shared(), this);
+  SharedFunctionInfo::EnsureSourcePositionsAvailable(this, shared);
   int position = frame->position();
 
   Object maybe_script = frame->function()->shared()->script();
@@ -2044,6 +2048,7 @@ bool Isolate::ComputeLocation(MessageLocation* target) {
   std::vector<FrameSummary> frames;
   frame->Summarize(&frames);
   FrameSummary& summary = frames.back();
+  summary.EnsureSourcePositionsAvailable();
   int pos = summary.SourcePosition();
   Handle<SharedFunctionInfo> shared;
   Handle<Object> script = summary.script();
@@ -2131,6 +2136,8 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
     Object script = fun->shared()->script();
     if (script->IsScript() &&
         !(Script::cast(script)->source()->IsUndefined(this))) {
+      Handle<SharedFunctionInfo> shared = handle(fun->shared(), this);
+      SharedFunctionInfo::EnsureSourcePositionsAvailable(this, shared);
       AbstractCode abstract_code = elements->Code(i);
       const int code_offset = elements->Offset(i)->value();
       const int pos = abstract_code->SourcePosition(code_offset);

src/log.cc

@@ -2061,6 +2061,7 @@ void ExistingCodeLogger::LogCompiledFunctions() {
   // During iteration, there can be heap allocation due to
   // GetScriptLineNumber call.
   for (int i = 0; i < compiled_funcs_count; ++i) {
+    SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate_, sfis[i]);
     if (sfis[i]->function_data()->IsInterpreterData()) {
       LogExistingFunction(
           sfis[i],

src/messages.cc

@@ -205,18 +205,6 @@ Object EvalFromFunctionName(Isolate* isolate, Handle<Script> script) {
   return shared->inferred_name();
 }
 
-Object EvalFromScript(Isolate* isolate, Handle<Script> script) {
-  if (!script->has_eval_from_shared()) {
-    return ReadOnlyRoots(isolate).undefined_value();
-  }
-
-  Handle<SharedFunctionInfo> eval_from_shared(script->eval_from_shared(),
-                                              isolate);
-  return eval_from_shared->script()->IsScript()
-             ? eval_from_shared->script()
-             : ReadOnlyRoots(isolate).undefined_value();
-}
-
 MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
   Handle<Object> sourceURL(script->GetNameOrSourceURL(), isolate);
   if (!sourceURL->IsUndefined(isolate)) {
@@ -239,44 +227,49 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
     builder.AppendCString("<anonymous>");
   }
 
-  Handle<Object> eval_from_script_obj =
-      handle(EvalFromScript(isolate, script), isolate);
-  if (eval_from_script_obj->IsScript()) {
-    Handle<Script> eval_from_script =
-        Handle<Script>::cast(eval_from_script_obj);
-    builder.AppendCString(" (");
-    if (eval_from_script->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
-      // Eval script originated from another eval.
-      Handle<String> str;
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, str, FormatEvalOrigin(isolate, eval_from_script), String);
-      builder.AppendString(str);
-    } else {
-      DCHECK(eval_from_script->compilation_type() !=
-             Script::COMPILATION_TYPE_EVAL);
-      // eval script originated from "real" source.
-      Handle<Object> name_obj = handle(eval_from_script->name(), isolate);
-      if (eval_from_script->name()->IsString()) {
-        builder.AppendString(Handle<String>::cast(name_obj));
-
-        Script::PositionInfo info;
-        if (Script::GetPositionInfo(eval_from_script, script->GetEvalPosition(),
-                                    &info, Script::NO_OFFSET)) {
-          builder.AppendCString(":");
-
-          Handle<String> str = isolate->factory()->NumberToString(
-              handle(Smi::FromInt(info.line + 1), isolate));
-          builder.AppendString(str);
-
-          builder.AppendCString(":");
-
-          str = isolate->factory()->NumberToString(
-              handle(Smi::FromInt(info.column + 1), isolate));
-          builder.AppendString(str);
-        }
+  if (script->has_eval_from_shared()) {
+    Handle<SharedFunctionInfo> eval_from_shared(script->eval_from_shared(),
+                                                isolate);
+    if (eval_from_shared->script()->IsScript()) {
+      Handle<Script> eval_from_script =
+          handle(Script::cast(eval_from_shared->script()), isolate);
+      builder.AppendCString(" (");
+      if (eval_from_script->compilation_type() ==
+          Script::COMPILATION_TYPE_EVAL) {
+        // Eval script originated from another eval.
+        Handle<String> str;
+        ASSIGN_RETURN_ON_EXCEPTION(
+            isolate, str, FormatEvalOrigin(isolate, eval_from_script), String);
+        builder.AppendString(str);
       } else {
-        DCHECK(!eval_from_script->name()->IsString());
-        builder.AppendCString("unknown source");
+        DCHECK(eval_from_script->compilation_type() !=
+               Script::COMPILATION_TYPE_EVAL);
+        // eval script originated from "real" source.
+        Handle<Object> name_obj = handle(eval_from_script->name(), isolate);
+        if (eval_from_script->name()->IsString()) {
+          builder.AppendString(Handle<String>::cast(name_obj));
+
+          Script::PositionInfo info;
+
+          if (Script::GetPositionInfo(eval_from_script,
+                                      Script::GetEvalPosition(isolate, script),
+                                      &info, Script::NO_OFFSET)) {
+            builder.AppendCString(":");
+
+            Handle<String> str = isolate->factory()->NumberToString(
+                handle(Smi::FromInt(info.line + 1), isolate));
+            builder.AppendString(str);
+
+            builder.AppendCString(":");
+
+            str = isolate->factory()->NumberToString(
+                handle(Smi::FromInt(info.column + 1), isolate));
+            builder.AppendString(str);
+          }
+        } else {
+          DCHECK(!eval_from_script->name()->IsString());
+          builder.AppendCString("unknown source");
+        }
       }
     }
     builder.AppendCString(")");
@@ -667,7 +660,11 @@ void JSStackFrame::ToString(IncrementalStringBuilder& builder) {
   return;
 }
 
-int JSStackFrame::GetPosition() const { return code_->SourcePosition(offset_); }
+int JSStackFrame::GetPosition() const {
+  Handle<SharedFunctionInfo> shared = handle(function_->shared(), isolate_);
+  SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate_, shared);
+  return code_->SourcePosition(offset_);
+}
 
 bool JSStackFrame::HasScript() const {
   return function_->shared()->script()->IsScript();

src/objects.cc

@@ -4667,22 +4667,24 @@ void Oddball::Initialize(Isolate* isolate, Handle<Oddball> oddball,
   oddball->set_kind(kind);
 }
 
-int Script::GetEvalPosition() {
-  DisallowHeapAllocation no_gc;
-  DCHECK(compilation_type() == Script::COMPILATION_TYPE_EVAL);
-  int position = eval_from_position();
+// static
+int Script::GetEvalPosition(Isolate* isolate, Handle<Script> script) {
+  DCHECK(script->compilation_type() == Script::COMPILATION_TYPE_EVAL);
+  int position = script->eval_from_position();
   if (position < 0) {
     // Due to laziness, the position may not have been translated from code
     // offset yet, which would be encoded as negative integer. In that case,
     // translate and set the position.
-    if (!has_eval_from_shared()) {
+    if (!script->has_eval_from_shared()) {
       position = 0;
     } else {
-      SharedFunctionInfo shared = eval_from_shared();
+      Handle<SharedFunctionInfo> shared =
+          handle(script->eval_from_shared(), isolate);
+      SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, shared);
       position = shared->abstract_code()->SourcePosition(-position);
     }
     DCHECK_GE(position, 0);
-    set_eval_from_position(position);
+    script->set_eval_from_position(position);
   }
   return position;
 }

src/objects/code-inl.h

@@ -233,8 +233,17 @@ void Code::clear_padding() {
          CodeSize() - (data_end - address()));
 }
 
+ByteArray Code::SourcePositionTableIfCollected() const {
+  ReadOnlyRoots roots = GetReadOnlyRoots();
+  Object maybe_table = source_position_table();
+  if (maybe_table->IsUndefined(roots) || maybe_table->IsException(roots))
+    return roots.empty_byte_array();
+  return SourcePositionTable();
+}
+
 ByteArray Code::SourcePositionTable() const {
   Object maybe_table = source_position_table();
+  DCHECK(!maybe_table->IsUndefined() && !maybe_table->IsException());
   if (maybe_table->IsByteArray()) return ByteArray::cast(maybe_table);
   DCHECK(maybe_table->IsSourcePositionTableWithFrameCache());
   return SourcePositionTableWithFrameCache::cast(maybe_table)
@@ -714,22 +723,37 @@ Address BytecodeArray::GetFirstBytecodeAddress() {
   return ptr() - kHeapObjectTag + kHeaderSize;
 }
 
-bool BytecodeArray::HasSourcePositionTable() {
+bool BytecodeArray::HasSourcePositionTable() const {
   Object maybe_table = source_position_table();
-  return !maybe_table->IsUndefined();
+  return !(maybe_table->IsUndefined() || DidSourcePositionGenerationFail());
+}
+
+bool BytecodeArray::DidSourcePositionGenerationFail() const {
+  return source_position_table()->IsException();
 }
 
-ByteArray BytecodeArray::SourcePositionTable() {
+void BytecodeArray::SetSourcePositionsFailedToCollect() {
+  set_source_position_table(GetReadOnlyRoots().exception());
+}
+
+ByteArray BytecodeArray::SourcePositionTable() const {
   Object maybe_table = source_position_table();
   if (maybe_table->IsByteArray()) return ByteArray::cast(maybe_table);
   ReadOnlyRoots roots = GetReadOnlyRoots();
-  if (maybe_table->IsUndefined(roots)) return roots.empty_byte_array();
+  if (maybe_table->IsException(roots)) return roots.empty_byte_array();
 
+  DCHECK(!maybe_table->IsUndefined(roots));
   DCHECK(maybe_table->IsSourcePositionTableWithFrameCache());
   return SourcePositionTableWithFrameCache::cast(maybe_table)
       ->source_position_table();
 }
 
+ByteArray BytecodeArray::SourcePositionTableIfCollected() const {
+  if (!HasSourcePositionTable()) return GetReadOnlyRoots().empty_byte_array();
+
+  return SourcePositionTable();
+}
+
 void BytecodeArray::ClearFrameCacheFromSourcePositionTable() {
   Object maybe_table = source_position_table();
   if (maybe_table->IsUndefined() || maybe_table->IsByteArray()) return;
@@ -744,7 +768,9 @@ int BytecodeArray::SizeIncludingMetadata() {
   int size = BytecodeArraySize();
   size += constant_pool()->Size();
   size += handler_table()->Size();
-  size += SourcePositionTable()->Size();
+  if (HasSourcePositionTable()) {
+    size += SourcePositionTable()->Size();
+  }
   return size;
 }
 

src/objects/code.cc

@@ -162,12 +162,14 @@ template <typename Code>
 void SetStackFrameCacheCommon(Isolate* isolate, Handle<Code> code,
                               Handle<SimpleNumberDictionary> cache) {
   Handle<Object> maybe_table(code->source_position_table(), isolate);
+  if (maybe_table->IsException(isolate)) return;
   if (maybe_table->IsSourcePositionTableWithFrameCache()) {
     Handle<SourcePositionTableWithFrameCache>::cast(maybe_table)
         ->set_stack_frame_cache(*cache);
     return;
   }
-  DCHECK(maybe_table->IsUndefined() || maybe_table->IsByteArray());
+  DCHECK(!maybe_table->IsUndefined(isolate));
+  DCHECK(maybe_table->IsByteArray());
   Handle<ByteArray> table(Handle<ByteArray>::cast(maybe_table));
   Handle<SourcePositionTableWithFrameCache> table_with_cache =
       isolate->factory()->NewSourcePositionTableWithFrameCache(table, cache);
@@ -211,10 +213,14 @@ void AbstractCode::DropStackFrameCache() {
 }
 
 int AbstractCode::SourcePosition(int offset) {
+  Object maybe_table = source_position_table();
+  if (maybe_table->IsException()) return kNoSourcePosition;
+
+  ByteArray source_position_table = ByteArray::cast(maybe_table);
   int position = 0;
   // Subtract one because the current PC is one instruction after the call site.
   if (IsCode()) offset--;
-  for (SourcePositionTableIterator iterator(source_position_table());
+  for (SourcePositionTableIterator iterator(source_position_table);
        !iterator.done() && iterator.code_offset() <= offset;
        iterator.Advance()) {
     position = iterator.source_position().ScriptOffset();
@@ -708,7 +714,8 @@ void Code::Disassemble(const char* name, std::ostream& os, Address current_pc) {
 
   {
     SourcePositionTableIterator it(
-        SourcePositionTable(), SourcePositionTableIterator::kJavaScriptOnly);
+        SourcePositionTableIfCollected(),
+        SourcePositionTableIterator::kJavaScriptOnly);
     if (!it.done()) {
       os << "Source positions:\n pc offset  position\n";
       for (; !it.done(); it.Advance()) {
@@ -721,7 +728,7 @@ void Code::Disassemble(const char* name, std::ostream& os, Address current_pc) {
   }
 
   {
-    SourcePositionTableIterator it(SourcePositionTable(),
+    SourcePositionTableIterator it(SourcePositionTableIfCollected(),
                                    SourcePositionTableIterator::kExternalOnly);
     if (!it.done()) {
       os << "External Source positions:\n pc offset  fileid  line\n";
@@ -804,7 +811,8 @@ void BytecodeArray::Disassemble(std::ostream& os) {
   os << "Frame size " << frame_size() << "\n";
 
   Address base_address = GetFirstBytecodeAddress();
-  SourcePositionTableIterator source_positions(SourcePositionTable());
+  SourcePositionTableIterator source_positions(
+      SourcePositionTableIfCollected());
 
   // Storage for backing the handle passed to the iterator. This handle won't be
   // updated by the gc, but that's ok because we've disallowed GCs anyway.

src/objects/code.h

@@ -89,6 +89,7 @@ class Code : public HeapObject {
   // SourcePositionTableWithFrameCache.
   DECL_ACCESSORS(source_position_table, Object)
   inline ByteArray SourcePositionTable() const;
+  inline ByteArray SourcePositionTableIfCollected() const;
 
   // [code_data_container]: A container indirection for all mutable fields.
   DECL_ACCESSORS(code_data_container, CodeDataContainer)
@@ -774,14 +775,33 @@ class BytecodeArray : public FixedArrayBase {
   // Accessors for handler table containing offsets of exception handlers.
   DECL_ACCESSORS(handler_table, ByteArray)
 
-  // Accessors for source position table containing mappings between byte code
-  // offset and source position or SourcePositionTableWithFrameCache.
+  // Accessors for source position table. Can contain:
+  // * undefined (initial value)
+  // * empty_byte_array (for bytecode generated for functions that will never
+  // have source positions, e.g. native functions).
+  // * ByteArray (when source positions have been collected for the bytecode)
+  // * SourcePositionTableWithFrameCache (as above but with a frame cache)
+  // * exception (when an error occurred while explicitly collecting source
+  // positions for pre-existing bytecode).
   DECL_ACCESSORS(source_position_table, Object)
 
-  inline ByteArray SourcePositionTable();
-  inline bool HasSourcePositionTable();
+  // This must only be called if source position collection has already been
+  // attempted. (If it failed because of an exception then it will return
+  // empty_byte_array).
+  inline ByteArray SourcePositionTable() const;
+  // If source positions have not been collected or an exception has been thrown
+  // this will return empty_byte_array.
+  inline ByteArray SourcePositionTableIfCollected() const;
+  inline bool HasSourcePositionTable() const;
+  inline bool DidSourcePositionGenerationFail() const;
   inline void ClearFrameCacheFromSourcePositionTable();
 
+  // Indicates that an attempt was made to collect source positions, but that it
+  // failed most likely due to stack exhaustion. When in this state
+  // |SourcePositionTable| will return an empty byte array rather than crashing
+  // as it would if no attempt was ever made to collect source positions.
+  inline void SetSourcePositionsFailedToCollect();
+
   DECL_CAST(BytecodeArray)
 
   // Dispatched behavior.

src/objects/script.h

@@ -133,7 +133,7 @@ class Script : public Struct {
   Object GetNameOrSourceURL();
 
   // Retrieve source position from where eval was called.
-  int GetEvalPosition();
+  static int GetEvalPosition(Isolate* isolate, Handle<Script> script);
 
   // Check if the script contains any Asm modules.
   bool ContainsAsmModule();

src/runtime/runtime-internal.cc

@@ -357,6 +357,7 @@ bool ComputeLocation(Isolate* isolate, MessageLocation* target) {
     auto& summary = frames.back().AsJavaScript();
     Handle<SharedFunctionInfo> shared(summary.function()->shared(), isolate);
     Handle<Object> script(shared->script(), isolate);
+    SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate, shared);
     int pos = summary.abstract_code()->SourcePosition(summary.code_offset());
     if (script->IsScript() &&
         !(Handle<Script>::cast(script)->source()->IsUndefined(isolate))) {

src/source-position-table.h

@@ -33,7 +33,16 @@ struct PositionTableEntry {
 
 class V8_EXPORT_PRIVATE SourcePositionTableBuilder {
  public:
-  enum RecordingMode { OMIT_SOURCE_POSITIONS, RECORD_SOURCE_POSITIONS };
+  enum RecordingMode {
+    // Indicates that source positions are never to be generated. (Resulting in
+    // an empty table).
+    OMIT_SOURCE_POSITIONS,
+    // Indicates that source positions are not currently required, but may be
+    // generated later.
+    LAZY_SOURCE_POSITIONS,
+    // Indicates that source positions should be immediately generated.
+    RECORD_SOURCE_POSITIONS
+  };
 
   explicit SourcePositionTableBuilder(
       RecordingMode mode = RECORD_SOURCE_POSITIONS);
@@ -44,7 +53,8 @@ class V8_EXPORT_PRIVATE SourcePositionTableBuilder {
   Handle<ByteArray> ToSourcePositionTable(Isolate* isolate);
   OwnedVector<byte> ToSourcePositionTableVector();
 
-  inline bool Omit() const { return mode_ == OMIT_SOURCE_POSITIONS; }
+  inline bool Omit() const { return mode_ != RECORD_SOURCE_POSITIONS; }
+  inline bool Lazy() const { return mode_ == LAZY_SOURCE_POSITIONS; }
 
  private:
   void AddEntry(const PositionTableEntry& entry);

src/unoptimized-compilation-info.cc

@@ -66,7 +66,7 @@ UnoptimizedCompilationInfo::SourcePositionRecordingMode() const {
   // compiled, e.g. class member initializer functions.
   return !literal_->AllowsLazyCompilation()
              ? SourcePositionTableBuilder::RECORD_SOURCE_POSITIONS
-             : SourcePositionTableBuilder::OMIT_SOURCE_POSITIONS;
+             : SourcePositionTableBuilder::LAZY_SOURCE_POSITIONS;
 }
 
 }  // namespace internal

test/cctest/cctest.status

@@ -552,8 +552,6 @@
 ##############################################################################
 ['lite_mode', {
   # TODO(v8:8510): Tests that currently fail with lazy source positions.
-  'test-cpu-profiler/TickLinesBaseline': [SKIP],
-  'test-cpu-profiler/TickLinesOptimized': [SKIP],
   'test-cpu-profiler/Inlining2': [SKIP],
 
   # TODO(mythria): Code logging tests that currently fail with lazy feedback

test/cctest/interpreter/test-interpreter.cc

@@ -5086,12 +5086,48 @@ TEST(InterpreterCollectSourcePositions) {
   Handle<SharedFunctionInfo> sfi = handle(function->shared(), isolate);
   Handle<BytecodeArray> bytecode_array =
       handle(sfi->GetBytecodeArray(), isolate);
+  CHECK(!bytecode_array->HasSourcePositionTable());
+
+  Compiler::CollectSourcePositions(isolate, sfi);
+
   ByteArray source_position_table = bytecode_array->SourcePositionTable();
-  CHECK_EQ(source_position_table->length(), 0);
+  CHECK(bytecode_array->HasSourcePositionTable());
+  CHECK_GT(source_position_table->length(), 0);
+}
+
+TEST(InterpreterCollectSourcePositions_StackOverflow) {
+  FLAG_enable_lazy_source_positions = true;
+  HandleAndZoneScope handles;
+  Isolate* isolate = handles.main_isolate();
+
+  const char* source =
+      "(function () {\n"
+      "  return 1;\n"
+      "})";
 
+  Handle<JSFunction> function = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
+      *v8::Local<v8::Function>::Cast(CompileRun(source))));
+
+  Handle<SharedFunctionInfo> sfi = handle(function->shared(), isolate);
+  Handle<BytecodeArray> bytecode_array =
+      handle(sfi->GetBytecodeArray(), isolate);
+  CHECK(!bytecode_array->HasSourcePositionTable());
+
+  // Make the stack limit the same as the current position so recompilation
+  // overflows.
+  uint64_t previous_limit = isolate->stack_guard()->real_climit();
+  isolate->stack_guard()->SetStackLimit(GetCurrentStackPosition());
   Compiler::CollectSourcePositions(isolate, sfi);
+  // Stack overflowed so source position table can be returned but is empty.
+  ByteArray source_position_table = bytecode_array->SourcePositionTable();
+  CHECK(!bytecode_array->HasSourcePositionTable());
+  CHECK_EQ(source_position_table->length(), 0);
 
+  // Reset the stack limit and try again.
+  isolate->stack_guard()->SetStackLimit(previous_limit);
+  Compiler::CollectSourcePositions(isolate, sfi);
   source_position_table = bytecode_array->SourcePositionTable();
+  CHECK(bytecode_array->HasSourcePositionTable());
   CHECK_GT(source_position_table->length(), 0);
 }
 
@@ -5130,8 +5166,7 @@ TEST(InterpreterCollectSourcePositions_GenerateStackTrace) {
   Handle<SharedFunctionInfo> sfi = handle(function->shared(), isolate);
   Handle<BytecodeArray> bytecode_array =
       handle(sfi->GetBytecodeArray(), isolate);
-  ByteArray source_position_table = bytecode_array->SourcePositionTable();
-  CHECK_EQ(source_position_table->length(), 0);
+  CHECK(!bytecode_array->HasSourcePositionTable());
 
   {
     Handle<Object> result =
@@ -5142,7 +5177,8 @@ TEST(InterpreterCollectSourcePositions_GenerateStackTrace) {
     CheckStringEqual("Error\n    at <anonymous>:4:17", result);
   }
 
-  source_position_table = bytecode_array->SourcePositionTable();
+  CHECK(bytecode_array->HasSourcePositionTable());
+  ByteArray source_position_table = bytecode_array->SourcePositionTable();
   CHECK_GT(source_position_table->length(), 0);
 }
 

test/cctest/test-cpu-profiler.cc

@@ -1154,6 +1154,11 @@ static void TickLines(bool optimize) {
                                   v8::base::TimeDelta::FromMicroseconds(100));
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
+  // TODO(delphick): Stop using the CpuProfiler internals here: This forces
+  // LogCompiledFunctions so that source positions are collected everywhere.
+  // This would normally happen automatically with CpuProfiler::StartProfiling
+  // but doesn't because it's constructed with a generator and a processor.
+  isolate->logger()->LogCompiledFunctions();
   processor->Start();
   ProfilerListener profiler_listener(isolate, processor);