[debug] Add new 'CreateMessageFromException' function

CDP has a "ExceptionDetails" structure that is attached to various
CDP commands, e.g. "Runtime#exceptionThrown" or "Runtime#evaluate".
The stack trace in the "ExceptionDetails" structure is used in
various places in DevTools. The information in the "ExceptionDetails"
structure is extracted from a v8::Message object. Message objects
are normally created at the exception throw site and may augment
the error with manually inspecting the stack (both to capture a fresh
stack trace in some cases, as well as to calculate location info).

The problem is that in some cases we want to get an "ExceptionDetails"
structure after the fact, e.g. when logging a JS "Error" object in
a catch block. This means we can't reuse Isolate::CreateMessage as
the JS stack at call time is unrelated to the time when an Error
object was thrown.

To re-use some of the code, this CL introduces a new
"CreateMessageFromException" method that is only available from the
debugging interface (not public V8 API!). The new method works
similar to Isolate::CreateMessage, but:
  1) Does not look at the current JS stack, neither for a fresh
     stack trace nor for location information.
  2) Only uses the "detailed" stack trace for location info.
     This is because the "simple" stack trace could have already
     been serialized by accessing Error#stack.

Bug: chromium:1278650
Doc: https://bit.ly/runtime-get-exception-details
Change-Id: I0144516001c71786b9f76ae4dec4442fa1468c5b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3337257Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78586}

src/common/message-template.h

