[compiler] Don't collect source positions when throwing exceptions

While crrev.com/c/1520721 tried to avoid collecting source positions
when throw exceptions, it failed because they were still collected in
Isolate::CaptureStackTrace.

This removes that collection point and lets SetStackFrameCacheCommon
bail out when trying to set the stack frame cache for a bytecode that
doesn't have source positions.

It also adds tests that ensure source positions are not collected when
an exception is thrown (although one is disabled as it does not yet
work).

Bug: v8:8510
Change-Id: Id5caf579dda549d637fa9b3129c419d524be5ff2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1565898
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60847}

src/isolate.cc

@@ -1013,7 +1013,6 @@ Handle<Object> CaptureStackTrace(Isolate* isolate, Handle<Object> caller,
               !summary.is_subject_to_debugging()) {
             continue;
           }
-          summary.EnsureSourcePositionsAvailable();
 
           if (summary.IsJavaScript()) {
             //=========================================================

src/objects/code.cc

@@ -162,13 +162,12 @@ 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->IsException(isolate) || maybe_table->IsUndefined()) return;
   if (maybe_table->IsSourcePositionTableWithFrameCache()) {
     Handle<SourcePositionTableWithFrameCache>::cast(maybe_table)
         ->set_stack_frame_cache(*cache);
     return;
   }
-  DCHECK(!maybe_table->IsUndefined(isolate));
   DCHECK(maybe_table->IsByteArray());
   Handle<ByteArray> table(Handle<ByteArray>::cast(maybe_table));
   Handle<SourcePositionTableWithFrameCache> table_with_cache =

test/cctest/interpreter/test-interpreter.cc

@@ -5131,6 +5131,80 @@ TEST(InterpreterCollectSourcePositions_StackOverflow) {
   CHECK_GT(source_position_table->length(), 0);
 }
 
+// TODO(v8:8510): When an exception is thrown, the top frame still has its
+// source positions collected. Re-enable this test when that is fixed.
+DISABLED_TEST(InterpreterCollectSourcePositions_ThrowFrom1stFrame) {
+  FLAG_enable_lazy_source_positions = true;
+  HandleAndZoneScope handles;
+  Isolate* isolate = handles.main_isolate();
+
+  const char* source =
+      R"javascript(
+      (function () {
+        throw new Error();
+      });
+      )javascript";
+
+  Handle<JSFunction> function = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
+      *v8::Local<v8::Function>::Cast(CompileRun(source))));
+
+  Handle<SharedFunctionInfo> sfi = handle(function->shared(), isolate);
+  // This is the bytecode for the top-level iife.
+  Handle<BytecodeArray> bytecode_array =
+      handle(sfi->GetBytecodeArray(), isolate);
+  CHECK(!bytecode_array->HasSourcePositionTable());
+
+  {
+    v8::TryCatch try_catch(CcTest::isolate());
+    MaybeHandle<Object> result = Execution::Call(
+        isolate, function, ReadOnlyRoots(isolate).undefined_value_handle(), 0,
+        nullptr);
+    CHECK(result.is_null());
+    CHECK(try_catch.HasCaught());
+  }
+
+  // The exception was caught but source positions were not retrieved from it so
+  // there should be no source position table.
+  CHECK(!bytecode_array->HasSourcePositionTable());
+}
+
+TEST(InterpreterCollectSourcePositions_ThrowFrom2ndFrame) {
+  FLAG_enable_lazy_source_positions = true;
+  HandleAndZoneScope handles;
+  Isolate* isolate = handles.main_isolate();
+
+  const char* source =
+      R"javascript(
+      (function () {
+        (function () {
+          throw new Error();
+        })();
+      });
+      )javascript";
+
+  Handle<JSFunction> function = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
+      *v8::Local<v8::Function>::Cast(CompileRun(source))));
+
+  Handle<SharedFunctionInfo> sfi = handle(function->shared(), isolate);
+  // This is the bytecode for the top-level iife.
+  Handle<BytecodeArray> bytecode_array =
+      handle(sfi->GetBytecodeArray(), isolate);
+  CHECK(!bytecode_array->HasSourcePositionTable());
+
+  {
+    v8::TryCatch try_catch(CcTest::isolate());
+    MaybeHandle<Object> result = Execution::Call(
+        isolate, function, ReadOnlyRoots(isolate).undefined_value_handle(), 0,
+        nullptr);
+    CHECK(result.is_null());
+    CHECK(try_catch.HasCaught());
+  }
+
+  // The exception was caught but source positions were not retrieved from it so
+  // there should be no source position table.
+  CHECK(!bytecode_array->HasSourcePositionTable());
+}
+
 namespace {
 
 void CheckStringEqual(const char* expected_ptr, Handle<Object> actual_handle) {