Refactoring and bug fixes in the V8 API

cmake/targets/BuildBun.cmake

@@ -569,6 +569,7 @@ file(GLOB BUN_CXX_SOURCES ${CONFIGURE_DEPENDS}
   ${CWD}/src/bun.js/bindings/webcrypto/*.cpp
   ${CWD}/src/bun.js/bindings/webcrypto/*/*.cpp
   ${CWD}/src/bun.js/bindings/v8/*.cpp
+  ${CWD}/src/bun.js/bindings/v8/shim/*.cpp
   ${CWD}/src/kit/*.cpp
   ${CWD}/src/deps/*.cpp
   ${BUN_USOCKETS_SOURCE}/src/crypto/*.cpp
@@ -616,6 +617,7 @@ list(APPEND BUN_CPP_SOURCES
   ${BUN_C_SOURCES}
   ${BUN_CXX_SOURCES}
   ${VENDOR_PATH}/picohttpparser/picohttpparser.c
+  ${NODEJS_HEADERS_PATH}/include/node/node_version.h
   ${BUN_ZIG_GENERATED_CLASSES_OUTPUTS}
   ${BUN_JS_SINK_OUTPUTS}
   ${BUN_JAVASCRIPT_OUTPUTS}
@@ -693,13 +695,15 @@ target_include_directories(${bun} PRIVATE
   ${CWD}/src/bun.js/bindings/webcore
   ${CWD}/src/bun.js/bindings/webcrypto
   ${CWD}/src/bun.js/bindings/sqlite
+  ${CWD}/src/bun.js/bindings/v8
   ${CWD}/src/bun.js/modules
   ${CWD}/src/js/builtins
   ${CWD}/src/napi
   ${CWD}/src/deps
   ${CODEGEN_PATH}
   ${VENDOR_PATH}
   ${VENDOR_PATH}/picohttpparser
+  ${NODEJS_HEADERS_PATH}/include
 )
 
 # --- C/C++ Definitions ---

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -147,7 +147,7 @@
 #include "Base64Helpers.h"
 #include "wtf/text/OrdinalNumber.h"
 #include "ErrorCode.h"
-#include "v8/V8GlobalInternals.h"
+#include "v8/shim/GlobalInternals.h"
 #include "EventLoopTask.h"
 
 #if ENABLE(REMOTE_INSPECTOR)
@@ -2726,11 +2726,11 @@ void GlobalObject::finishCreation(VM& vm)
         });
 
     m_V8GlobalInternals.initLater(
-        [](const JSC::LazyProperty<JSC::JSGlobalObject, v8::GlobalInternals>::Initializer& init) {
+        [](const JSC::LazyProperty<JSC::JSGlobalObject, v8::shim::GlobalInternals>::Initializer& init) {
             init.set(
-                v8::GlobalInternals::create(
+                v8::shim::GlobalInternals::create(
                     init.vm,
-                    v8::GlobalInternals::createStructure(init.vm, init.owner),
+                    v8::shim::GlobalInternals::createStructure(init.vm, init.owner),
                     jsDynamicCast<Zig::GlobalObject*>(init.owner)));
         });
 

src/bun.js/bindings/ZigGlobalObject.h

@@ -33,7 +33,9 @@ class NapiHandleScopeImpl;
 } // namespace Bun
 
 namespace v8 {
+namespace shim {
 class GlobalInternals;
+} // namespace shim
 } // namespace v8
 
 #include "root.h"
@@ -289,7 +291,7 @@ class GlobalObject : public Bun::GlobalScope {
 
     Structure* JSSQLStatementStructure() const { return m_JSSQLStatementStructure.getInitializedOnMainThread(this); }
 
-    v8::GlobalInternals* V8GlobalInternals() const { return m_V8GlobalInternals.getInitializedOnMainThread(this); }
+    v8::shim::GlobalInternals* V8GlobalInternals() const { return m_V8GlobalInternals.getInitializedOnMainThread(this); }
 
     bool hasProcessObject() const { return m_processObject.isInitialized(); }
 
@@ -576,7 +578,7 @@ class GlobalObject : public Bun::GlobalScope {
     LazyProperty<JSGlobalObject, Structure> m_NapiHandleScopeImplStructure;
 
     LazyProperty<JSGlobalObject, Structure> m_JSSQLStatementStructure;
-    LazyProperty<JSGlobalObject, v8::GlobalInternals> m_V8GlobalInternals;
+    LazyProperty<JSGlobalObject, v8::shim::GlobalInternals> m_V8GlobalInternals;
 
     LazyProperty<JSGlobalObject, JSObject> m_bunObject;
     LazyProperty<JSGlobalObject, JSObject> m_cryptoObject;

src/bun.js/bindings/v8/V8Array.cpp

@@ -1,6 +1,9 @@
 #include "V8Array.h"
 
 #include "V8HandleScope.h"
+#include "v8_compatibility_assertions.h"
+
+ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Array)
 
 using JSC::ArrayAllocationProfile;
 using JSC::JSArray;
@@ -20,4 +23,4 @@ Local<Array> Array::New(Isolate* isolate, Local<Value>* elements, size_t length)
     return isolate->currentHandleScope()->createLocal<Array>(isolate->vm(), array);
 }
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Array.h

@@ -13,4 +13,4 @@ class Array : public Object {
     BUN_EXPORT static Local<Array> New(Isolate* isolate, Local<Value>* elements, size_t length);
 };
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Boolean.cpp

@@ -1,16 +1,26 @@
 #include "V8Boolean.h"
 #include "V8HandleScope.h"
+#include "v8_compatibility_assertions.h"
+
+ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Boolean)
 
 namespace v8 {
 
 bool Boolean::Value() const
 {
-    return localToJSValue(Isolate::GetCurrent()->globalInternals()).asBoolean();
+    JSC::JSValue jsv = localToOddball();
+    if (jsv.isTrue()) {
+        return true;
+    } else if (jsv.isFalse()) {
+        return false;
+    } else {
+        RELEASE_ASSERT_NOT_REACHED("non-Boolean passed to Boolean::Value");
+    }
 }
 
 Local<Boolean> Boolean::New(Isolate* isolate, bool value)
 {
     return isolate->currentHandleScope()->createLocal<Boolean>(isolate->vm(), JSC::jsBoolean(value));
 }
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Boolean.h

@@ -12,4 +12,4 @@ class Boolean : public Primitive {
     BUN_EXPORT static Local<Boolean> New(Isolate* isolate, bool value);
 };
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Context.cpp

@@ -1,10 +1,13 @@
 #include "V8Context.h"
+#include "v8_compatibility_assertions.h"
+
+ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Context)
 
 namespace v8 {
 
 Isolate* Context::GetIsolate()
 {
-    return reinterpret_cast<Isolate*>(localToPointer());
+    return globalObject()->V8GlobalInternals()->isolate();
 }
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Context.h

@@ -1,32 +1,33 @@
 #pragma once
 
 #include "ZigGlobalObject.h"
-#include "V8GlobalInternals.h"
 #include "V8Data.h"
 
 namespace v8 {
 
+namespace shim {
 class Isolate;
+}
 
-// Context is always a reinterpret pointer to V8::Roots, so that inlined V8 functions can find
-// values they expect to find at fixed offsets
+// Context is always a reinterpret pointer to Zig::GlobalObject, so that functions accepting a
+// Context can quickly access JSC data
 class Context : public Data {
 public:
     BUN_EXPORT Isolate* GetIsolate();
 
     JSC::VM& vm() const
     {
-        return globalObject()->vm();
+        return localToCell()->vm();
     }
 
     const Zig::GlobalObject* globalObject() const
     {
-        return reinterpret_cast<const Roots*>(localToCell())->parent->globalObject;
+        return JSC::jsDynamicCast<const Zig::GlobalObject*>(localToCell());
     }
 
     Zig::GlobalObject* globalObject()
     {
-        return reinterpret_cast<const Roots*>(localToCell())->parent->globalObject;
+        return JSC::jsDynamicCast<Zig::GlobalObject*>(localToCell());
     }
 
     HandleScope* currentHandleScope() const
@@ -35,4 +36,4 @@ class Context : public Data {
     };
 };
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8Data.h

@@ -1,30 +1,25 @@
 #pragma once
 
 #include "root.h"
-#include "V8TaggedPointer.h"
-#include "V8Handle.h"
-#include "V8GlobalInternals.h"
+#include "shim/TaggedPointer.h"
+#include "shim/Handle.h"
+#include "shim/GlobalInternals.h"
 
 namespace v8 {
 
 class Data {
+
 public:
     // Functions beginning with "localTo" must only be used when "this" comes from a v8::Local (i.e.
     // in public V8 functions), as they make assumptions about how V8 lays out local handles. They
     // will segfault or worse otherwise.
 
-    // Recover an opaque pointer out of a v8::Local which is not a number
-    void* localToPointer()
-    {
-        TaggedPointer tagged = localToTagged();
-        RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi);
-        return tagged.getPtr<void>();
-    }
-
     // Recover a JSCell pointer out of a v8::Local
     JSC::JSCell* localToCell()
     {
-        return reinterpret_cast<JSC::JSCell*>(localToPointer());
+        TaggedPointer root = localToTagged();
+        RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi);
+        return root.getPtr<shim::ObjectLayout>()->asCell();
     }
 
     // Recover a pointer to a JSCell subclass out of a v8::Local
@@ -35,47 +30,43 @@ class Data {
         return JSC::jsDynamicCast<T*>(localToCell());
     }
 
+    // Get this as a JSValue when this is a v8::Local containing a boolean, null, or undefined
+    JSC::JSValue localToOddball() const
+    {
+        TaggedPointer root = localToTagged();
+        shim::Oddball* oddball = root.getPtr<shim::Oddball>();
+        RELEASE_ASSERT(
+            oddball->m_map.getPtr<const shim::Map>()->m_instanceType == shim::InstanceType::Oddball);
+        return oddball->toJSValue();
+    }
+
     // Get this as a JSValue when this is a v8::Local
-    JSC::JSValue localToJSValue(GlobalInternals* globalInternals) const
+    JSC::JSValue localToJSValue() const
     {
-        TaggedPointer root = *reinterpret_cast<const TaggedPointer*>(this);
-        if (root.type() == TaggedPointer::Type::Smi) {
+        TaggedPointer root = localToTagged();
+        if (root.tag() == TaggedPointer::Tag::Smi) {
             return JSC::jsNumber(root.getSmiUnchecked());
         } else {
-            void* raw_ptr = root.getPtr<void>();
-            // check if this pointer is identical to the fixed locations where these primitive
-            // values are stored
-            if (raw_ptr == globalInternals->undefinedSlot()->getPtr<void>()) {
-                return JSC::jsUndefined();
-            } else if (raw_ptr == globalInternals->nullSlot()->getPtr<void>()) {
-                return JSC::jsNull();
-            } else if (raw_ptr == globalInternals->trueSlot()->getPtr<void>()) {
-                return JSC::jsBoolean(true);
-            } else if (raw_ptr == globalInternals->falseSlot()->getPtr<void>()) {
-                return JSC::jsBoolean(false);
-            }
+            using shim::InstanceType;
+            auto* v8_object = root.getPtr<shim::ObjectLayout>();
 
-            ObjectLayout* v8_object = reinterpret_cast<ObjectLayout*>(raw_ptr);
-            if (v8_object->map()->instance_type == InstanceType::HeapNumber) {
+            switch (v8_object->map()->m_instanceType) {
+            case InstanceType::Oddball:
+                return reinterpret_cast<shim::Oddball*>(v8_object)->toJSValue();
+            case InstanceType::HeapNumber:
                 return JSC::jsDoubleNumber(v8_object->asDouble());
-            } else {
-                return JSC::JSValue(v8_object->asCell());
+            default:
+                return v8_object->asCell();
             }
         }
     }
 
-    // Recover an opaque pointer out of a v8::Local which is not a number
-    const void* localToPointer() const
-    {
-        TaggedPointer tagged = localToTagged();
-        RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi);
-        return tagged.getPtr<const void>();
-    }
-
     // Recover a JSCell pointer out of a v8::Local
     const JSC::JSCell* localToCell() const
     {
-        return reinterpret_cast<const JSC::JSCell*>(localToPointer());
+        TaggedPointer root = localToTagged();
+        RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi);
+        return root.getPtr<shim::ObjectLayout>()->asCell();
     }
 
     // Recover a pointer to a JSCell subclass out of a v8::Local
@@ -87,20 +78,11 @@ class Data {
     }
 
 private:
-    // Convert the local handle into either a smi or a pointer to some non-V8 type.
+    // Convert the local handle into either a smi or an ObjectLayout pointer.
     TaggedPointer localToTagged() const
     {
-        TaggedPointer root = *reinterpret_cast<const TaggedPointer*>(this);
-        if (root.type() == TaggedPointer::Type::Smi) {
-            return root;
-        } else {
-            // root points to the V8 object. The first field of the V8 object is the map, and the
-            // second is a pointer to some object we have stored. So we ignore the map and recover
-            // the object pointer.
-            ObjectLayout* v8_object = root.getPtr<ObjectLayout>();
-            return TaggedPointer(v8_object->asRaw());
-        }
+        return *reinterpret_cast<const TaggedPointer*>(this);
     }
 };
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8EscapableHandleScope.cpp

@@ -1,4 +1,7 @@
 #include "V8EscapableHandleScope.h"
+#include "v8_compatibility_assertions.h"
+
+ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::EscapableHandleScope)
 
 namespace v8 {
 
@@ -9,7 +12,6 @@ EscapableHandleScope::EscapableHandleScope(Isolate* isolate)
 
 EscapableHandleScope::~EscapableHandleScope()
 {
-    EscapableHandleScopeBase::~EscapableHandleScopeBase();
 }
 
-}
+} // namespace v8

src/bun.js/bindings/v8/V8EscapableHandleScope.h

@@ -10,4 +10,6 @@ class EscapableHandleScope : public EscapableHandleScopeBase {
     BUN_EXPORT ~EscapableHandleScope();
 };
 
-}
+static_assert(sizeof(EscapableHandleScope) == 32, "EscapableHandleScope has wrong layout");
+
+} // namespace v8

src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp

@@ -1,4 +1,7 @@
 #include "V8EscapableHandleScopeBase.h"
+#include "v8_compatibility_assertions.h"
+
+ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::EscapableHandleScopeBase)
 
 namespace v8 {
 
@@ -7,16 +10,21 @@ EscapableHandleScopeBase::EscapableHandleScopeBase(Isolate* isolate)
 {
     // at this point isolate->currentHandleScope() would just be this, so instead we have to get the
     // previous one
-    auto& handle = prev->buffer->createEmptyHandle();
-    escape_slot = &handle;
+    auto& handle = m_previousHandleScope->m_buffer->createEmptyHandle();
+    m_escapeSlot = &handle;
 }
 
 // Store the handle escape_value in the escape slot that we have allocated from the parent
 // HandleScope, and return the escape slot
 uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value)
 {
-    *escape_slot = *reinterpret_cast<Handle*>(escape_value);
-    return &escape_slot->to_v8_object.value;
+    RELEASE_ASSERT(m_escapeSlot != nullptr, "EscapableHandleScope::Escape called multiple times");
+    TaggedPointer* newHandle = m_previousHandleScope->m_buffer->createHandleFromExistingObject(
+        TaggedPointer::fromRaw(*escape_value),
+        m_isolate,
+        m_escapeSlot);
+    m_escapeSlot = nullptr;
+    return newHandle->asRawPtrLocation();
 }
 
-}
+} // namespace v8