@@ -18,6 +18,7 @@ namespace internal {
   T(DebuggerLoading, "Error loading debugger")                                 \
   T(DefaultOptionsMissing, "Internal % error. Default options are missing.")   \
   T(DeletePrivateField, "Private fields can not be deleted")                   \
+  T(PlaceholderOnly, "%")                                                      \
   T(UncaughtException, "Uncaught %")                                           \
   T(Unsupported, "Not supported")                                              \
   T(WrongServiceType, "Internal error, wrong service type: %")                 \

src/debug/debug-interface.cc

@@ -878,6 +878,16 @@ ConsoleCallArguments::ConsoleCallArguments(
           args.length() > 1 ? args.address_of_first_argument() : nullptr,
           args.length() - 1) {}
 
+v8::Local<v8::Message> CreateMessageFromException(
+    Isolate* v8_isolate, v8::Local<v8::Value> v8_error) {
+  i::Handle<i::Object> obj = Utils::OpenHandle(*v8_error);
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
+  i::HandleScope scope(isolate);
+  return Utils::MessageToLocal(
+      scope.CloseAndEscape(isolate->CreateMessageFromException(obj)));
+}
+
 MaybeLocal<Script> GeneratorObject::Script() {
   i::Handle<i::JSGeneratorObject> obj = Utils::OpenHandle(this);
   i::Object maybe_script = obj->function().shared().script();

src/debug/debug-interface.h

@@ -284,6 +284,9 @@ Local<Function> GetBuiltin(Isolate* isolate, Builtin builtin);
 V8_EXPORT_PRIVATE void SetConsoleDelegate(Isolate* isolate,
                                           ConsoleDelegate* delegate);
 
+V8_EXPORT_PRIVATE v8::Local<v8::Message> CreateMessageFromException(
+    Isolate* isolate, v8::Local<v8::Value> error);
+
 /**
  * Native wrapper around v8::internal::JSGeneratorObject object.
  */

src/execution/isolate.cc

@@ -2326,8 +2326,8 @@ bool Isolate::ComputeLocationFromException(MessageLocation* target,
   return true;
 }
 
-bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
-                                            Handle<Object> exception) {
+bool Isolate::ComputeLocationFromSimpleStackTrace(MessageLocation* target,
+                                                  Handle<Object> exception) {
   if (!exception->IsJSReceiver()) {
     return false;
   }
@@ -2343,6 +2343,23 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
   return false;
 }
 
+bool Isolate::ComputeLocationFromDetailedStackTrace(MessageLocation* target,
+                                                    Handle<Object> exception) {
+  if (!exception->IsJSReceiver()) return false;
+
+  Handle<FixedArray> stack_frame_infos =
+      GetDetailedStackTrace(Handle<JSReceiver>::cast(exception));
+  if (stack_frame_infos.is_null() || stack_frame_infos->length() == 0) {
+    return false;
+  }
+
+  Handle<StackFrameInfo> info(StackFrameInfo::cast(stack_frame_infos->get(0)),
+                              this);
+  const int pos = StackFrameInfo::GetSourcePosition(info);
+  *target = MessageLocation(handle(info->script(), this), pos, pos + 1);
+  return true;
+}
+
 Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
                                                MessageLocation* location) {
   Handle<FixedArray> stack_trace_object;
@@ -2365,7 +2382,7 @@ Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
   MessageLocation computed_location;
   if (location == nullptr &&
       (ComputeLocationFromException(&computed_location, exception) ||
-       ComputeLocationFromStackTrace(&computed_location, exception) ||
+       ComputeLocationFromSimpleStackTrace(&computed_location, exception) ||
        ComputeLocation(&computed_location))) {
     location = &computed_location;
   }
@@ -2375,6 +2392,26 @@ Handle<JSMessageObject> Isolate::CreateMessage(Handle<Object> exception,
       stack_trace_object);
 }
 
+Handle<JSMessageObject> Isolate::CreateMessageFromException(
+    Handle<Object> exception) {
+  Handle<FixedArray> stack_trace_object;
+  if (exception->IsJSError()) {
+    stack_trace_object =
+        GetDetailedStackTrace(Handle<JSObject>::cast(exception));
+  }
+
+  MessageLocation* location = nullptr;
+  MessageLocation computed_location;
+  if (ComputeLocationFromException(&computed_location, exception) ||
+      ComputeLocationFromDetailedStackTrace(&computed_location, exception)) {
+    location = &computed_location;
+  }
+
+  return MessageHandler::MakeMessageObject(
+      this, MessageTemplate::kPlaceholderOnly, location, exception,
+      stack_trace_object);
+}
+
 Isolate::ExceptionHandlerType Isolate::TopExceptionHandlerType(
     Object exception) {
   DCHECK_NE(ReadOnlyRoots(heap()).the_hole_value(), exception);

src/execution/isolate.h

@@ -992,13 +992,19 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   bool ComputeLocation(MessageLocation* target);
   bool ComputeLocationFromException(MessageLocation* target,
                                     Handle<Object> exception);
-  bool ComputeLocationFromStackTrace(MessageLocation* target,
-                                     Handle<Object> exception);
+  bool ComputeLocationFromSimpleStackTrace(MessageLocation* target,
+                                           Handle<Object> exception);
+  bool ComputeLocationFromDetailedStackTrace(MessageLocation* target,
+                                             Handle<Object> exception);
 
   Handle<JSMessageObject> CreateMessage(Handle<Object> exception,
                                         MessageLocation* location);
   Handle<JSMessageObject> CreateMessageOrAbort(Handle<Object> exception,
                                                MessageLocation* location);
+  // Similar to Isolate::CreateMessage but DOESN'T inspect the JS stack and
+  // only looks at the "detailed stack trace" as the "simple stack trace" might
+  // have already been stringified.
+  Handle<JSMessageObject> CreateMessageFromException(Handle<Object> exception);
 
   // Out of resource exception helpers.
   Object StackOverflow();

test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

@@ -83,7 +83,7 @@ bytecodes: [
   /*   48 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   53 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   58 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(290),
+                B(Wide), B(LdaSmi), I16(291),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -115,7 +115,7 @@ bytecodes: [
   /*   41 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   46 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   51 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(289),
+                B(Wide), B(LdaSmi), I16(290),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -147,7 +147,7 @@ bytecodes: [
   /*   48 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   53 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   58 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(290),
+                B(Wide), B(LdaSmi), I16(291),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -179,7 +179,7 @@ bytecodes: [
   /*   41 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   46 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   51 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(289),
+                B(Wide), B(LdaSmi), I16(290),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),

test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden

@@ -56,7 +56,7 @@ bytecodes: [
   /*   44 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   49 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   54 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(288),
+                B(Wide), B(LdaSmi), I16(289),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -89,7 +89,7 @@ bytecodes: [
   /*   44 E> */ B(StaKeyedPropertyAsDefine), R(this), R(0), U8(0),
   /*   49 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   54 E> */ B(LdaKeyedProperty), R(this), U8(2),
-                B(Wide), B(LdaSmi), I16(288),
+                B(Wide), B(LdaSmi), I16(289),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),

test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden

@@ -24,7 +24,7 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(1),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -59,13 +59,13 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),
   /*   61 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(1), U8(2),
                 B(Throw),
-                B(Wide), B(LdaSmi), I16(288),
+                B(Wide), B(LdaSmi), I16(289),
                 B(Star3),
                 B(LdaConstant), U8(1),
                 B(Star4),
@@ -97,13 +97,13 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),
   /*   61 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(1), U8(2),
                 B(Throw),
-                B(Wide), B(LdaSmi), I16(288),
+                B(Wide), B(LdaSmi), I16(289),
                 B(Star3),
                 B(LdaConstant), U8(1),
                 B(Star4),
@@ -143,7 +143,7 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -165,7 +165,7 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star3),
                 B(LdaConstant), U8(0),
                 B(Star4),
@@ -180,7 +180,7 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star2),
                 B(LdaConstant), U8(0),
                 B(Star3),
@@ -214,13 +214,13 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),
   /*   65 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(1), U8(2),
                 B(Throw),
-                B(Wide), B(LdaSmi), I16(290),
+                B(Wide), B(LdaSmi), I16(291),
                 B(Star3),
                 B(LdaConstant), U8(1),
                 B(Star4),
@@ -251,13 +251,13 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),
   /*   58 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(1), U8(2),
                 B(Throw),
-                B(Wide), B(LdaSmi), I16(289),
+                B(Wide), B(LdaSmi), I16(290),
                 B(Star3),
                 B(LdaConstant), U8(1),
                 B(Star4),
@@ -288,13 +288,13 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(16),
-                B(Wide), B(LdaSmi), I16(282),
+                B(Wide), B(LdaSmi), I16(283),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),
   /*   65 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(1), U8(2),
                 B(Throw),
-                B(Wide), B(LdaSmi), I16(290),
+                B(Wide), B(LdaSmi), I16(291),
                 B(Star3),
                 B(LdaConstant), U8(1),
                 B(Star4),
@@ -323,7 +323,7 @@ bytecode array length: 19
 bytecodes: [
   /*   46 S> */ B(LdaImmutableCurrentContextSlot), U8(3),
   /*   51 E> */ B(LdaKeyedProperty), R(this), U8(0),
-                B(Wide), B(LdaSmi), I16(289),
+                B(Wide), B(LdaSmi), I16(290),
                 B(Star1),
                 B(LdaConstant), U8(0),
                 B(Star2),

test/cctest/test-debug.cc

@@ -5725,3 +5725,69 @@ TEST(AwaitCleansUpGlobalPromiseStack) {
   v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded();
 }
+
+TEST(CreateMessageFromOldException) {
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+
+  context->GetIsolate()->SetCaptureStackTraceForUncaughtExceptions(true);
+
+  v8::Local<v8::Value> error;
+  {
+    v8::TryCatch try_catch(context->GetIsolate());
+    CompileRun(R"javascript(
+        function f1() {
+          throw new Error('error in f1');
+        };
+        f1();
+    )javascript");
+    CHECK(try_catch.HasCaught());
+
+    error = try_catch.Exception();
+  }
+  CHECK(error->IsObject());
+
+  v8::Local<v8::Message> message =
+      v8::debug::CreateMessageFromException(context->GetIsolate(), error);
+  CHECK(!message.IsEmpty());
+  CHECK_EQ(3, message->GetLineNumber(context.local()).FromJust());
+  CHECK_EQ(16, message->GetStartColumn(context.local()).FromJust());
+
+  v8::Local<v8::StackTrace> stackTrace = message->GetStackTrace();
+  CHECK(!stackTrace.IsEmpty());
+  CHECK_EQ(2, stackTrace->GetFrameCount());
+
+  stackTrace = v8::Exception::GetStackTrace(error);
+  CHECK(!stackTrace.IsEmpty());
+  CHECK_EQ(2, stackTrace->GetFrameCount());
+}
+
+TEST(CreateMessageDoesNotInspectStack) {
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+
+  // Do not enable Isolate::SetCaptureStackTraceForUncaughtExceptions.
+
+  v8::Local<v8::Value> error;
+  {
+    v8::TryCatch try_catch(context->GetIsolate());
+    CompileRun(R"javascript(
+        function f1() {
+          throw new Error('error in f1');
+        };
+        f1();
+    )javascript");
+    CHECK(try_catch.HasCaught());
+
+    error = try_catch.Exception();
+  }
+  // The caught error should not have a stack trace attached.
+  CHECK(error->IsObject());
+  CHECK(v8::Exception::GetStackTrace(error).IsEmpty());
+
+  // The corresponding message should also not have a stack trace.
+  v8::Local<v8::Message> message =
+      v8::debug::CreateMessageFromException(context->GetIsolate(), error);
+  CHECK(!message.IsEmpty());
+  CHECK(message->GetStackTrace().IsEmpty());
+}

test/cctest/wasm/test-wasm-stack.cc

@@ -91,7 +91,7 @@ void CheckComputeLocation(v8::internal::Isolate* i_isolate, Handle<Object> exc,
                           const ExceptionInfo& topLocation,
                           const v8::Local<v8::StackFrame> stackFrame) {
   MessageLocation loc;
-  CHECK(i_isolate->ComputeLocationFromStackTrace(&loc, exc));
+  CHECK(i_isolate->ComputeLocationFromSimpleStackTrace(&loc, exc));
   printf("loc start: %d, end: %d\n", loc.start_pos(), loc.end_pos());
   Handle<JSMessageObject> message = i_isolate->CreateMessage(exc, nullptr);
   printf("msg start: %d, end: %d, line: %d, col: %d\n",