[runtime] DHECK that builtins correctly return the exception object

Runtime and Builtin function should always return the exception object
as a marker if there is a pending_exception on the current isolate.

Change-Id: I7c255aa501800384c288664a9ca6578afbe0a103
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3610449Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80237}

src/builtins/builtins-utils.h

@@ -89,7 +89,7 @@ class BuiltinArguments : public JavaScriptArguments {
     RCS_SCOPE(isolate, RuntimeCallCounterId::kBuiltin_##name);              \
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.runtime"),                   \
                  "V8.Builtin_" #name);                                      \
-    return CONVERT_OBJECT(Builtin_Impl_##name(args, isolate));              \
+    return BUILTIN_CONVERT_RESULT(Builtin_Impl_##name(args, isolate));      \
   }                                                                         \
                                                                             \
   V8_WARN_UNUSED_RESULT Address Builtin_##name(                             \
@@ -99,7 +99,7 @@ class BuiltinArguments : public JavaScriptArguments {
       return Builtin_Impl_Stats_##name(args_length, args_object, isolate);  \
     }                                                                       \
     BuiltinArguments args(args_length, args_object);                        \
-    return CONVERT_OBJECT(Builtin_Impl_##name(args, isolate));              \
+    return BUILTIN_CONVERT_RESULT(Builtin_Impl_##name(args, isolate));      \
   }                                                                         \
                                                                             \
   V8_WARN_UNUSED_RESULT static Object Builtin_Impl_##name(                  \
@@ -113,7 +113,7 @@ class BuiltinArguments : public JavaScriptArguments {
       int args_length, Address* args_object, Isolate* isolate) {            \
     DCHECK(isolate->context().is_null() || isolate->context().IsContext()); \
     BuiltinArguments args(args_length, args_object);                        \
-    return CONVERT_OBJECT(Builtin_Impl_##name(args, isolate));              \
+    return BUILTIN_CONVERT_RESULT(Builtin_Impl_##name(args, isolate));      \
   }                                                                         \
                                                                             \
   V8_WARN_UNUSED_RESULT static Object Builtin_Impl_##name(                  \

src/execution/arguments.h

@@ -151,15 +151,20 @@ Handle<S> Arguments<T>::at(int index) const {
                                                                             \
   static InternalType __RT_impl_##Name(RuntimeArguments args, Isolate* isolate)
 
-#define CONVERT_OBJECT(x) (x).ptr()
-#define CONVERT_OBJECTPAIR(x) (x)
+#ifdef DEBUG
+#define BUILTIN_CONVERT_RESULT(x) (isolate->VerifyBuiltinsResult(x)).ptr()
+#define BUILTIN_CONVERT_RESULT_PAIR(x) isolate->VerifyBuiltinsResult(x)
+#else  // DEBUG
+#define BUILTIN_CONVERT_RESULT(x) (x).ptr()
+#define BUILTIN_CONVERT_RESULT_PAIR(x) (x)
+#endif  // DEBUG
 
 #define RUNTIME_FUNCTION(Name) \
-  RUNTIME_FUNCTION_RETURNS_TYPE(Address, Object, CONVERT_OBJECT, Name)
+  RUNTIME_FUNCTION_RETURNS_TYPE(Address, Object, BUILTIN_CONVERT_RESULT, Name)
 
-#define RUNTIME_FUNCTION_RETURN_PAIR(Name)                                  \
-  RUNTIME_FUNCTION_RETURNS_TYPE(ObjectPair, ObjectPair, CONVERT_OBJECTPAIR, \
-                                Name)
+#define RUNTIME_FUNCTION_RETURN_PAIR(Name)              \
+  RUNTIME_FUNCTION_RETURNS_TYPE(ObjectPair, ObjectPair, \
+                                BUILTIN_CONVERT_RESULT_PAIR, Name)
 
 }  // namespace internal
 }  // namespace v8

src/execution/isolate-inl.h

@@ -15,6 +15,10 @@
 #include "src/objects/shared-function-info.h"
 #include "src/objects/source-text-module-inl.h"
 
+#ifdef DEBUG
+#include "src/runtime/runtime-utils.h"
+#endif
+
 namespace v8 {
 namespace internal {
 
@@ -92,6 +96,24 @@ void Isolate::set_scheduled_exception(Object exception) {
   thread_local_top()->scheduled_exception_ = exception;
 }
 
+#ifdef DEBUG
+Object Isolate::VerifyBuiltinsResult(Object result) {
+  if (has_pending_exception()) {
+    CHECK_EQ(result, ReadOnlyRoots(this).exception());
+  }
+  return result;
+}
+
+ObjectPair Isolate::VerifyBuiltinsResult(ObjectPair pair) {
+#ifdef V8_HOST_ARCH_64_BIT
+  if (has_pending_exception()) {
+    CHECK(pair.x == ReadOnlyRoots(this).exception().ptr());
+  }
+#endif  // V8_HOST_ARCH_64_BIT
+  return pair;
+}
+#endif  // DEBUG
+
 bool Isolate::is_catchable_by_javascript(Object exception) {
   return exception != ReadOnlyRoots(heap()).termination_exception();
 }

src/execution/isolate.h

@@ -44,6 +44,10 @@
 #include "src/strings/unicode.h"
 #include "src/utils/allocation.h"
 
+#ifdef DEBUG
+#include "src/runtime/runtime-utils.h"
+#endif
+
 #ifdef V8_INTL_SUPPORT
 #include "unicode/uversion.h"  // Define U_ICU_NAMESPACE.
 namespace U_ICU_NAMESPACE {
@@ -791,6 +795,11 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   inline void clear_scheduled_exception();
   inline void set_scheduled_exception(Object exception);
 
+#ifdef DEBUG
+  inline Object VerifyBuiltinsResult(Object result);
+  inline ObjectPair VerifyBuiltinsResult(ObjectPair pair);
+#endif
+
   enum class ExceptionHandlerType {
     kJavaScriptHandler,
     kExternalTryCatch,