From fe61bf65f50f1c591f1aeb3a914c01332e5ac6d3 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 29 Aug 2024 17:43:34 -0700 Subject: [PATCH 01/30] Remove unused TaggedPointer function --- src/bun.js/bindings/v8/V8TaggedPointer.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/bun.js/bindings/v8/V8TaggedPointer.h b/src/bun.js/bindings/v8/V8TaggedPointer.h index 910adb2845a14..3d1d0d2b5d69d 100644 --- a/src/bun.js/bindings/v8/V8TaggedPointer.h +++ b/src/bun.js/bindings/v8/V8TaggedPointer.h @@ -1,6 +1,6 @@ #pragma once -#include "v8.h" +#include namespace v8 { @@ -78,15 +78,6 @@ struct TaggedPointer { ASSERT(type() == Type::Smi); return static_cast(value >> 32); } - - JSC::JSValue getJSValue() const - { - int32_t smi; - if (getSmi(smi)) { - return JSC::jsNumber(smi); - } - return getPtr(); - } }; } From 32035428f6daf44f742ac9b6ac01bf15ef73bb86 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 29 Aug 2024 18:25:40 -0700 Subject: [PATCH 02/30] Start changing Context to be a Zig::GlobalObject --- src/bun.js/bindings/v8/V8Context.cpp | 2 +- src/bun.js/bindings/v8/V8Context.h | 8 ++++---- src/bun.js/bindings/v8/V8Data.h | 1 + src/bun.js/bindings/v8/V8Handle.h | 17 +---------------- src/bun.js/bindings/v8/V8HandleScope.h | 6 ------ src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp | 7 ------- src/bun.js/bindings/v8/V8Isolate.cpp | 4 +++- src/bun.js/bindings/v8/V8TaggedPointer.h | 2 +- 8 files changed, 11 insertions(+), 36 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Context.cpp b/src/bun.js/bindings/v8/V8Context.cpp index 1fe001f237048..c0ab6047284e9 100644 --- a/src/bun.js/bindings/v8/V8Context.cpp +++ b/src/bun.js/bindings/v8/V8Context.cpp @@ -4,7 +4,7 @@ namespace v8 { Isolate* Context::GetIsolate() { - return reinterpret_cast(localToPointer()); + return reinterpret_cast(&globalObject()->V8GlobalInternals()->roots); } } diff --git a/src/bun.js/bindings/v8/V8Context.h b/src/bun.js/bindings/v8/V8Context.h index 2293a22516ff2..3b8839849158e 100644 --- a/src/bun.js/bindings/v8/V8Context.h +++ b/src/bun.js/bindings/v8/V8Context.h @@ -8,8 +8,8 @@ namespace v8 { 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(); @@ -21,12 +21,12 @@ class Context : public Data { const Zig::GlobalObject* globalObject() const { - return reinterpret_cast(localToCell())->parent->globalObject; + return JSC::jsDynamicCast(localToCell()); } Zig::GlobalObject* globalObject() { - return reinterpret_cast(localToCell())->parent->globalObject; + return JSC::jsDynamicCast(localToCell()); } HandleScope* currentHandleScope() const diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index aded85f5c9bba..d47e11d1f479f 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -98,6 +98,7 @@ class Data { // 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(); + // REFACTOR return TaggedPointer(v8_object->asRaw()); } } diff --git a/src/bun.js/bindings/v8/V8Handle.h b/src/bun.js/bindings/v8/V8Handle.h index 709481e95e4bb..c89cf6b1f2947 100644 --- a/src/bun.js/bindings/v8/V8Handle.h +++ b/src/bun.js/bindings/v8/V8Handle.h @@ -11,7 +11,6 @@ class ObjectLayout { union { JSC::WriteBarrier cell; double number; - void* raw; } contents; public: @@ -19,7 +18,7 @@ class ObjectLayout { // using a smi value for map is most likely to catch bugs as almost every access will expect // map to be a pointer (and even if the assertion is bypassed, it'll be a null pointer) : tagged_map(0) - , contents({ .raw = nullptr }) + , contents({ .cell = {} }) { } @@ -35,20 +34,12 @@ class ObjectLayout { { } - ObjectLayout(void* raw) - : tagged_map(const_cast(&Map::raw_ptr_map)) - , contents({ .raw = raw }) - { - } - const Map* map() const { return tagged_map.getPtr(); } double asDouble() const { return contents.number; } JSC::JSCell* asCell() const { return contents.cell.get(); } - void* asRaw() const { return contents.raw; } - friend class Handle; friend class HandleScopeBuffer; }; @@ -83,12 +74,6 @@ struct Handle { { } - Handle(void* raw) - : to_v8_object(&this->object) - , object(raw) - { - } - Handle(int32_t smi) : to_v8_object(smi) , object() diff --git a/src/bun.js/bindings/v8/V8HandleScope.h b/src/bun.js/bindings/v8/V8HandleScope.h index 81f73e08ee1a8..b32eb05a05cbc 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.h +++ b/src/bun.js/bindings/v8/V8HandleScope.h @@ -39,12 +39,6 @@ class HandleScope { } } - template Local createRawLocal(void* ptr) - { - TaggedPointer* handle = buffer->createRawHandle(ptr); - return Local(handle); - } - friend class EscapableHandleScopeBase; protected: diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp index 8468cc64674d5..3f7e793f57238 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp @@ -52,13 +52,6 @@ TaggedPointer* HandleScopeBuffer::createHandle(JSCell* ptr, const Map* map, JSC: return &handle.to_v8_object; } -TaggedPointer* HandleScopeBuffer::createRawHandle(void* ptr) -{ - auto& handle = createEmptyHandle(); - handle = Handle(ptr); - return &handle.to_v8_object; -} - TaggedPointer* HandleScopeBuffer::createSmiHandle(int32_t smi) { auto& handle = createEmptyHandle(); diff --git a/src/bun.js/bindings/v8/V8Isolate.cpp b/src/bun.js/bindings/v8/V8Isolate.cpp index 80e173814a4a9..537463d02d0ac 100644 --- a/src/bun.js/bindings/v8/V8Isolate.cpp +++ b/src/bun.js/bindings/v8/V8Isolate.cpp @@ -21,7 +21,9 @@ Isolate* Isolate::GetCurrent() Local Isolate::GetCurrentContext() { - return currentHandleScope()->createRawLocal(this); + auto* globalInternals = reinterpret_cast(this)->parent; + auto* globalObject = globalInternals->globalObject; + return globalInternals->currentHandleScope()->createLocal(globalObject->vm(), globalObject); } } diff --git a/src/bun.js/bindings/v8/V8TaggedPointer.h b/src/bun.js/bindings/v8/V8TaggedPointer.h index 3d1d0d2b5d69d..370b67bbe9c32 100644 --- a/src/bun.js/bindings/v8/V8TaggedPointer.h +++ b/src/bun.js/bindings/v8/V8TaggedPointer.h @@ -1,6 +1,6 @@ #pragma once -#include +#include "v8.h" namespace v8 { From 9ef834673099907fc1ebe35d9ab4b79ab1071191 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 30 Aug 2024 10:52:05 -0700 Subject: [PATCH 03/30] Finish removing raw pointer handles --- src/bun.js/bindings/v8/V8Data.h | 3 +-- src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 3 ++- src/bun.js/bindings/v8/V8Handle.h | 2 +- src/bun.js/bindings/v8/V8HandleScopeBuffer.h | 1 - src/bun.js/bindings/v8/V8Map.cpp | 1 - src/bun.js/bindings/v8/V8Map.h | 2 -- 6 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index d47e11d1f479f..0d5d5a72cb469 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -98,8 +98,7 @@ class Data { // 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(); - // REFACTOR - return TaggedPointer(v8_object->asRaw()); + return TaggedPointer(v8_object->asCell()); } } }; diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index b83d5c6fbf6c4..3d45f3aba3d14 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -113,7 +113,8 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb ImplicitArgs implicit_args = { .holder = nullptr, .isolate = isolate, - .context = reinterpret_cast(isolate), + // set to nullptr to catch any case where this is actually used + .context = nullptr, .return_value = TaggedPointer(), // data may be an object // put it in the handle scope so that it has a map ptr diff --git a/src/bun.js/bindings/v8/V8Handle.h b/src/bun.js/bindings/v8/V8Handle.h index c89cf6b1f2947..0b443f39a7546 100644 --- a/src/bun.js/bindings/v8/V8Handle.h +++ b/src/bun.js/bindings/v8/V8Handle.h @@ -117,7 +117,7 @@ struct Handle { // TODO(@190n) exhaustively switch on InstanceType if (map_ptr == &Map::object_map || map_ptr == &Map::string_map) { return true; - } else if (map_ptr == &Map::map_map || map_ptr == &Map::raw_ptr_map || map_ptr == &Map::oddball_map + } else if (map_ptr == &Map::map_map || map_ptr == &Map::oddball_map || map_ptr == &Map::boolean_map || map_ptr == &Map::heap_number_map) { return false; } else { diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h index a045951436973..d76f15f023f04 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h @@ -34,7 +34,6 @@ class HandleScopeBuffer : public JSC::JSCell { } TaggedPointer* createHandle(JSC::JSCell* object, const Map* map, JSC::VM& vm); - TaggedPointer* createRawHandle(void* ptr); TaggedPointer* createSmiHandle(int32_t smi); TaggedPointer* createDoubleHandle(double value); TaggedPointer* createHandleFromExistingHandle(TaggedPointer address); diff --git a/src/bun.js/bindings/v8/V8Map.cpp b/src/bun.js/bindings/v8/V8Map.cpp index 300c242958c2b..0303754e04f60 100644 --- a/src/bun.js/bindings/v8/V8Map.cpp +++ b/src/bun.js/bindings/v8/V8Map.cpp @@ -5,7 +5,6 @@ namespace v8 { // TODO give these more appropriate instance types const Map Map::map_map(InstanceType::Object); const Map Map::object_map(InstanceType::Object); -const Map Map::raw_ptr_map(InstanceType::Object); const Map Map::oddball_map(InstanceType::Oddball); const Map Map::boolean_map(InstanceType::Oddball); const Map Map::string_map(InstanceType::String); diff --git a/src/bun.js/bindings/v8/V8Map.h b/src/bun.js/bindings/v8/V8Map.h index 4448c235aaa75..656536bf7183b 100644 --- a/src/bun.js/bindings/v8/V8Map.h +++ b/src/bun.js/bindings/v8/V8Map.h @@ -36,8 +36,6 @@ struct Map { static const Map map_map; // the map used by objects inheriting JSCell static const Map object_map; - // the map used by pointers to non-JSCell objects stored in handles - static const Map raw_ptr_map; // the map used by oddballs (null, undefined) static const Map oddball_map; // the map used by booleans From 151fcd64b20e7293e0a7eb59c67b4cefefd308b9 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 30 Aug 2024 11:29:38 -0700 Subject: [PATCH 04/30] Get rid of Map::boolean_map --- src/bun.js/bindings/v8/V8GlobalInternals.h | 4 ++-- src/bun.js/bindings/v8/V8Handle.h | 2 +- src/bun.js/bindings/v8/V8Map.cpp | 1 - src/bun.js/bindings/v8/V8Map.h | 17 ++++++++++------- src/bun.js/bindings/v8/V8Oddball.h | 4 ++-- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/bun.js/bindings/v8/V8GlobalInternals.h b/src/bun.js/bindings/v8/V8GlobalInternals.h index bdb2bbc936ce3..3d31d03858a74 100644 --- a/src/bun.js/bindings/v8/V8GlobalInternals.h +++ b/src/bun.js/bindings/v8/V8GlobalInternals.h @@ -97,8 +97,8 @@ class GlobalInternals : public JSC::JSCell { , m_CurrentHandleScope(nullptr) , undefinedValue(Oddball::Kind::kUndefined) , nullValue(Oddball::Kind::kNull) - , trueValue(Oddball::Kind::kTrue, &Map::boolean_map) - , falseValue(Oddball::Kind::kFalse, &Map::boolean_map) + , trueValue(Oddball::Kind::kTrue) + , falseValue(Oddball::Kind::kFalse) , roots(this) , globalObject(globalObject_) { diff --git a/src/bun.js/bindings/v8/V8Handle.h b/src/bun.js/bindings/v8/V8Handle.h index 0b443f39a7546..27a5497bb6661 100644 --- a/src/bun.js/bindings/v8/V8Handle.h +++ b/src/bun.js/bindings/v8/V8Handle.h @@ -118,7 +118,7 @@ struct Handle { if (map_ptr == &Map::object_map || map_ptr == &Map::string_map) { return true; } else if (map_ptr == &Map::map_map || map_ptr == &Map::oddball_map - || map_ptr == &Map::boolean_map || map_ptr == &Map::heap_number_map) { + || map_ptr == &Map::heap_number_map) { return false; } else { RELEASE_ASSERT_NOT_REACHED("unknown Map at %p with instance type %" PRIx16, diff --git a/src/bun.js/bindings/v8/V8Map.cpp b/src/bun.js/bindings/v8/V8Map.cpp index 0303754e04f60..feeacf8b66c17 100644 --- a/src/bun.js/bindings/v8/V8Map.cpp +++ b/src/bun.js/bindings/v8/V8Map.cpp @@ -6,7 +6,6 @@ namespace v8 { const Map Map::map_map(InstanceType::Object); const Map Map::object_map(InstanceType::Object); const Map Map::oddball_map(InstanceType::Oddball); -const Map Map::boolean_map(InstanceType::Oddball); const Map Map::string_map(InstanceType::String); const Map Map::heap_number_map(InstanceType::HeapNumber); diff --git a/src/bun.js/bindings/v8/V8Map.h b/src/bun.js/bindings/v8/V8Map.h index 656536bf7183b..5c9e672ff3d28 100644 --- a/src/bun.js/bindings/v8/V8Map.h +++ b/src/bun.js/bindings/v8/V8Map.h @@ -32,17 +32,18 @@ struct Map { // (v8::internal::Internals::CanHaveInternalField, in v8-internal.h) InstanceType instance_type; - // the map used by maps + // Since maps are V8 objects, they each also have a map pointer at the start, which is this one static const Map map_map; - // the map used by objects inheriting JSCell + // All V8 values not covered by a more specific map use this one static const Map object_map; - // the map used by oddballs (null, undefined) + // The map used by null, undefined, true, and false. Required since V8 checks these values' + // instance type in the inline QuickIs* functions static const Map oddball_map; - // the map used by booleans - static const Map boolean_map; - // the map used by strings + // All strings use this map. Required since V8's inline QuickIsString() checks the instance + // type. static const Map string_map; - // the map used by heap numbers + // Handles containing a double instead of a JSCell pointer use this map so that we can tell they + // are numbers. static const Map heap_number_map; Map(InstanceType instance_type_) @@ -54,5 +55,7 @@ struct Map { }; static_assert(sizeof(Map) == 16, "Map has wrong layout"); +static_assert(offsetof(Map, meta_map) == 0, "Map has wrong layout"); +static_assert(offsetof(Map, instance_type) == 12, "Map has wrong layout"); } diff --git a/src/bun.js/bindings/v8/V8Oddball.h b/src/bun.js/bindings/v8/V8Oddball.h index c58f40c149753..511ece34c5d74 100644 --- a/src/bun.js/bindings/v8/V8Oddball.h +++ b/src/bun.js/bindings/v8/V8Oddball.h @@ -18,8 +18,8 @@ struct Oddball { uintptr_t unused[4]; TaggedPointer kind; - Oddball(Kind kind_, const Map* map_ = &Map::oddball_map) - : map(const_cast(map_)) + Oddball(Kind kind_) + : map(const_cast(&Map::oddball_map)) , kind(TaggedPointer(static_cast(kind_))) { } From 1e0aef96ba6fa00c9ca099c40f694e3b2367f81f Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 30 Aug 2024 11:30:02 -0700 Subject: [PATCH 05/30] Write test for EscapableHandleScope and attempt to catch UAF of handles --- src/bun.js/bindings/v8/V8HandleScope.cpp | 1 + .../bindings/v8/V8HandleScopeBuffer.cpp | 10 +++++++ src/bun.js/bindings/v8/V8HandleScopeBuffer.h | 2 ++ test/v8/v8-module/main.cpp | 26 ++++++++++++++++--- test/v8/v8.test.ts | 6 +++++ 5 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index f7d20b76d2cf1..7f4b498a15c9e 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -15,6 +15,7 @@ HandleScope::HandleScope(Isolate* isolate_) HandleScope::~HandleScope() { isolate->globalInternals()->setCurrentHandleScope(prev); + buffer->clear(); buffer = nullptr; } diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp index 3f7e793f57238..af3b236f31669 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp @@ -79,4 +79,14 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingHandle(TaggedPointer a return &handle.to_v8_object; } +void HandleScopeBuffer::clear() +{ + // detect use-after-free of handles + WTF::Locker locker { gc_lock }; + for (auto& handle : storage) { + handle = Handle(); + } + storage.clear(); +} + } diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h index d76f15f023f04..fb404294f3b27 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h @@ -38,6 +38,8 @@ class HandleScopeBuffer : public JSC::JSCell { TaggedPointer* createDoubleHandle(double value); TaggedPointer* createHandleFromExistingHandle(TaggedPointer address); + void clear(); + DECLARE_INFO; DECLARE_VISIT_CHILDREN; diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 76dd3c0120f2c..6c241cf38521a 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -407,15 +407,16 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { String::NewFromUtf8(isolate, cpp_str.c_str()).ToLocalChecked(); } - // allocate some objects with internal fields, to check that those are traced + // allocate some objects with internal fields, to check that those are + // traced Local tmp = ObjectTemplate::New(isolate); tmp->SetInternalFieldCount(2); Local objects[num_small_allocs]; for (size_t i = 0; i < num_small_allocs; i++) { std::string cpp_str = std::to_string(i + num_small_allocs); - // this uses a function so that the strings aren't kept alive by the current - // handle scope + // this uses a function so that the strings aren't kept alive by the + // current handle scope objects[i] = setup_object_with_string_field(isolate, context, tmp, i, cpp_str); } @@ -466,6 +467,23 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { delete[] string_data; } +Local return_escaped_handle(Isolate *isolate) { + EscapableHandleScope ehs(isolate); + Local s = String::NewFromUtf8(isolate, "hello").ToLocalChecked(); + return ehs.Escape(s); +} + +void test_v8_escapable_handle_scope(const FunctionCallbackInfo &info) { + Isolate *isolate = info.GetIsolate(); + Local s = return_escaped_handle(isolate); + // n should overwrite the handle from s if s is stale + Local n = Number::New(isolate, 6.5); + char buf[16]; + s->WriteUtf8(isolate, buf); + LOG_EXPR(buf); + LOG_EXPR(n->Value()); +} + void initialize(Local exports, Local module, Local context) { NODE_SET_METHOD(exports, "test_v8_native_call", test_v8_native_call); @@ -492,6 +510,8 @@ void initialize(Local exports, Local module, NODE_SET_METHOD(exports, "global_set", GlobalTestWrapper::set); NODE_SET_METHOD(exports, "test_many_v8_locals", test_many_v8_locals); NODE_SET_METHOD(exports, "test_handle_scope_gc", test_handle_scope_gc); + NODE_SET_METHOD(exports, "test_v8_escapable_handle_scope", + test_v8_escapable_handle_scope); // without this, node hits a UAF deleting the Global node::AddEnvironmentCleanupHook(context->GetIsolate(), diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index 9d27cdd9528ce..30f8cb23df1ff 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -194,6 +194,12 @@ describe("HandleScope", () => { }, 10000); }); +describe("EscapableHandleScope", () => { + it("keeps handles alive in the outer scope", () => { + checkSameOutput("test_v8_escapable_handle_scope", []); + }); +}); + afterAll(async () => { await Promise.all([ fs.rm(directories.bunRelease, { recursive: true, force: true }), From e040644ce6ee5b5a408699e0fe9b9895a19e4e62 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 30 Aug 2024 17:11:07 -0700 Subject: [PATCH 06/30] Fix Global and EscapableHandleScope in the presence of oddball values --- .../v8/V8EscapableHandleScopeBase.cpp | 9 +++-- src/bun.js/bindings/v8/V8HandleScope.cpp | 4 ++- .../bindings/v8/V8HandleScopeBuffer.cpp | 28 +++++++++++++--- src/bun.js/bindings/v8/V8HandleScopeBuffer.h | 11 ++++++- src/bun.js/bindings/v8/V8Roots.h | 14 ++++---- src/bun.js/bindings/v8/v8_api_internal.cpp | 3 +- test/v8/v8-module/main.cpp | 33 +++++++++++++++---- test/v8/v8-module/module.js | 13 ++++++-- 8 files changed, 90 insertions(+), 25 deletions(-) diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index 9bc31b58b5040..1256bb7ebae94 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -15,8 +15,13 @@ EscapableHandleScopeBase::EscapableHandleScopeBase(Isolate* isolate) // HandleScope, and return the escape slot uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value) { - *escape_slot = *reinterpret_cast(escape_value); - return &escape_slot->to_v8_object.value; + RELEASE_ASSERT(escape_slot != nullptr, "EscapableHandleScope::Escape called multiple times"); + TaggedPointer* newHandle = prev->buffer->createHandleFromExistingObject( + TaggedPointer::fromRaw(*escape_value), + reinterpret_cast(isolate), + escape_slot); + escape_slot = nullptr; + return &newHandle->value; } } diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index 7f4b498a15c9e..20a83a622bfd4 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -22,7 +22,9 @@ HandleScope::~HandleScope() uintptr_t* HandleScope::CreateHandle(internal::Isolate* isolate, uintptr_t value) { auto* handleScope = reinterpret_cast(isolate)->globalInternals()->currentHandleScope(); - return &handleScope->buffer->createHandleFromExistingHandle(TaggedPointer::fromRaw(value))->value; + TaggedPointer* newSlot = handleScope->buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), reinterpret_cast(isolate)); + // basically a reinterpret + return &newSlot->value; } } diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp index af3b236f31669..d9f65e3efacf5 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp @@ -1,4 +1,5 @@ #include "V8HandleScopeBuffer.h" +#include "V8GlobalInternals.h" namespace v8 { @@ -66,17 +67,34 @@ TaggedPointer* HandleScopeBuffer::createDoubleHandle(double value) return &handle.to_v8_object; } -TaggedPointer* HandleScopeBuffer::createHandleFromExistingHandle(TaggedPointer address) +TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer address, Roots* roots, Handle* reuseHandle) { - auto& handle = createEmptyHandle(); int32_t smi; if (address.getSmi(smi)) { - handle = Handle(smi); + if (reuseHandle) { + *reuseHandle = Handle(smi); + return &reuseHandle->to_v8_object; + } else { + return createSmiHandle(smi); + } } else { auto* v8_object = address.getPtr(); - handle = Handle(v8_object); + if (v8_object->tagged_map.getPtr()->instance_type == InstanceType::Oddball) { + // find which oddball this is + for (int i = 0; i < Roots::rootsSize; i++) { + if (roots->roots[i] == address) { + return &roots->roots[i]; + } + } + RELEASE_ASSERT_NOT_REACHED("HandleScopeBuffer::createHandleFromExistingObject passed an Oddball which does not exist in Roots"); + } + if (reuseHandle) { + *reuseHandle = Handle(v8_object->map(), v8_object->asCell(), vm(), this); + return &reuseHandle->to_v8_object; + } else { + return createHandle(v8_object->asCell(), v8_object->map(), vm()); + } } - return &handle.to_v8_object; } void HandleScopeBuffer::clear() diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h index fb404294f3b27..b3b906d44f609 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h @@ -4,6 +4,7 @@ #include "V8TaggedPointer.h" #include "V8Map.h" #include "V8Handle.h" +#include "V8Roots.h" namespace v8 { @@ -36,7 +37,15 @@ class HandleScopeBuffer : public JSC::JSCell { TaggedPointer* createHandle(JSC::JSCell* object, const Map* map, JSC::VM& vm); TaggedPointer* createSmiHandle(int32_t smi); TaggedPointer* createDoubleHandle(double value); - TaggedPointer* createHandleFromExistingHandle(TaggedPointer address); + + // Given a tagged pointer from V8, create a handle around the same object or the same + // numeric value + // + // address: V8 object pointer or Smi + // roots: for now, a reinterpreted Isolate pointer + // reuseHandle: if nonnull, change this handle instead of creating a new one + // returns the location of the new handle's V8 object pointer or Smi + TaggedPointer* createHandleFromExistingObject(TaggedPointer address, Roots* roots, Handle* reuseHandle = nullptr); void clear(); diff --git a/src/bun.js/bindings/v8/V8Roots.h b/src/bun.js/bindings/v8/V8Roots.h index 1a538388e4416..1b3f56d57795b 100644 --- a/src/bun.js/bindings/v8/V8Roots.h +++ b/src/bun.js/bindings/v8/V8Roots.h @@ -11,17 +11,19 @@ class GlobalInternals; // the layout is correct. struct Roots { // v8-internal.h:775 - static const int kUndefinedValueRootIndex = 4; - static const int kTheHoleValueRootIndex = 5; - static const int kNullValueRootIndex = 6; - static const int kTrueValueRootIndex = 7; - static const int kFalseValueRootIndex = 8; + static constexpr int kUndefinedValueRootIndex = 4; + static constexpr int kTheHoleValueRootIndex = 5; + static constexpr int kNullValueRootIndex = 6; + static constexpr int kTrueValueRootIndex = 7; + static constexpr int kFalseValueRootIndex = 8; + + static constexpr int rootsSize = 9; GlobalInternals* parent; uintptr_t padding[73]; - TaggedPointer roots[9]; + TaggedPointer roots[rootsSize]; Roots(GlobalInternals* parent); }; diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index f610051696143..ff3d4151f918d 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -13,7 +13,8 @@ void ToLocalEmpty() uintptr_t* GlobalizeReference(v8::internal::Isolate* isolate, uintptr_t address) { auto* globalHandles = reinterpret_cast(isolate)->globalInternals()->globalHandles(); - return &globalHandles->createHandleFromExistingHandle(TaggedPointer::fromRaw(address))->value; + TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), reinterpret_cast(isolate)); + return &newSlot->value; } void DisposeGlobal(uintptr_t* location) diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 6c241cf38521a..0a909242a0147 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -467,17 +467,38 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { delete[] string_data; } -Local return_escaped_handle(Isolate *isolate) { +Local escape_object(Isolate *isolate) { EscapableHandleScope ehs(isolate); - Local s = String::NewFromUtf8(isolate, "hello").ToLocalChecked(); - return ehs.Escape(s); + Local invalidated = + String::NewFromUtf8(isolate, "hello").ToLocalChecked(); + Local escaped = ehs.Escape(invalidated); + return escaped; +} + +Local escape_smi(Isolate *isolate) { + EscapableHandleScope ehs(isolate); + Local invalidated = Number::New(isolate, 3.0); + Local escaped = ehs.Escape(invalidated); + return escaped; +} + +Local escape_true(Isolate *isolate) { + EscapableHandleScope ehs(isolate); + Local invalidated = v8::True(isolate); + Local escaped = ehs.Escape(invalidated); + return escaped; } void test_v8_escapable_handle_scope(const FunctionCallbackInfo &info) { Isolate *isolate = info.GetIsolate(); - Local s = return_escaped_handle(isolate); - // n should overwrite the handle from s if s is stale - Local n = Number::New(isolate, 6.5); + Local s = escape_object(isolate); + Local n = escape_smi(isolate); + Local t = escape_true(isolate); + + LOG_VALUE_KIND(s); + LOG_VALUE_KIND(n); + LOG_VALUE_KIND(t); + char buf[16]; s->WriteUtf8(isolate, buf); LOG_EXPR(buf); diff --git a/test/v8/v8-module/module.js b/test/v8/v8-module/module.js index b15152026be43..fcbd23231c87b 100644 --- a/test/v8/v8-module/module.js +++ b/test/v8/v8-module/module.js @@ -2,16 +2,23 @@ module.exports = debugMode => { const nativeModule = require(`./build/${debugMode ? "Debug" : "Release"}/v8tests`); return { ...nativeModule, + test_v8_global() { - console.log(nativeModule.global_get()); + console.log("global initial value =", nativeModule.global_get()); + nativeModule.global_set(123); - console.log(nativeModule.global_get()); + console.log("global after setting to 123 =", nativeModule.global_get()); + nativeModule.global_set({ foo: 5, bar: ["one", "two", "three"] }); if (process.isBun) { Bun.gc(true); } - console.log(JSON.stringify(nativeModule.global_get())); + console.log("global after setting to object =", JSON.stringify(nativeModule.global_get())); + + nativeModule.global_set(true); + console.log("global after setting to true =", nativeModule.global_get()); }, + test_v8_function_template() { const f = nativeModule.create_function_with_data(); if (process.isBun) { From 86d3a241c2770f2a1a2b5809dd180b09402b5ded Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 30 Aug 2024 17:59:29 -0700 Subject: [PATCH 07/30] Delete v8::Roots and move its contents into v8::Isolate, and perform the necessary refactoring --- src/bun.js/bindings/napi.cpp | 3 +- src/bun.js/bindings/v8/V8Context.cpp | 2 +- src/bun.js/bindings/v8/V8Context.h | 3 +- .../v8/V8EscapableHandleScopeBase.cpp | 2 +- src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 2 +- src/bun.js/bindings/v8/V8GlobalInternals.cpp | 22 +++---- src/bun.js/bindings/v8/V8GlobalInternals.h | 66 ++++++++++--------- src/bun.js/bindings/v8/V8HandleScope.cpp | 7 +- src/bun.js/bindings/v8/V8HandleScope.h | 1 + .../bindings/v8/V8HandleScopeBuffer.cpp | 9 +-- src/bun.js/bindings/v8/V8HandleScopeBuffer.h | 7 +- src/bun.js/bindings/v8/V8Isolate.cpp | 24 +++++-- src/bun.js/bindings/v8/V8Isolate.h | 37 ++++++++--- src/bun.js/bindings/v8/V8Object.cpp | 2 +- src/bun.js/bindings/v8/V8Roots.cpp | 15 ----- src/bun.js/bindings/v8/V8Roots.h | 34 ---------- src/bun.js/bindings/v8/node.cpp | 13 +--- src/bun.js/bindings/v8/v8_api_internal.cpp | 8 ++- src/bun.js/bindings/v8/v8_internal.h | 1 + 19 files changed, 120 insertions(+), 138 deletions(-) delete mode 100644 src/bun.js/bindings/v8/V8Roots.cpp delete mode 100644 src/bun.js/bindings/v8/V8Roots.h diff --git a/src/bun.js/bindings/napi.cpp b/src/bun.js/bindings/napi.cpp index 2d122605e67c3..7eb2061bbde1a 100644 --- a/src/bun.js/bindings/napi.cpp +++ b/src/bun.js/bindings/napi.cpp @@ -342,7 +342,7 @@ class NAPIFunction : public JSC::JSFunction { NAPICallFrame frame(JSC::ArgList(args), function->m_dataPtr); auto scope = DECLARE_THROW_SCOPE(vm); - v8::HandleScope handleScope(v8::Isolate::fromGlobalObject(static_cast(globalObject))); + v8::HandleScope handleScope(jsCast(globalObject)->V8GlobalInternals()->isolate()); auto result = callback(env, NAPICallFrame::toNapiCallbackInfo(frame)); @@ -1239,7 +1239,6 @@ static JSValue createErrorForNapi(napi_env env, napi_value code, napi_value msg, } } - auto* error = constructor(globalObject, message); if (codeValue && error) { diff --git a/src/bun.js/bindings/v8/V8Context.cpp b/src/bun.js/bindings/v8/V8Context.cpp index c0ab6047284e9..9efe2a0ac66be 100644 --- a/src/bun.js/bindings/v8/V8Context.cpp +++ b/src/bun.js/bindings/v8/V8Context.cpp @@ -4,7 +4,7 @@ namespace v8 { Isolate* Context::GetIsolate() { - return reinterpret_cast(&globalObject()->V8GlobalInternals()->roots); + return globalObject()->V8GlobalInternals()->isolate(); } } diff --git a/src/bun.js/bindings/v8/V8Context.h b/src/bun.js/bindings/v8/V8Context.h index 3b8839849158e..efaae1a175fda 100644 --- a/src/bun.js/bindings/v8/V8Context.h +++ b/src/bun.js/bindings/v8/V8Context.h @@ -1,7 +1,6 @@ #pragma once #include "ZigGlobalObject.h" -#include "V8GlobalInternals.h" #include "V8Data.h" namespace v8 { @@ -16,7 +15,7 @@ class Context : public Data { JSC::VM& vm() const { - return globalObject()->vm(); + return localToCell()->vm(); } const Zig::GlobalObject* globalObject() const diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index 1256bb7ebae94..bb08c9da1845b 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -18,7 +18,7 @@ uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value) RELEASE_ASSERT(escape_slot != nullptr, "EscapableHandleScope::Escape called multiple times"); TaggedPointer* newHandle = prev->buffer->createHandleFromExistingObject( TaggedPointer::fromRaw(*escape_value), - reinterpret_cast(isolate), + isolate, escape_slot); escape_slot = nullptr; return &newHandle->value; diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index 3d45f3aba3d14..df3d50a8ff036 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -94,7 +94,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb { auto* callee = JSC::jsDynamicCast(callFrame->jsCallee()); auto* functionTemplate = callee->functionTemplate(); - auto* isolate = Isolate::fromGlobalObject(JSC::jsDynamicCast(globalObject)); + auto* isolate = JSC::jsCast(globalObject)->V8GlobalInternals()->isolate(); auto& vm = globalObject->vm(); WTF::Vector args(callFrame->argumentCount() + 1); diff --git a/src/bun.js/bindings/v8/V8GlobalInternals.cpp b/src/bun.js/bindings/v8/V8GlobalInternals.cpp index 6a5a36fa87293..1e71ccee4aa2f 100644 --- a/src/bun.js/bindings/v8/V8GlobalInternals.cpp +++ b/src/bun.js/bindings/v8/V8GlobalInternals.cpp @@ -33,21 +33,21 @@ GlobalInternals* GlobalInternals::create(VM& vm, Structure* structure, Zig::Glob void GlobalInternals::finishCreation(VM& vm) { Base::finishCreation(vm); - m_ObjectTemplateStructure.initLater([](LazyClassStructure::Initializer& init) { + m_objectTemplateStructure.initLater([](LazyClassStructure::Initializer& init) { init.setStructure(ObjectTemplate::createStructure(init.vm, init.global, init.global->functionPrototype())); }); - m_HandleScopeBufferStructure.initLater([](LazyClassStructure::Initializer& init) { + m_handleScopeBufferStructure.initLater([](LazyClassStructure::Initializer& init) { init.setStructure(HandleScopeBuffer::createStructure(init.vm, init.global)); }); - m_FunctionTemplateStructure.initLater([](LazyClassStructure::Initializer& init) { + m_functionTemplateStructure.initLater([](LazyClassStructure::Initializer& init) { init.setStructure(FunctionTemplate::createStructure(init.vm, init.global)); }); - m_V8FunctionStructure.initLater([](LazyClassStructure::Initializer& init) { + m_v8FunctionStructure.initLater([](LazyClassStructure::Initializer& init) { init.setStructure(Function::createStructure(init.vm, init.global)); }); - m_GlobalHandles.initLater([](const LazyProperty::Initializer& init) { + m_globalHandles.initLater([](const LazyProperty::Initializer& init) { init.set(HandleScopeBuffer::create(init.vm, - init.owner->handleScopeBufferStructure(init.owner->globalObject))); + init.owner->handleScopeBufferStructure(init.owner->m_globalObject))); }); } @@ -58,11 +58,11 @@ void GlobalInternals::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(thisObject, info()); Base::visitChildren(thisObject, visitor); - thisObject->m_ObjectTemplateStructure.visit(visitor); - thisObject->m_HandleScopeBufferStructure.visit(visitor); - thisObject->m_FunctionTemplateStructure.visit(visitor); - thisObject->m_V8FunctionStructure.visit(visitor); - thisObject->m_GlobalHandles.visit(visitor); + thisObject->m_objectTemplateStructure.visit(visitor); + thisObject->m_handleScopeBufferStructure.visit(visitor); + thisObject->m_functionTemplateStructure.visit(visitor); + thisObject->m_v8FunctionStructure.visit(visitor); + thisObject->m_globalHandles.visit(visitor); } DEFINE_VISIT_CHILDREN_WITH_MODIFIER(JS_EXPORT_PRIVATE, GlobalInternals); diff --git a/src/bun.js/bindings/v8/V8GlobalInternals.h b/src/bun.js/bindings/v8/V8GlobalInternals.h index 3d31d03858a74..734a2e76b7f1b 100644 --- a/src/bun.js/bindings/v8/V8GlobalInternals.h +++ b/src/bun.js/bindings/v8/V8GlobalInternals.h @@ -2,7 +2,7 @@ #include "BunClientData.h" -#include "V8Roots.h" +#include "V8Isolate.h" #include "V8Oddball.h" namespace v8 { @@ -36,37 +36,39 @@ class GlobalInternals : public JSC::JSCell { JSC::Structure* objectTemplateStructure(JSC::JSGlobalObject* globalObject) const { - return m_ObjectTemplateStructure.getInitializedOnMainThread(globalObject); + return m_objectTemplateStructure.getInitializedOnMainThread(globalObject); } JSC::Structure* handleScopeBufferStructure(JSC::JSGlobalObject* globalObject) const { - return m_HandleScopeBufferStructure.getInitializedOnMainThread(globalObject); + return m_handleScopeBufferStructure.getInitializedOnMainThread(globalObject); } JSC::Structure* functionTemplateStructure(JSC::JSGlobalObject* globalObject) const { - return m_FunctionTemplateStructure.getInitializedOnMainThread(globalObject); + return m_functionTemplateStructure.getInitializedOnMainThread(globalObject); } JSC::Structure* v8FunctionStructure(JSC::JSGlobalObject* globalObject) const { - return m_V8FunctionStructure.getInitializedOnMainThread(globalObject); + return m_v8FunctionStructure.getInitializedOnMainThread(globalObject); } - HandleScopeBuffer* globalHandles() const { return m_GlobalHandles.getInitializedOnMainThread(this); } + HandleScopeBuffer* globalHandles() const { return m_globalHandles.getInitializedOnMainThread(this); } - HandleScope* currentHandleScope() const { return m_CurrentHandleScope; } + HandleScope* currentHandleScope() const { return m_currentHandleScope; } - void setCurrentHandleScope(HandleScope* handleScope) { m_CurrentHandleScope = handleScope; } + void setCurrentHandleScope(HandleScope* handleScope) { m_currentHandleScope = handleScope; } - TaggedPointer* undefinedSlot() { return &roots.roots[Roots::kUndefinedValueRootIndex]; } + TaggedPointer* undefinedSlot() { return m_isolate.getRoot(Isolate::kUndefinedValueRootIndex); } - TaggedPointer* nullSlot() { return &roots.roots[Roots::kNullValueRootIndex]; } + TaggedPointer* nullSlot() { return m_isolate.getRoot(Isolate::kNullValueRootIndex); } - TaggedPointer* trueSlot() { return &roots.roots[Roots::kTrueValueRootIndex]; } + TaggedPointer* trueSlot() { return m_isolate.getRoot(Isolate::kTrueValueRootIndex); } - TaggedPointer* falseSlot() { return &roots.roots[Roots::kFalseValueRootIndex]; } + TaggedPointer* falseSlot() { return m_isolate.getRoot(Isolate::kFalseValueRootIndex); } + + Isolate* isolate() { return &m_isolate; } DECLARE_INFO; DECLARE_VISIT_CHILDREN_WITH_MODIFIER(JS_EXPORT_PRIVATE); @@ -76,31 +78,31 @@ class GlobalInternals : public JSC::JSCell { friend class Context; private: - Zig::GlobalObject* globalObject; - JSC::LazyClassStructure m_ObjectTemplateStructure; - JSC::LazyClassStructure m_HandleScopeBufferStructure; - JSC::LazyClassStructure m_FunctionTemplateStructure; - JSC::LazyClassStructure m_V8FunctionStructure; - HandleScope* m_CurrentHandleScope; - JSC::LazyProperty m_GlobalHandles; + Zig::GlobalObject* m_globalObject; + JSC::LazyClassStructure m_objectTemplateStructure; + JSC::LazyClassStructure m_handleScopeBufferStructure; + JSC::LazyClassStructure m_functionTemplateStructure; + JSC::LazyClassStructure m_v8FunctionStructure; + HandleScope* m_currentHandleScope; + JSC::LazyProperty m_globalHandles; - Oddball undefinedValue; - Oddball nullValue; - Oddball trueValue; - Oddball falseValue; + Oddball m_undefinedValue; + Oddball m_nullValue; + Oddball m_trueValue; + Oddball m_falseValue; - Roots roots; + Isolate m_isolate; void finishCreation(JSC::VM& vm); - GlobalInternals(JSC::VM& vm, JSC::Structure* structure, Zig::GlobalObject* globalObject_) + GlobalInternals(JSC::VM& vm, JSC::Structure* structure, Zig::GlobalObject* globalObject) : Base(vm, structure) - , m_CurrentHandleScope(nullptr) - , undefinedValue(Oddball::Kind::kUndefined) - , nullValue(Oddball::Kind::kNull) - , trueValue(Oddball::Kind::kTrue) - , falseValue(Oddball::Kind::kFalse) - , roots(this) - , globalObject(globalObject_) + , m_currentHandleScope(nullptr) + , m_undefinedValue(Oddball::Kind::kUndefined) + , m_nullValue(Oddball::Kind::kNull) + , m_trueValue(Oddball::Kind::kTrue) + , m_falseValue(Oddball::Kind::kFalse) + , m_isolate(this) + , m_globalObject(globalObject) { } }; diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index 20a83a622bfd4..eb32591e98e9c 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -19,10 +19,11 @@ HandleScope::~HandleScope() buffer = nullptr; } -uintptr_t* HandleScope::CreateHandle(internal::Isolate* isolate, uintptr_t value) +uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t value) { - auto* handleScope = reinterpret_cast(isolate)->globalInternals()->currentHandleScope(); - TaggedPointer* newSlot = handleScope->buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), reinterpret_cast(isolate)); + auto* isolate = reinterpret_cast(i_isolate); + auto* handleScope = isolate->globalInternals()->currentHandleScope(); + TaggedPointer* newSlot = handleScope->buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate); // basically a reinterpret return &newSlot->value; } diff --git a/src/bun.js/bindings/v8/V8HandleScope.h b/src/bun.js/bindings/v8/V8HandleScope.h index b32eb05a05cbc..668f3ae1c0325 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.h +++ b/src/bun.js/bindings/v8/V8HandleScope.h @@ -4,6 +4,7 @@ #include "V8Isolate.h" #include "v8_internal.h" #include "V8HandleScopeBuffer.h" +#include "V8GlobalInternals.h" namespace v8 { diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp index d9f65e3efacf5..da9cc7d674925 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp @@ -1,5 +1,6 @@ #include "V8HandleScopeBuffer.h" #include "V8GlobalInternals.h" +#include "V8Isolate.h" namespace v8 { @@ -67,7 +68,7 @@ TaggedPointer* HandleScopeBuffer::createDoubleHandle(double value) return &handle.to_v8_object; } -TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer address, Roots* roots, Handle* reuseHandle) +TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer address, Isolate* isolate, Handle* reuseHandle) { int32_t smi; if (address.getSmi(smi)) { @@ -81,9 +82,9 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a auto* v8_object = address.getPtr(); if (v8_object->tagged_map.getPtr()->instance_type == InstanceType::Oddball) { // find which oddball this is - for (int i = 0; i < Roots::rootsSize; i++) { - if (roots->roots[i] == address) { - return &roots->roots[i]; + for (auto& root : isolate->m_roots) { + if (root == address) { + return &root; } } RELEASE_ASSERT_NOT_REACHED("HandleScopeBuffer::createHandleFromExistingObject passed an Oddball which does not exist in Roots"); diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h index b3b906d44f609..fe1abb11447dd 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h @@ -4,10 +4,11 @@ #include "V8TaggedPointer.h" #include "V8Map.h" #include "V8Handle.h" -#include "V8Roots.h" namespace v8 { +class Isolate; + // An array used by HandleScope to store the items. Must keep pointer stability when resized, since // v8::Locals point inside this array. class HandleScopeBuffer : public JSC::JSCell { @@ -42,10 +43,10 @@ class HandleScopeBuffer : public JSC::JSCell { // numeric value // // address: V8 object pointer or Smi - // roots: for now, a reinterpreted Isolate pointer + // isolate: received in any V8 method // reuseHandle: if nonnull, change this handle instead of creating a new one // returns the location of the new handle's V8 object pointer or Smi - TaggedPointer* createHandleFromExistingObject(TaggedPointer address, Roots* roots, Handle* reuseHandle = nullptr); + TaggedPointer* createHandleFromExistingObject(TaggedPointer address, Isolate* isolate, Handle* reuseHandle = nullptr); void clear(); diff --git a/src/bun.js/bindings/v8/V8Isolate.cpp b/src/bun.js/bindings/v8/V8Isolate.cpp index 537463d02d0ac..b5b548be232cf 100644 --- a/src/bun.js/bindings/v8/V8Isolate.cpp +++ b/src/bun.js/bindings/v8/V8Isolate.cpp @@ -1,5 +1,6 @@ #include "V8Isolate.h" #include "V8HandleScope.h" +#include "V8GlobalInternals.h" namespace v8 { @@ -8,7 +9,7 @@ Isolate* Isolate::TryGetCurrent() { auto* global = Bun__getDefaultGlobalObject(); - return global ? reinterpret_cast(&global->V8GlobalInternals()->roots) : nullptr; + return global ? &global->V8GlobalInternals()->m_isolate : nullptr; } // Returns the isolate inside which the current thread is running. @@ -16,14 +17,27 @@ Isolate* Isolate::GetCurrent() { auto* global = Bun__getDefaultGlobalObject(); - return global ? reinterpret_cast(&global->V8GlobalInternals()->roots) : nullptr; + return global ? &global->V8GlobalInternals()->m_isolate : nullptr; } Local Isolate::GetCurrentContext() { - auto* globalInternals = reinterpret_cast(this)->parent; - auto* globalObject = globalInternals->globalObject; - return globalInternals->currentHandleScope()->createLocal(globalObject->vm(), globalObject); + return currentHandleScope()->createLocal(m_globalObject->vm(), m_globalObject); +} + +Isolate::Isolate(GlobalInternals* globalInternals) + : m_globalInternals(globalInternals) + , m_globalObject(globalInternals->m_globalObject) +{ + m_roots[kUndefinedValueRootIndex] = TaggedPointer(&globalInternals->m_undefinedValue); + m_roots[kNullValueRootIndex] = TaggedPointer(&globalInternals->m_nullValue); + m_roots[kTrueValueRootIndex] = TaggedPointer(&globalInternals->m_trueValue); + m_roots[kFalseValueRootIndex] = TaggedPointer(&globalInternals->m_falseValue); +} + +HandleScope* Isolate::currentHandleScope() +{ + return m_globalInternals->currentHandleScope(); } } diff --git a/src/bun.js/bindings/v8/V8Isolate.h b/src/bun.js/bindings/v8/V8Isolate.h index 4616c22060bd3..929e76bf79e28 100644 --- a/src/bun.js/bindings/v8/V8Isolate.h +++ b/src/bun.js/bindings/v8/V8Isolate.h @@ -1,20 +1,27 @@ #pragma once #include "v8.h" -#include "V8Context.h" #include "V8Local.h" -#include "V8GlobalInternals.h" namespace v8 { class HandleScope; +class Context; +class GlobalInternals; -// This currently is just a pointer to a v8::Roots -// We do that so that we can recover the context and the VM from the "Isolate," and so that inlined -// V8 functions can find values at certain fixed offsets from the Isolate +// The only fields here are "roots," which are the global locations of V8's versions of nullish and +// boolean values. These are computed as offsets from an Isolate pointer in many V8 functions so +// they need to have the correct layout. class Isolate final { public: - Isolate() = default; + // v8-internal.h:775 + static constexpr int kUndefinedValueRootIndex = 4; + static constexpr int kTheHoleValueRootIndex = 5; + static constexpr int kNullValueRootIndex = 6; + static constexpr int kTrueValueRootIndex = 7; + static constexpr int kFalseValueRootIndex = 8; + + Isolate(GlobalInternals* globalInternals); // Returns the isolate inside which the current thread is running or nullptr. BUN_EXPORT static Isolate* TryGetCurrent(); @@ -24,11 +31,21 @@ class Isolate final { BUN_EXPORT Local GetCurrentContext(); - static Isolate* fromGlobalObject(Zig::GlobalObject* globalObject) { return reinterpret_cast(&globalObject->V8GlobalInternals()->roots); } - Zig::GlobalObject* globalObject() { return reinterpret_cast(this)->parent->globalObject; } + Zig::GlobalObject* globalObject() { return m_globalObject; } JSC::VM& vm() { return globalObject()->vm(); } - GlobalInternals* globalInternals() { return globalObject()->V8GlobalInternals(); } - HandleScope* currentHandleScope() { return globalInternals()->currentHandleScope(); } + GlobalInternals* globalInternals() { return m_globalInternals; } + HandleScope* currentHandleScope(); + + TaggedPointer* getRoot(int index) { return &m_roots[index]; } + + GlobalInternals* m_globalInternals; + Zig::GlobalObject* m_globalObject; + + uintptr_t m_padding[72]; + + std::array m_roots; }; +static_assert(offsetof(Isolate, m_roots) == 592, "Isolate has wrong layout"); + } diff --git a/src/bun.js/bindings/v8/V8Object.cpp b/src/bun.js/bindings/v8/V8Object.cpp index cb6d5215eab23..ba9d9205eb52f 100644 --- a/src/bun.js/bindings/v8/V8Object.cpp +++ b/src/bun.js/bindings/v8/V8Object.cpp @@ -80,7 +80,7 @@ Local Object::SlowGetInternalField(int index) auto* fields = getInternalFieldsContainer(this); JSObject* js_object = localToObjectPointer(); auto* globalObject = JSC::jsDynamicCast(js_object->globalObject()); - HandleScope* handleScope = Isolate::fromGlobalObject(globalObject)->currentHandleScope(); + HandleScope* handleScope = globalObject->V8GlobalInternals()->currentHandleScope(); if (fields && index >= 0 && index < fields->size()) { auto& field = fields->at(index); return handleScope->createLocal(globalObject->vm(), field.get()); diff --git a/src/bun.js/bindings/v8/V8Roots.cpp b/src/bun.js/bindings/v8/V8Roots.cpp deleted file mode 100644 index 025f68a714cf0..0000000000000 --- a/src/bun.js/bindings/v8/V8Roots.cpp +++ /dev/null @@ -1,15 +0,0 @@ -#include "V8Roots.h" -#include "V8GlobalInternals.h" - -namespace v8 { - -Roots::Roots(GlobalInternals* parent_) - : parent(parent_) -{ - roots[kUndefinedValueRootIndex] = TaggedPointer(&parent->undefinedValue); - roots[kNullValueRootIndex] = TaggedPointer(&parent->nullValue); - roots[kTrueValueRootIndex] = TaggedPointer(&parent->trueValue); - roots[kFalseValueRootIndex] = TaggedPointer(&parent->falseValue); -} - -} diff --git a/src/bun.js/bindings/v8/V8Roots.h b/src/bun.js/bindings/v8/V8Roots.h deleted file mode 100644 index 1b3f56d57795b..0000000000000 --- a/src/bun.js/bindings/v8/V8Roots.h +++ /dev/null @@ -1,34 +0,0 @@ -#pragma once - -#include "V8TaggedPointer.h" - -namespace v8 { - -class GlobalInternals; - -// Container for some data that V8 expects to find at certain offsets. Isolate and Context pointers -// actually point to this object. It is a separate struct so that we can use offsetof() to make sure -// the layout is correct. -struct Roots { - // v8-internal.h:775 - static constexpr int kUndefinedValueRootIndex = 4; - static constexpr int kTheHoleValueRootIndex = 5; - static constexpr int kNullValueRootIndex = 6; - static constexpr int kTrueValueRootIndex = 7; - static constexpr int kFalseValueRootIndex = 8; - - static constexpr int rootsSize = 9; - - GlobalInternals* parent; - - uintptr_t padding[73]; - - TaggedPointer roots[rootsSize]; - - Roots(GlobalInternals* parent); -}; - -// kIsolateRootsOffset at v8-internal.h:744 -static_assert(offsetof(Roots, roots) == 592, "Roots does not match V8 layout"); - -} diff --git a/src/bun.js/bindings/v8/node.cpp b/src/bun.js/bindings/v8/node.cpp index 1cf41766ded62..4d148f286dd15 100644 --- a/src/bun.js/bindings/v8/node.cpp +++ b/src/bun.js/bindings/v8/node.cpp @@ -38,14 +38,6 @@ void node_module_register(void* opaque_mod) auto* globalObject = Bun__getDefaultGlobalObject(); auto& vm = globalObject->vm(); auto* mod = reinterpret_cast(opaque_mod); - // Error: The module '/Users/ben/code/bun/test/v8/v8-module/build/Release/v8tests.node' - // was compiled against a different Node.js version using - // NODE_MODULE_VERSION 127. This version of Node.js requires - // NODE_MODULE_VERSION 108. Please try re-compiling or re-installing - // the module (for instance, using `npm rebuild` or `npm install`). - - if (mod->nm_version != REPORTED_NODEJS_ABI_VERSION) { - } auto keyStr = WTF::String::fromUTF8(mod->nm_modname); globalObject->napiModuleRegisterCallCount++; @@ -86,12 +78,13 @@ void node_module_register(void* opaque_mod) JSC::Strong strongObject = { vm, object }; - HandleScope hs(Isolate::fromGlobalObject(globalObject)); + auto* isolate = globalObject->V8GlobalInternals()->isolate(); + HandleScope hs(isolate); // exports, module Local exports = hs.createLocal(vm, *strongExportsObject); Local module = hs.createLocal(vm, object); - Local context = Isolate::fromGlobalObject(globalObject)->GetCurrentContext(); + Local context = isolate->GetCurrentContext(); if (mod->nm_context_register_func) { mod->nm_context_register_func(exports, module, context, mod->nm_priv); } else if (mod->nm_register_func) { diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index ff3d4151f918d..d5b35de82bae5 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -1,6 +1,7 @@ #include "v8_api_internal.h" #include "V8Isolate.h" #include "V8HandleScopeBuffer.h" +#include "V8GlobalInternals.h" namespace v8 { namespace api_internal { @@ -10,10 +11,11 @@ void ToLocalEmpty() BUN_PANIC("Attempt to unwrap an empty v8::MaybeLocal"); } -uintptr_t* GlobalizeReference(v8::internal::Isolate* isolate, uintptr_t address) +uintptr_t* GlobalizeReference(internal::Isolate* i_isolate, uintptr_t address) { - auto* globalHandles = reinterpret_cast(isolate)->globalInternals()->globalHandles(); - TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), reinterpret_cast(isolate)); + auto* isolate = reinterpret_cast(i_isolate); + auto* globalHandles = isolate->globalInternals()->globalHandles(); + TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), isolate); return &newSlot->value; } diff --git a/src/bun.js/bindings/v8/v8_internal.h b/src/bun.js/bindings/v8/v8_internal.h index da9be2b59bded..98c511f5804fc 100644 --- a/src/bun.js/bindings/v8/v8_internal.h +++ b/src/bun.js/bindings/v8/v8_internal.h @@ -5,6 +5,7 @@ namespace v8 { namespace internal { +// identical to v8::Isolate class Isolate {}; BUN_EXPORT Isolate* IsolateFromNeverReadOnlySpaceObject(uintptr_t obj); From 0d47d2fb370263a834c391893b4e0eba0ebc9a2d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 10:11:28 -0700 Subject: [PATCH 08/30] Use m_ for all fields --- src/bun.js/bindings/v8/V8Data.h | 2 +- .../bindings/v8/V8EscapableHandleScope.h | 2 + .../v8/V8EscapableHandleScopeBase.cpp | 16 ++--- .../bindings/v8/V8EscapableHandleScopeBase.h | 2 +- src/bun.js/bindings/v8/V8Function.cpp | 4 +- src/bun.js/bindings/v8/V8Function.h | 8 ++- src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 6 +- src/bun.js/bindings/v8/V8FunctionTemplate.h | 20 +++--- src/bun.js/bindings/v8/V8Handle.h | 64 +++++++++---------- src/bun.js/bindings/v8/V8HandleScope.cpp | 18 +++--- src/bun.js/bindings/v8/V8HandleScope.h | 22 +++---- .../bindings/v8/V8HandleScopeBuffer.cpp | 30 ++++----- src/bun.js/bindings/v8/V8HandleScopeBuffer.h | 4 +- .../bindings/v8/V8InternalFieldObject.cpp | 2 +- .../bindings/v8/V8InternalFieldObject.h | 7 +- src/bun.js/bindings/v8/V8Local.h | 16 ++--- src/bun.js/bindings/v8/V8Map.h | 18 +++--- src/bun.js/bindings/v8/V8Maybe.h | 10 +-- src/bun.js/bindings/v8/V8MaybeLocal.h | 8 +-- src/bun.js/bindings/v8/V8ObjectTemplate.cpp | 10 +-- src/bun.js/bindings/v8/V8ObjectTemplate.h | 9 ++- src/bun.js/bindings/v8/V8Oddball.h | 12 ++-- src/bun.js/bindings/v8/V8TaggedPointer.h | 18 +++--- src/bun.js/bindings/v8/v8_api_internal.cpp | 2 +- 24 files changed, 159 insertions(+), 151 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index 0d5d5a72cb469..ea92f1db9263b 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -56,7 +56,7 @@ class Data { } ObjectLayout* v8_object = reinterpret_cast(raw_ptr); - if (v8_object->map()->instance_type == InstanceType::HeapNumber) { + if (v8_object->map()->m_instanceType == InstanceType::HeapNumber) { return JSC::jsDoubleNumber(v8_object->asDouble()); } else { return JSC::JSValue(v8_object->asCell()); diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScope.h b/src/bun.js/bindings/v8/V8EscapableHandleScope.h index ad5f39ebd7752..1d111a4cc18b2 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScope.h +++ b/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"); + } diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index bb08c9da1845b..e0cf8ee8819bf 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -7,21 +7,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) { - RELEASE_ASSERT(escape_slot != nullptr, "EscapableHandleScope::Escape called multiple times"); - TaggedPointer* newHandle = prev->buffer->createHandleFromExistingObject( + RELEASE_ASSERT(m_escapeSlot != nullptr, "EscapableHandleScope::Escape called multiple times"); + TaggedPointer* newHandle = m_previousHandleScope->m_buffer->createHandleFromExistingObject( TaggedPointer::fromRaw(*escape_value), - isolate, - escape_slot); - escape_slot = nullptr; - return &newHandle->value; + m_isolate, + m_escapeSlot); + m_escapeSlot = nullptr; + return &newHandle->m_value; } } diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h index 1b82748dc7ab8..1ec7a72627dfc 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h @@ -14,7 +14,7 @@ class EscapableHandleScopeBase : public HandleScope { BUN_EXPORT uintptr_t* EscapeSlot(uintptr_t* escape_value); private: - Handle* escape_slot; + Handle* m_escapeSlot; }; } diff --git a/src/bun.js/bindings/v8/V8Function.cpp b/src/bun.js/bindings/v8/V8Function.cpp index 3ff1dc4ae1719..b1458aac822d5 100644 --- a/src/bun.js/bindings/v8/V8Function.cpp +++ b/src/bun.js/bindings/v8/V8Function.cpp @@ -37,7 +37,7 @@ void Function::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(fn, info()); Base::visitChildren(fn, visitor); - visitor.append(fn->__internals.functionTemplate); + visitor.append(fn->__internals.m_functionTemplate); } DEFINE_VISIT_CHILDREN(Function); @@ -52,7 +52,7 @@ Function* Function::create(VM& vm, Structure* structure, FunctionTemplate* funct void Function::finishCreation(VM& vm, FunctionTemplate* functionTemplate) { Base::finishCreation(vm, 0, "Function"_s); - __internals.functionTemplate.set(vm, this, functionTemplate); + __internals.m_functionTemplate.set(vm, this, functionTemplate); } void Function::SetName(Local name) diff --git a/src/bun.js/bindings/v8/V8Function.h b/src/bun.js/bindings/v8/V8Function.h index 9b2d1f552eabb..b7a715d79707f 100644 --- a/src/bun.js/bindings/v8/V8Function.h +++ b/src/bun.js/bindings/v8/V8Function.h @@ -37,17 +37,19 @@ class Function : public JSC::InternalFunction { FunctionTemplate* functionTemplate() const { - return __internals.functionTemplate.get(); + return __internals.m_functionTemplate.get(); } private: class Internals { private: - JSC::WriteBarrier functionTemplate; + JSC::WriteBarrier m_functionTemplate; friend class Function; friend class FunctionTemplate; }; + // Only access this directly in functions called by JSC code, or on a valid v8::Function + // pointer. In functions called on a Local, use internals() Internals __internals; Function(JSC::VM& vm, JSC::Structure* structure) @@ -65,6 +67,8 @@ class Function : public JSC::InternalFunction { return reinterpret_cast(this)->localToObjectPointer(); } + // Only call this in functions called on a Local. When you have a valid v8::Function + // pointer, use __internals Internals& internals() { return localToObjectPointer()->__internals; diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index df3d50a8ff036..1c1b9e8432d2e 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -85,7 +85,7 @@ void FunctionTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(fn, info()); Base::visitChildren(fn, visitor); - visitor.append(fn->__internals.data); + visitor.append(fn->__internals.m_data); } DEFINE_VISIT_CHILDREN(FunctionTemplate); @@ -108,7 +108,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb args[i + 1] = argValue.tagged(); } - Local data = hs.createLocal(vm, functionTemplate->__internals.data.get()); + Local data = hs.createLocal(vm, functionTemplate->__internals.m_data.get()); ImplicitArgs implicit_args = { .holder = nullptr, @@ -124,7 +124,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb FunctionCallbackInfo info(&implicit_args, args.data() + 1, callFrame->argumentCount()); - functionTemplate->__internals.callback(info); + functionTemplate->__internals.m_callback(info); if (implicit_args.return_value.type() != TaggedPointer::Type::Smi && implicit_args.return_value.getPtr() == nullptr) { // callback forgot to set a return value, so return undefined diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.h b/src/bun.js/bindings/v8/V8FunctionTemplate.h index 3d3fe261c188c..b1ba3399b0e0d 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.h +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.h @@ -104,25 +104,26 @@ class FunctionTemplate : public JSC::InternalFunction { FunctionCallback callback() const { - return __internals.callback; + return __internals.m_callback; } private: class Internals { private: - FunctionCallback callback; - JSC::WriteBarrier data; + FunctionCallback m_callback; + JSC::WriteBarrier m_data; - Internals(FunctionCallback callback_, JSC::VM& vm, FunctionTemplate* owner, JSC::JSValue data_) - : callback(callback_) - , data(vm, owner, data_) + Internals(FunctionCallback callback, JSC::VM& vm, FunctionTemplate* owner, JSC::JSValue data) + : m_callback(callback) + , m_data(vm, owner, data) { } friend class FunctionTemplate; }; - // only use from functions called directly on FunctionTemplate + // Only access this directly in functions called by JSC code, or on a valid v8::FunctionTemplate + // pointer. In functions called on a Local, use internals() Internals __internals; FunctionTemplate* localToObjectPointer() @@ -135,7 +136,8 @@ class FunctionTemplate : public JSC::InternalFunction { return reinterpret_cast(this)->localToObjectPointer(); } - // only use from functions called on Local + // Only call this in functions called on a Local. When you have a valid + // v8::FunctionTemplate pointer, use __internals Internals& internals() { return localToObjectPointer()->__internals; @@ -148,8 +150,6 @@ class FunctionTemplate : public JSC::InternalFunction { , Base(vm, structure, functionCall, JSC::callHostFunctionAsConstructor) { } - - // some kind of static trampoline }; } diff --git a/src/bun.js/bindings/v8/V8Handle.h b/src/bun.js/bindings/v8/V8Handle.h index 27a5497bb6661..6b19617485d80 100644 --- a/src/bun.js/bindings/v8/V8Handle.h +++ b/src/bun.js/bindings/v8/V8Handle.h @@ -7,38 +7,38 @@ namespace v8 { class ObjectLayout { private: // these two fields are laid out so that V8 can find the map - TaggedPointer tagged_map; + TaggedPointer m_taggedMap; union { JSC::WriteBarrier cell; double number; - } contents; + } m_contents; public: ObjectLayout() // using a smi value for map is most likely to catch bugs as almost every access will expect // map to be a pointer (and even if the assertion is bypassed, it'll be a null pointer) - : tagged_map(0) - , contents({ .cell = {} }) + : m_taggedMap(0) + , m_contents({ .cell = {} }) { } ObjectLayout(const Map* map_ptr, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner) - : tagged_map(const_cast(map_ptr)) - , contents({ .cell = JSC::WriteBarrier(vm, owner, cell) }) + : m_taggedMap(const_cast(map_ptr)) + , m_contents({ .cell = JSC::WriteBarrier(vm, owner, cell) }) { } ObjectLayout(double number) - : tagged_map(const_cast(&Map::heap_number_map)) - , contents({ .number = number }) + : m_taggedMap(const_cast(&Map::heap_number_map)) + , m_contents({ .number = number }) { } - const Map* map() const { return tagged_map.getPtr(); } + const Map* map() const { return m_taggedMap.getPtr(); } - double asDouble() const { return contents.number; } + double asDouble() const { return m_contents.number; } - JSC::JSCell* asCell() const { return contents.cell.get(); } + JSC::JSCell* asCell() const { return m_contents.cell.get(); } friend class Handle; friend class HandleScopeBuffer; @@ -58,25 +58,25 @@ class ObjectLayout { // the third field to get the actual object (either a JSCell* or a void*, depending on whether map // points to Map::object_map or Map::raw_ptr_map). struct Handle { - static_assert(offsetof(ObjectLayout, tagged_map) == 0, "ObjectLayout is wrong"); - static_assert(offsetof(ObjectLayout, contents) == 8, "ObjectLayout is wrong"); + static_assert(offsetof(ObjectLayout, m_taggedMap) == 0, "ObjectLayout is wrong"); + static_assert(offsetof(ObjectLayout, m_contents) == 8, "ObjectLayout is wrong"); static_assert(sizeof(ObjectLayout) == 16, "ObjectLayout is wrong"); Handle(const Map* map, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner) - : to_v8_object(&this->object) - , object(map, cell, vm, owner) + : m_toV8Object(&this->m_object) + , m_object(map, cell, vm, owner) { } Handle(double number) - : to_v8_object(&this->object) - , object(number) + : m_toV8Object(&this->m_object) + , m_object(number) { } Handle(int32_t smi) - : to_v8_object(smi) - , object() + : m_toV8Object(smi) + , m_object() { } @@ -86,34 +86,34 @@ struct Handle { } Handle(const ObjectLayout* that) - : to_v8_object(&this->object) + : m_toV8Object(&this->m_object) { - object = *that; + m_object = *that; } Handle& operator=(const Handle& that) { - object = that.object; - if (that.to_v8_object.type() == TaggedPointer::Type::Smi) { - to_v8_object = that.to_v8_object; + m_object = that.m_object; + if (that.m_toV8Object.type() == TaggedPointer::Type::Smi) { + m_toV8Object = that.m_toV8Object; } else { - to_v8_object = &this->object; + m_toV8Object = &this->m_object; } return *this; } Handle() - : to_v8_object(0) - , object() + : m_toV8Object(0) + , m_object() { } bool isCell() const { - if (to_v8_object.type() == TaggedPointer::Type::Smi) { + if (m_toV8Object.type() == TaggedPointer::Type::Smi) { return false; } - const Map* map_ptr = object.map(); + const Map* map_ptr = m_object.map(); // TODO(@190n) exhaustively switch on InstanceType if (map_ptr == &Map::object_map || map_ptr == &Map::string_map) { return true; @@ -122,13 +122,13 @@ struct Handle { return false; } else { RELEASE_ASSERT_NOT_REACHED("unknown Map at %p with instance type %" PRIx16, - map_ptr, map_ptr->instance_type); + map_ptr, map_ptr->m_instanceType); } } // if not SMI, holds &this->map so that V8 can see what kind of object this is - TaggedPointer to_v8_object; - ObjectLayout object; + TaggedPointer m_toV8Object; + ObjectLayout m_object; }; } diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index eb32591e98e9c..cac39fe2d5e07 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -5,27 +5,27 @@ namespace v8 { HandleScope::HandleScope(Isolate* isolate_) - : isolate(isolate_) - , prev(isolate->globalInternals()->currentHandleScope()) - , buffer(HandleScopeBuffer::create(isolate_->vm(), isolate_->globalInternals()->handleScopeBufferStructure(isolate_->globalObject()))) + : m_isolate(isolate_) + , m_previousHandleScope(m_isolate->globalInternals()->currentHandleScope()) + , m_buffer(HandleScopeBuffer::create(isolate_->vm(), isolate_->globalInternals()->handleScopeBufferStructure(isolate_->globalObject()))) { - isolate->globalInternals()->setCurrentHandleScope(this); + m_isolate->globalInternals()->setCurrentHandleScope(this); } HandleScope::~HandleScope() { - isolate->globalInternals()->setCurrentHandleScope(prev); - buffer->clear(); - buffer = nullptr; + m_isolate->globalInternals()->setCurrentHandleScope(m_previousHandleScope); + m_buffer->clear(); + m_buffer = nullptr; } uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t value) { auto* isolate = reinterpret_cast(i_isolate); auto* handleScope = isolate->globalInternals()->currentHandleScope(); - TaggedPointer* newSlot = handleScope->buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate); + TaggedPointer* newSlot = handleScope->m_buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate); // basically a reinterpret - return &newSlot->value; + return &newSlot->m_value; } } diff --git a/src/bun.js/bindings/v8/V8HandleScope.h b/src/bun.js/bindings/v8/V8HandleScope.h index 668f3ae1c0325..00b5be645b805 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.h +++ b/src/bun.js/bindings/v8/V8HandleScope.h @@ -19,21 +19,21 @@ class HandleScope { { // TODO(@190n) handle more types if (value.isString()) { - return Local(buffer->createHandle(value.asCell(), &Map::string_map, vm)); + return Local(m_buffer->createHandle(value.asCell(), &Map::string_map, vm)); } else if (value.isCell()) { - return Local(buffer->createHandle(value.asCell(), &Map::object_map, vm)); + return Local(m_buffer->createHandle(value.asCell(), &Map::object_map, vm)); } else if (value.isInt32()) { - return Local(buffer->createSmiHandle(value.asInt32())); + return Local(m_buffer->createSmiHandle(value.asInt32())); } else if (value.isNumber()) { - return Local(buffer->createDoubleHandle(value.asNumber())); + return Local(m_buffer->createDoubleHandle(value.asNumber())); } else if (value.isUndefined()) { - return Local(isolate->globalInternals()->undefinedSlot()); + return Local(m_isolate->globalInternals()->undefinedSlot()); } else if (value.isNull()) { - return Local(isolate->globalInternals()->nullSlot()); + return Local(m_isolate->globalInternals()->nullSlot()); } else if (value.isTrue()) { - return Local(isolate->globalInternals()->trueSlot()); + return Local(m_isolate->globalInternals()->trueSlot()); } else if (value.isFalse()) { - return Local(isolate->globalInternals()->falseSlot()); + return Local(m_isolate->globalInternals()->falseSlot()); } else { V8_UNIMPLEMENTED(); return Local(); @@ -44,9 +44,9 @@ class HandleScope { protected: // must be 24 bytes to match V8 layout - Isolate* isolate; - HandleScope* prev; - HandleScopeBuffer* buffer; + Isolate* m_isolate; + HandleScope* m_previousHandleScope; + HandleScopeBuffer* m_buffer; // is protected in v8, which matters on windows BUN_EXPORT static uintptr_t* CreateHandle(internal::Isolate* isolate, uintptr_t value); diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp index da9cc7d674925..8cee152cad401 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp @@ -29,11 +29,11 @@ void HandleScopeBuffer::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(thisObject, info()); Base::visitChildren(thisObject, visitor); - WTF::Locker locker { thisObject->gc_lock }; + WTF::Locker locker { thisObject->m_gcLock }; - for (auto& handle : thisObject->storage) { + for (auto& handle : thisObject->m_storage) { if (handle.isCell()) { - visitor.append(handle.object.contents.cell); + visitor.append(handle.m_object.m_contents.cell); } } } @@ -42,30 +42,30 @@ DEFINE_VISIT_CHILDREN(HandleScopeBuffer); Handle& HandleScopeBuffer::createEmptyHandle() { - WTF::Locker locker { gc_lock }; - storage.append(Handle {}); - return storage.last(); + WTF::Locker locker { m_gcLock }; + m_storage.append(Handle {}); + return m_storage.last(); } TaggedPointer* HandleScopeBuffer::createHandle(JSCell* ptr, const Map* map, JSC::VM& vm) { auto& handle = createEmptyHandle(); handle = Handle(map, ptr, vm, this); - return &handle.to_v8_object; + return &handle.m_toV8Object; } TaggedPointer* HandleScopeBuffer::createSmiHandle(int32_t smi) { auto& handle = createEmptyHandle(); handle = Handle(smi); - return &handle.to_v8_object; + return &handle.m_toV8Object; } TaggedPointer* HandleScopeBuffer::createDoubleHandle(double value) { auto& handle = createEmptyHandle(); handle = Handle(value); - return &handle.to_v8_object; + return &handle.m_toV8Object; } TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer address, Isolate* isolate, Handle* reuseHandle) @@ -74,13 +74,13 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a if (address.getSmi(smi)) { if (reuseHandle) { *reuseHandle = Handle(smi); - return &reuseHandle->to_v8_object; + return &reuseHandle->m_toV8Object; } else { return createSmiHandle(smi); } } else { auto* v8_object = address.getPtr(); - if (v8_object->tagged_map.getPtr()->instance_type == InstanceType::Oddball) { + if (v8_object->m_taggedMap.getPtr()->m_instanceType == InstanceType::Oddball) { // find which oddball this is for (auto& root : isolate->m_roots) { if (root == address) { @@ -91,7 +91,7 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a } if (reuseHandle) { *reuseHandle = Handle(v8_object->map(), v8_object->asCell(), vm(), this); - return &reuseHandle->to_v8_object; + return &reuseHandle->m_toV8Object; } else { return createHandle(v8_object->asCell(), v8_object->map(), vm()); } @@ -101,11 +101,11 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a void HandleScopeBuffer::clear() { // detect use-after-free of handles - WTF::Locker locker { gc_lock }; - for (auto& handle : storage) { + WTF::Locker locker { m_gcLock }; + for (auto& handle : m_storage) { handle = Handle(); } - storage.clear(); + m_storage.clear(); } } diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h index fe1abb11447dd..ffc86c18e8a90 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/V8HandleScopeBuffer.h @@ -56,8 +56,8 @@ class HandleScopeBuffer : public JSC::JSCell { friend class EscapableHandleScopeBase; private: - WTF::Lock gc_lock; - WTF::SegmentedVector storage; + WTF::Lock m_gcLock; + WTF::SegmentedVector m_storage; Handle& createEmptyHandle(); diff --git a/src/bun.js/bindings/v8/V8InternalFieldObject.cpp b/src/bun.js/bindings/v8/V8InternalFieldObject.cpp index 69dd3878da145..09bbdc4d720e9 100644 --- a/src/bun.js/bindings/v8/V8InternalFieldObject.cpp +++ b/src/bun.js/bindings/v8/V8InternalFieldObject.cpp @@ -29,7 +29,7 @@ void InternalFieldObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(thisObject, info()); Base::visitChildren(thisObject, visitor); - for (auto& value : thisObject->fields) { + for (auto& value : thisObject->m_fields) { visitor.append(value); } } diff --git a/src/bun.js/bindings/v8/V8InternalFieldObject.h b/src/bun.js/bindings/v8/V8InternalFieldObject.h index 6a118786bf178..2d483d1a4e478 100644 --- a/src/bun.js/bindings/v8/V8InternalFieldObject.h +++ b/src/bun.js/bindings/v8/V8InternalFieldObject.h @@ -26,7 +26,7 @@ class InternalFieldObject : public JSC::JSDestructibleObject { // never changes size using FieldContainer = WTF::FixedVector>; - FieldContainer* internalFields() { return &fields; } + FieldContainer* internalFields() { return &m_fields; } static InternalFieldObject* create(JSC::VM& vm, JSC::Structure* structure, Local objectTemplate); DECLARE_VISIT_CHILDREN; @@ -34,13 +34,12 @@ class InternalFieldObject : public JSC::JSDestructibleObject { protected: InternalFieldObject(JSC::VM& vm, JSC::Structure* structure, int internalFieldCount) : Base(vm, structure) - , fields(internalFieldCount, JSC::WriteBarrier(vm, this, JSC::jsUndefined())) + , m_fields(internalFieldCount, JSC::WriteBarrier(vm, this, JSC::jsUndefined())) { } private: - // TODO(@190n) use template with fixed size array for small counts - FieldContainer fields; + FieldContainer m_fields; }; } diff --git a/src/bun.js/bindings/v8/V8Local.h b/src/bun.js/bindings/v8/V8Local.h index a2f23ec1b496a..1408d8e12bfa7 100644 --- a/src/bun.js/bindings/v8/V8Local.h +++ b/src/bun.js/bindings/v8/V8Local.h @@ -10,33 +10,33 @@ template class Local final { public: Local() - : location(nullptr) + : m_location(nullptr) { } Local(TaggedPointer* slot) - : location(slot) + : m_location(slot) { } - bool IsEmpty() const { return location == nullptr; } + bool IsEmpty() const { return m_location == nullptr; } - T* operator*() const { return reinterpret_cast(location); } - T* operator->() const { return reinterpret_cast(location); } + T* operator*() const { return reinterpret_cast(m_location); } + T* operator->() const { return reinterpret_cast(m_location); } template Local reinterpret() const { - return Local(location); + return Local(m_location); } TaggedPointer tagged() const { - return *location; + return *m_location; } private: - TaggedPointer* location; + TaggedPointer* m_location; }; } diff --git a/src/bun.js/bindings/v8/V8Map.h b/src/bun.js/bindings/v8/V8Map.h index 5c9e672ff3d28..4bd5b12a657bb 100644 --- a/src/bun.js/bindings/v8/V8Map.h +++ b/src/bun.js/bindings/v8/V8Map.h @@ -23,14 +23,14 @@ enum class InstanceType : uint16_t { // V8's description of the structure of an object struct Map { // the structure of the map itself (always points to map_map) - TaggedPointer meta_map; + TaggedPointer m_metaMap; // TBD whether we need to put anything here to please inlined V8 functions - uint32_t unused; + uint32_t m_unused; // describes which kind of object this is. we shouldn't actually need to create very many // instance types -- only ones for primitives, and one to make sure V8 thinks it cannot take the // fast path when accessing internal fields // (v8::internal::Internals::CanHaveInternalField, in v8-internal.h) - InstanceType instance_type; + InstanceType m_instanceType; // Since maps are V8 objects, they each also have a map pointer at the start, which is this one static const Map map_map; @@ -46,16 +46,16 @@ struct Map { // are numbers. static const Map heap_number_map; - Map(InstanceType instance_type_) - : meta_map(const_cast(&map_map)) - , unused(0xaaaaaaaa) - , instance_type(instance_type_) + Map(InstanceType instance_type) + : m_metaMap(const_cast(&map_map)) + , m_unused(0xaaaaaaaa) + , m_instanceType(instance_type) { } }; static_assert(sizeof(Map) == 16, "Map has wrong layout"); -static_assert(offsetof(Map, meta_map) == 0, "Map has wrong layout"); -static_assert(offsetof(Map, instance_type) == 12, "Map has wrong layout"); +static_assert(offsetof(Map, m_metaMap) == 0, "Map has wrong layout"); +static_assert(offsetof(Map, m_instanceType) == 12, "Map has wrong layout"); } diff --git a/src/bun.js/bindings/v8/V8Maybe.h b/src/bun.js/bindings/v8/V8Maybe.h index 6ad41688adb8f..2c0f52f5ec716 100644 --- a/src/bun.js/bindings/v8/V8Maybe.h +++ b/src/bun.js/bindings/v8/V8Maybe.h @@ -6,16 +6,16 @@ template class Maybe { public: Maybe() - : has_value(false) + : m_hasValue(false) { } explicit Maybe(const T& t) - : has_value(true) - , value(t) + : m_hasValue(true) + , m_value(t) { } - bool has_value; - T value; + bool m_hasValue; + T m_value; }; template diff --git a/src/bun.js/bindings/v8/V8MaybeLocal.h b/src/bun.js/bindings/v8/V8MaybeLocal.h index f53a6feedbf94..82cdad09bc6b5 100644 --- a/src/bun.js/bindings/v8/V8MaybeLocal.h +++ b/src/bun.js/bindings/v8/V8MaybeLocal.h @@ -8,17 +8,17 @@ template class MaybeLocal { public: MaybeLocal() - : local_(Local()) {}; + : m_local(Local()) {}; template MaybeLocal(Local that) - : local_(that) + : m_local(that) { } - bool IsEmpty() const { return local_.IsEmpty(); } + bool IsEmpty() const { return m_local.IsEmpty(); } private: - Local local_; + Local m_local; }; } diff --git a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp index 07b32031f13fe..46cd993268d24 100644 --- a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp +++ b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp @@ -17,7 +17,7 @@ namespace v8 { void ObjectTemplate::finishCreation(JSC::VM& vm) { Base::finishCreation(vm); - __internals.objectStructure.initLater([](const LazyProperty::Initializer& init) { + __internals.m_objectStructure.initLater([](const LazyProperty::Initializer& init) { init.set(JSC::Structure::create( init.vm, init.owner->globalObject(), @@ -61,7 +61,7 @@ MaybeLocal ObjectTemplate::NewInstance(Local context) // get a structure // must take thisObj because JSC needs the native pointer - auto structure = internals().objectStructure.get(thisObj); + auto structure = internals().m_objectStructure.get(thisObj); // create object from it // InternalFieldObject needs a Local, which we can create using the `this` @@ -80,19 +80,19 @@ void ObjectTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(fn, info()); Base::visitChildren(fn, visitor); - fn->__internals.objectStructure.visit(visitor); + fn->__internals.m_objectStructure.visit(visitor); } DEFINE_VISIT_CHILDREN(ObjectTemplate); void ObjectTemplate::SetInternalFieldCount(int value) { - internals().internalFieldCount = value; + internals().m_internalFieldCount = value; } int ObjectTemplate::InternalFieldCount() const { - return internals().internalFieldCount; + return internals().m_internalFieldCount; } Structure* ObjectTemplate::createStructure(JSC::VM& vm, JSGlobalObject* globalObject, JSValue prototype) diff --git a/src/bun.js/bindings/v8/V8ObjectTemplate.h b/src/bun.js/bindings/v8/V8ObjectTemplate.h index b8d92118f7f73..4659c624a94fd 100644 --- a/src/bun.js/bindings/v8/V8ObjectTemplate.h +++ b/src/bun.js/bindings/v8/V8ObjectTemplate.h @@ -44,12 +44,13 @@ class ObjectTemplate : public JSC::InternalFunction { private: class Internals { - int internalFieldCount = 0; - JSC::LazyProperty objectStructure; + int m_internalFieldCount = 0; + JSC::LazyProperty m_objectStructure; friend class ObjectTemplate; }; - // do not use directly inside exported V8 functions, use internals() + // Only access this directly in functions called by JSC code, or on a valid v8::ObjectTemplate + // pointer. In functions called on a Local, use internals() Internals __internals; ObjectTemplate* localToObjectPointer() @@ -62,6 +63,8 @@ class ObjectTemplate : public JSC::InternalFunction { return reinterpret_cast(this)->localToObjectPointer(); } + // Only call this in functions called on a Local. When you have a valid + // v8::ObjectTemplate pointer, use __internals Internals& internals() { return localToObjectPointer()->__internals; diff --git a/src/bun.js/bindings/v8/V8Oddball.h b/src/bun.js/bindings/v8/V8Oddball.h index 511ece34c5d74..2ca1d869d5678 100644 --- a/src/bun.js/bindings/v8/V8Oddball.h +++ b/src/bun.js/bindings/v8/V8Oddball.h @@ -14,13 +14,13 @@ struct Oddball { kFalse = 0, }; - TaggedPointer map; - uintptr_t unused[4]; - TaggedPointer kind; + TaggedPointer m_map; + uintptr_t m_unused[4]; + TaggedPointer m_kind; - Oddball(Kind kind_) - : map(const_cast(&Map::oddball_map)) - , kind(TaggedPointer(static_cast(kind_))) + Oddball(Kind kind) + : m_map(const_cast(&Map::oddball_map)) + , m_kind(TaggedPointer(static_cast(kind))) { } }; diff --git a/src/bun.js/bindings/v8/V8TaggedPointer.h b/src/bun.js/bindings/v8/V8TaggedPointer.h index 370b67bbe9c32..ddb428be5b77f 100644 --- a/src/bun.js/bindings/v8/V8TaggedPointer.h +++ b/src/bun.js/bindings/v8/V8TaggedPointer.h @@ -5,7 +5,7 @@ namespace v8 { struct TaggedPointer { - uintptr_t value; + uintptr_t m_value; enum class Type : uint8_t { Smi, @@ -17,10 +17,10 @@ struct TaggedPointer { : TaggedPointer(nullptr) {}; TaggedPointer(const TaggedPointer&) = default; TaggedPointer& operator=(const TaggedPointer&) = default; - bool operator==(const TaggedPointer& other) const { return value == other.value; } + bool operator==(const TaggedPointer& other) const { return m_value == other.m_value; } TaggedPointer(void* ptr, bool weak) - : value(reinterpret_cast(ptr) | (weak ? 3 : 1)) + : m_value(reinterpret_cast(ptr) | (weak ? 3 : 1)) { RELEASE_ASSERT((reinterpret_cast(ptr) & 3) == 0); } @@ -31,20 +31,20 @@ struct TaggedPointer { } TaggedPointer(int32_t smi) - : value(static_cast(smi) << 32) + : m_value(static_cast(smi) << 32) { } static TaggedPointer fromRaw(uintptr_t raw) { TaggedPointer tagged; - tagged.value = raw; + tagged.m_value = raw; return tagged; } Type type() const { - switch (value & 3) { + switch (m_value & 3) { case 0: return Type::Smi; case 1: @@ -61,7 +61,7 @@ struct TaggedPointer { if (type() == Type::Smi) { return nullptr; } - return reinterpret_cast(value & ~3ull); + return reinterpret_cast(m_value & ~3ull); } bool getSmi(int32_t& smi) const @@ -69,14 +69,14 @@ struct TaggedPointer { if (type() != Type::Smi) { return false; } - smi = static_cast(value >> 32); + smi = static_cast(m_value >> 32); return true; } int32_t getSmiUnchecked() const { ASSERT(type() == Type::Smi); - return static_cast(value >> 32); + return static_cast(m_value >> 32); } }; diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index d5b35de82bae5..75a4abd6b2053 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -16,7 +16,7 @@ uintptr_t* GlobalizeReference(internal::Isolate* i_isolate, uintptr_t address) auto* isolate = reinterpret_cast(i_isolate); auto* globalHandles = isolate->globalInternals()->globalHandles(); TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), isolate); - return &newSlot->value; + return &newSlot->m_value; } void DisposeGlobal(uintptr_t* location) From cba2a95541c3715059e5599b8558387e64aadbfa Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 10:19:51 -0700 Subject: [PATCH 09/30] Merge v8::TaggedPointer constructors --- src/bun.js/bindings/v8/V8TaggedPointer.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/bun.js/bindings/v8/V8TaggedPointer.h b/src/bun.js/bindings/v8/V8TaggedPointer.h index ddb428be5b77f..2ad827f25f2da 100644 --- a/src/bun.js/bindings/v8/V8TaggedPointer.h +++ b/src/bun.js/bindings/v8/V8TaggedPointer.h @@ -19,17 +19,12 @@ struct TaggedPointer { TaggedPointer& operator=(const TaggedPointer&) = default; bool operator==(const TaggedPointer& other) const { return m_value == other.m_value; } - TaggedPointer(void* ptr, bool weak) + TaggedPointer(void* ptr, bool weak = false) : m_value(reinterpret_cast(ptr) | (weak ? 3 : 1)) { RELEASE_ASSERT((reinterpret_cast(ptr) & 3) == 0); } - TaggedPointer(void* ptr) - : TaggedPointer(ptr, false) - { - } - TaggedPointer(int32_t smi) : m_value(static_cast(smi) << 32) { From e2ed8eaf1f617229ca4bb5ad2b527fbb83c53e28 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 14:28:59 -0700 Subject: [PATCH 10/30] Move Bun-specific V8 classes into namespace v8::shim The v8 namespace was confusingly polluted with both classes that exist in the actual V8 API, as well as new classes to support Bun's specific implementation of that API. This commit moves the latter into the namespace v8::shim. It also makes a duplicate of classes with fields in the v8::shim namespace. For those classes, the v8:: version has no fields, no JSC superclass, and only implements the actual V8 API functions, while the v8::shim:: version has the inheritance and functions necessary to be allocated as a JSC object and implements helper functions to support the V8 APIs. --- CMakeLists.txt | 1 + src/bun.js/bindings/ZigGlobalObject.cpp | 8 +- src/bun.js/bindings/ZigGlobalObject.h | 6 +- src/bun.js/bindings/v8/V8Array.cpp | 2 +- src/bun.js/bindings/v8/V8Array.h | 2 +- src/bun.js/bindings/v8/V8Boolean.cpp | 2 +- src/bun.js/bindings/v8/V8Boolean.h | 2 +- src/bun.js/bindings/v8/V8Context.cpp | 2 +- src/bun.js/bindings/v8/V8Context.h | 4 +- src/bun.js/bindings/v8/V8Data.h | 17 +-- .../bindings/v8/V8EscapableHandleScope.cpp | 2 +- .../bindings/v8/V8EscapableHandleScope.h | 2 +- .../v8/V8EscapableHandleScopeBase.cpp | 2 +- .../bindings/v8/V8EscapableHandleScopeBase.h | 6 +- src/bun.js/bindings/v8/V8External.cpp | 2 +- src/bun.js/bindings/v8/V8External.h | 2 +- src/bun.js/bindings/v8/V8Function.cpp | 57 +-------- src/bun.js/bindings/v8/V8Function.h | 67 ++--------- .../bindings/v8/V8FunctionCallbackInfo.h | 43 +++++++ src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 113 ++++-------------- src/bun.js/bindings/v8/V8FunctionTemplate.h | 111 ++--------------- src/bun.js/bindings/v8/V8HandleScope.cpp | 12 +- src/bun.js/bindings/v8/V8HandleScope.h | 13 +- src/bun.js/bindings/v8/V8Isolate.cpp | 6 +- src/bun.js/bindings/v8/V8Isolate.h | 11 +- src/bun.js/bindings/v8/V8Local.h | 4 +- src/bun.js/bindings/v8/V8Maybe.h | 2 +- src/bun.js/bindings/v8/V8MaybeLocal.h | 2 +- src/bun.js/bindings/v8/V8Number.cpp | 2 +- src/bun.js/bindings/v8/V8Number.h | 2 +- src/bun.js/bindings/v8/V8Object.cpp | 8 +- src/bun.js/bindings/v8/V8Object.h | 2 +- src/bun.js/bindings/v8/V8ObjectTemplate.cpp | 85 ++----------- src/bun.js/bindings/v8/V8ObjectTemplate.h | 66 ++-------- src/bun.js/bindings/v8/V8Primitive.h | 2 +- src/bun.js/bindings/v8/V8Signature.h | 2 +- src/bun.js/bindings/v8/V8String.cpp | 2 +- src/bun.js/bindings/v8/V8String.h | 2 +- src/bun.js/bindings/v8/V8Template.cpp | 2 +- src/bun.js/bindings/v8/V8Template.h | 2 +- src/bun.js/bindings/v8/V8Value.cpp | 2 +- src/bun.js/bindings/v8/V8Value.h | 2 +- src/bun.js/bindings/v8/node.cpp | 2 +- src/bun.js/bindings/v8/node.h | 2 +- src/bun.js/bindings/v8/shim/Function.cpp | 63 ++++++++++ src/bun.js/bindings/v8/shim/Function.h | 49 ++++++++ .../bindings/v8/shim/FunctionTemplate.cpp | 101 ++++++++++++++++ .../bindings/v8/shim/FunctionTemplate.h | 51 ++++++++ .../GlobalInternals.cpp} | 16 +-- .../GlobalInternals.h} | 15 ++- .../bindings/v8/{V8Handle.h => shim/Handle.h} | 7 +- .../HandleScopeBuffer.cpp} | 10 +- .../HandleScopeBuffer.h} | 17 ++- .../InternalFieldObject.cpp} | 12 +- .../InternalFieldObject.h} | 11 +- .../bindings/v8/{V8Map.cpp => shim/Map.cpp} | 6 +- .../bindings/v8/{V8Map.h => shim/Map.h} | 6 +- .../bindings/v8/shim/ObjectTemplate.cpp | 76 ++++++++++++ src/bun.js/bindings/v8/shim/ObjectTemplate.h | 61 ++++++++++ .../v8/{V8Oddball.h => shim/Oddball.h} | 8 +- .../TaggedPointer.h} | 14 ++- src/bun.js/bindings/v8/v8.h | 6 + src/bun.js/bindings/v8/v8_api_internal.cpp | 9 +- src/bun.js/bindings/v8/v8_api_internal.h | 4 +- src/bun.js/bindings/v8/v8_internal.cpp | 4 +- src/bun.js/bindings/v8/v8_internal.h | 4 +- 66 files changed, 678 insertions(+), 560 deletions(-) create mode 100644 src/bun.js/bindings/v8/V8FunctionCallbackInfo.h create mode 100644 src/bun.js/bindings/v8/shim/Function.cpp create mode 100644 src/bun.js/bindings/v8/shim/Function.h create mode 100644 src/bun.js/bindings/v8/shim/FunctionTemplate.cpp create mode 100644 src/bun.js/bindings/v8/shim/FunctionTemplate.h rename src/bun.js/bindings/v8/{V8GlobalInternals.cpp => shim/GlobalInternals.cpp} (91%) rename src/bun.js/bindings/v8/{V8GlobalInternals.h => shim/GlobalInternals.h} (95%) rename src/bun.js/bindings/v8/{V8Handle.h => shim/Handle.h} (97%) rename src/bun.js/bindings/v8/{V8HandleScopeBuffer.cpp => shim/HandleScopeBuffer.cpp} (95%) rename src/bun.js/bindings/v8/{V8HandleScopeBuffer.h => shim/HandleScopeBuffer.h} (90%) rename src/bun.js/bindings/v8/{V8InternalFieldObject.cpp => shim/InternalFieldObject.cpp} (80%) rename src/bun.js/bindings/v8/{V8InternalFieldObject.h => shim/InternalFieldObject.h} (91%) rename src/bun.js/bindings/v8/{V8Map.cpp => shim/Map.cpp} (81%) rename src/bun.js/bindings/v8/{V8Map.h => shim/Map.h} (96%) create mode 100644 src/bun.js/bindings/v8/shim/ObjectTemplate.cpp create mode 100644 src/bun.js/bindings/v8/shim/ObjectTemplate.h rename src/bun.js/bindings/v8/{V8Oddball.h => shim/Oddball.h} (80%) rename src/bun.js/bindings/v8/{V8TaggedPointer.h => shim/TaggedPointer.h} (91%) diff --git a/CMakeLists.txt b/CMakeLists.txt index f2b6f85f119d4..dcbeb52067793 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -668,6 +668,7 @@ file(GLOB BUN_CPP ${CONFIGURE_DEPENDS} "${BUN_SRC}/bun.js/bindings/webcrypto/*.cpp" "${BUN_SRC}/bun.js/bindings/webcrypto/*/*.cpp" "${BUN_SRC}/bun.js/bindings/v8/*.cpp" + "${BUN_SRC}/bun.js/bindings/v8/shim/*.cpp" "${BUN_SRC}/deps/picohttpparser/picohttpparser.c" ) list(APPEND BUN_RAW_SOURCES ${BUN_CPP}) diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 78cf870c59521..d5f2cf18446b3 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -148,7 +148,7 @@ #include "Base64Helpers.h" #include "wtf/text/OrdinalNumber.h" #include "ErrorCode.h" -#include "v8/V8GlobalInternals.h" +#include "v8/shim/GlobalInternals.h" #if ENABLE(REMOTE_INSPECTOR) #include "JavaScriptCore/RemoteInspectorServer.h" @@ -2686,11 +2686,11 @@ void GlobalObject::finishCreation(VM& vm) }); m_V8GlobalInternals.initLater( - [](const JSC::LazyProperty::Initializer& init) { + [](const JSC::LazyProperty::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(init.owner))); }); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 96eaddc9541e1..cc75c8255f114 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -32,7 +32,9 @@ class InternalModuleRegistry; } // namespace Bun namespace v8 { +namespace shim { class GlobalInternals; +} // namespace shim } // namespace v8 #include "root.h" @@ -286,7 +288,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(); } @@ -565,7 +567,7 @@ class GlobalObject : public Bun::GlobalScope { LazyProperty m_NapiPrototypeStructure; LazyProperty m_NAPIFunctionStructure; LazyProperty m_JSSQLStatementStructure; - LazyProperty m_V8GlobalInternals; + LazyProperty m_V8GlobalInternals; LazyProperty m_bunObject; LazyProperty m_cryptoObject; diff --git a/src/bun.js/bindings/v8/V8Array.cpp b/src/bun.js/bindings/v8/V8Array.cpp index 97dd9cf31925d..0af06548d4201 100644 --- a/src/bun.js/bindings/v8/V8Array.cpp +++ b/src/bun.js/bindings/v8/V8Array.cpp @@ -20,4 +20,4 @@ Local Array::New(Isolate* isolate, Local* elements, size_t length) return isolate->currentHandleScope()->createLocal(isolate->vm(), array); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Array.h b/src/bun.js/bindings/v8/V8Array.h index d88418894e899..3e673b030892a 100644 --- a/src/bun.js/bindings/v8/V8Array.h +++ b/src/bun.js/bindings/v8/V8Array.h @@ -13,4 +13,4 @@ class Array : public Object { BUN_EXPORT static Local New(Isolate* isolate, Local* elements, size_t length); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Boolean.cpp b/src/bun.js/bindings/v8/V8Boolean.cpp index 3bc95e043c978..05374dbc980df 100644 --- a/src/bun.js/bindings/v8/V8Boolean.cpp +++ b/src/bun.js/bindings/v8/V8Boolean.cpp @@ -13,4 +13,4 @@ Local Boolean::New(Isolate* isolate, bool value) return isolate->currentHandleScope()->createLocal(isolate->vm(), JSC::jsBoolean(value)); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Boolean.h b/src/bun.js/bindings/v8/V8Boolean.h index 9dd76f124a575..e90244d1b04c6 100644 --- a/src/bun.js/bindings/v8/V8Boolean.h +++ b/src/bun.js/bindings/v8/V8Boolean.h @@ -12,4 +12,4 @@ class Boolean : public Primitive { BUN_EXPORT static Local New(Isolate* isolate, bool value); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Context.cpp b/src/bun.js/bindings/v8/V8Context.cpp index 9efe2a0ac66be..2019de4eaec2c 100644 --- a/src/bun.js/bindings/v8/V8Context.cpp +++ b/src/bun.js/bindings/v8/V8Context.cpp @@ -7,4 +7,4 @@ Isolate* Context::GetIsolate() return globalObject()->V8GlobalInternals()->isolate(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Context.h b/src/bun.js/bindings/v8/V8Context.h index efaae1a175fda..164513277e47f 100644 --- a/src/bun.js/bindings/v8/V8Context.h +++ b/src/bun.js/bindings/v8/V8Context.h @@ -5,7 +5,9 @@ namespace v8 { +namespace shim { class Isolate; +} // Context is always a reinterpret pointer to Zig::GlobalObject, so that functions accepting a // Context can quickly access JSC data @@ -34,4 +36,4 @@ class Context : public Data { }; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index ea92f1db9263b..ea12cfa986c00 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -1,13 +1,14 @@ #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 @@ -36,7 +37,7 @@ class Data { } // Get this as a JSValue when this is a v8::Local - JSC::JSValue localToJSValue(GlobalInternals* globalInternals) const + JSC::JSValue localToJSValue(shim::GlobalInternals* globalInternals) const { TaggedPointer root = *reinterpret_cast(this); if (root.type() == TaggedPointer::Type::Smi) { @@ -55,8 +56,8 @@ class Data { return JSC::jsBoolean(false); } - ObjectLayout* v8_object = reinterpret_cast(raw_ptr); - if (v8_object->map()->m_instanceType == InstanceType::HeapNumber) { + shim::ObjectLayout* v8_object = reinterpret_cast(raw_ptr); + if (v8_object->map()->m_instanceType == shim::InstanceType::HeapNumber) { return JSC::jsDoubleNumber(v8_object->asDouble()); } else { return JSC::JSValue(v8_object->asCell()); @@ -97,10 +98,10 @@ class Data { // 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(); + shim::ObjectLayout* v8_object = root.getPtr(); return TaggedPointer(v8_object->asCell()); } } }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp index c5e7bcb9134b7..6208c077c1f82 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp @@ -12,4 +12,4 @@ EscapableHandleScope::~EscapableHandleScope() EscapableHandleScopeBase::~EscapableHandleScopeBase(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScope.h b/src/bun.js/bindings/v8/V8EscapableHandleScope.h index 1d111a4cc18b2..75f44470c17d9 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScope.h +++ b/src/bun.js/bindings/v8/V8EscapableHandleScope.h @@ -12,4 +12,4 @@ class EscapableHandleScope : public EscapableHandleScopeBase { static_assert(sizeof(EscapableHandleScope) == 32, "EscapableHandleScope has wrong layout"); -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index e0cf8ee8819bf..cb24b65e19af2 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -24,4 +24,4 @@ uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value) return &newHandle->m_value; } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h index 1ec7a72627dfc..097f7aa0a5722 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.h @@ -1,8 +1,8 @@ #pragma once #include "v8.h" -#include "V8HandleScope.h" #include "V8Isolate.h" +#include "V8HandleScope.h" namespace v8 { @@ -14,7 +14,7 @@ class EscapableHandleScopeBase : public HandleScope { BUN_EXPORT uintptr_t* EscapeSlot(uintptr_t* escape_value); private: - Handle* m_escapeSlot; + shim::Handle* m_escapeSlot; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8External.cpp b/src/bun.js/bindings/v8/V8External.cpp index 64a437490a080..7d21c67ed153c 100644 --- a/src/bun.js/bindings/v8/V8External.cpp +++ b/src/bun.js/bindings/v8/V8External.cpp @@ -23,4 +23,4 @@ void* External::Value() const return external->value(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8External.h b/src/bun.js/bindings/v8/V8External.h index 8c01c3db66a9e..3d9a0fccae3c1 100644 --- a/src/bun.js/bindings/v8/V8External.h +++ b/src/bun.js/bindings/v8/V8External.h @@ -13,4 +13,4 @@ class External : public Value { BUN_EXPORT void* Value() const; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Function.cpp b/src/bun.js/bindings/v8/V8Function.cpp index b1458aac822d5..37bee7fb16738 100644 --- a/src/bun.js/bindings/v8/V8Function.cpp +++ b/src/bun.js/bindings/v8/V8Function.cpp @@ -1,64 +1,13 @@ #include "V8Function.h" -#include "V8FunctionTemplate.h" - -#include "JavaScriptCore/FunctionPrototype.h" - -using JSC::Structure; -using JSC::VM; +#include "shim/Function.h" namespace v8 { -// for CREATE_METHOD_TABLE -namespace JSCastingHelpers = JSC::JSCastingHelpers; - -const JSC::ClassInfo Function::s_info = { - "Function"_s, - &Base::s_info, - nullptr, - nullptr, - CREATE_METHOD_TABLE(Function) -}; - -Structure* Function::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) -{ - return Structure::create( - vm, - globalObject, - globalObject->functionPrototype(), - JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), - info()); -} - -template -void Function::visitChildrenImpl(JSCell* cell, Visitor& visitor) -{ - Function* fn = jsCast(cell); - ASSERT_GC_OBJECT_INHERITS(fn, info()); - Base::visitChildren(fn, visitor); - - visitor.append(fn->__internals.m_functionTemplate); -} - -DEFINE_VISIT_CHILDREN(Function); - -Function* Function::create(VM& vm, Structure* structure, FunctionTemplate* functionTemplate) -{ - auto* function = new (NotNull, JSC::allocateCell(vm)) Function(vm, structure); - function->finishCreation(vm, functionTemplate); - return function; -} - -void Function::finishCreation(VM& vm, FunctionTemplate* functionTemplate) -{ - Base::finishCreation(vm, 0, "Function"_s); - __internals.m_functionTemplate.set(vm, this, functionTemplate); -} - void Function::SetName(Local name) { auto* thisObj = localToObjectPointer(); - thisObj->m_originalName.set(Isolate::GetCurrent()->vm(), thisObj, name->localToJSString()); + thisObj->setName(name->localToJSString()); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Function.h b/src/bun.js/bindings/v8/V8Function.h index b7a715d79707f..f4d801a329c3b 100644 --- a/src/bun.js/bindings/v8/V8Function.h +++ b/src/bun.js/bindings/v8/V8Function.h @@ -4,77 +4,24 @@ #include "V8FunctionTemplate.h" #include "V8Local.h" #include "V8String.h" +#include "shim/Function.h" namespace v8 { -// If this inherited Object like it does in V8, the layout would be wrong for JSC HeapCell. -// Inheritance shouldn't matter for the ABI. -class Function : public JSC::InternalFunction { +class Function : public Object { public: - using Base = JSC::InternalFunction; - - static Function* create(JSC::VM& vm, JSC::Structure* structure, FunctionTemplate* functionTemplate); - - DECLARE_INFO; - DECLARE_VISIT_CHILDREN; - - static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); - - template - static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) - { - if constexpr (mode == JSC::SubspaceAccess::Concurrently) - return nullptr; - return WebCore::subspaceForImpl( - vm, - [](auto& spaces) { return spaces.m_clientSubspaceForV8Function.get(); }, - [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForV8Function = std::forward(space); }, - [](auto& spaces) { return spaces.m_subspaceForV8Function.get(); }, - [](auto& spaces, auto&& space) { spaces.m_subspaceForV8Function = std::forward(space); }); - } - BUN_EXPORT void SetName(Local name); - FunctionTemplate* functionTemplate() const - { - return __internals.m_functionTemplate.get(); - } - private: - class Internals { - private: - JSC::WriteBarrier m_functionTemplate; - friend class Function; - friend class FunctionTemplate; - }; - - // Only access this directly in functions called by JSC code, or on a valid v8::Function - // pointer. In functions called on a Local, use internals() - Internals __internals; - - Function(JSC::VM& vm, JSC::Structure* structure) - : Base(vm, structure, FunctionTemplate::functionCall) - { - } - - Function* localToObjectPointer() + shim::Function* localToObjectPointer() { - return reinterpret_cast(this)->localToObjectPointer(); + return Data::localToObjectPointer(); } - const Function* localToObjectPointer() const + const shim::Function* localToObjectPointer() const { - return reinterpret_cast(this)->localToObjectPointer(); + return Data::localToObjectPointer(); } - - // Only call this in functions called on a Local. When you have a valid v8::Function - // pointer, use __internals - Internals& internals() - { - return localToObjectPointer()->__internals; - } - - void finishCreation(JSC::VM& vm, FunctionTemplate* functionTemplate); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h new file mode 100644 index 0000000000000..194af693619fd --- /dev/null +++ b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h @@ -0,0 +1,43 @@ +#pragma once + +#include "shim/TaggedPointer.h" + +namespace v8 { + +class Isolate; +class Context; +class Value; + +struct ImplicitArgs { + // v8-function-callback.h:168 + void* holder; + Isolate* isolate; + Context* context; + // overwritten by the callback + TaggedPointer return_value; + // holds the value passed for data in FunctionTemplate::New + TaggedPointer target; + void* new_target; +}; + +// T = return value +template +class FunctionCallbackInfo { + // V8 treats this as an array of pointers + ImplicitArgs* implicit_args; + // index -1 is this + TaggedPointer* values; + int length; + +public: + FunctionCallbackInfo(ImplicitArgs* implicit_args_, TaggedPointer* values_, int length_) + : implicit_args(implicit_args_) + , values(values_) + , length(length_) + { + } +}; + +using FunctionCallback = void (*)(const FunctionCallbackInfo&); + +} diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index 1c1b9e8432d2e..1cbe29ef61615 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -1,8 +1,7 @@ #include "V8FunctionTemplate.h" #include "V8Function.h" #include "V8HandleScope.h" - -#include "JavaScriptCore/FunctionPrototype.h" +#include "shim/FunctionTemplate.h" using JSC::JSCell; using JSC::JSValue; @@ -10,17 +9,6 @@ using JSC::Structure; namespace v8 { -// for CREATE_METHOD_TABLE -namespace JSCastingHelpers = JSC::JSCastingHelpers; - -const JSC::ClassInfo FunctionTemplate::s_info = { - "FunctionTemplate"_s, - &Base::s_info, - nullptr, - nullptr, - CREATE_METHOD_TABLE(FunctionTemplate) -}; - Local FunctionTemplate::New( Isolate* isolate, FunctionCallback callback, @@ -36,14 +24,22 @@ Local FunctionTemplate::New( { // only handling simpler cases for now // (pass most of these into v8::Function / JSC::InternalFunction) - RELEASE_ASSERT(signature.IsEmpty()); - RELEASE_ASSERT(length == 0); - RELEASE_ASSERT(behavior == ConstructorBehavior::kAllow); - RELEASE_ASSERT(side_effect_type == SideEffectType::kHasSideEffect); - RELEASE_ASSERT(c_function == nullptr); - RELEASE_ASSERT(instance_type == 0); - RELEASE_ASSERT(allowed_receiver_instance_type_range_start == 0); - RELEASE_ASSERT(allowed_receiver_instance_type_range_end == 0); + RELEASE_ASSERT_WITH_MESSAGE(signature.IsEmpty(), + "Passing signature to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(length == 0, + "Passing length to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(behavior == ConstructorBehavior::kAllow, + "Passing behavior to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(side_effect_type == SideEffectType::kHasSideEffect, + "Passing side_effect_type to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(c_function == nullptr, + "Passing c_function to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(instance_type == 0, + "Passing instance_type to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(allowed_receiver_instance_type_range_start == 0, + "Passing allowed_receiver_instance_type_range_start to FunctionTemplate::New is not yet supported"); + RELEASE_ASSERT_WITH_MESSAGE(allowed_receiver_instance_type_range_end == 0, + "Passing allowed_receiver_instance_type_range_end to FunctionTemplate::New is not yet supported"); auto globalObject = isolate->globalObject(); auto& vm = globalObject->vm(); @@ -51,9 +47,7 @@ Local FunctionTemplate::New( JSValue jsc_data = data.IsEmpty() ? JSC::jsUndefined() : data->localToJSValue(globalInternals); Structure* structure = globalInternals->functionTemplateStructure(globalObject); - auto* functionTemplate = new (NotNull, JSC::allocateCell(vm)) FunctionTemplate( - vm, structure, callback, jsc_data); - functionTemplate->finishCreation(vm); + auto* functionTemplate = shim::FunctionTemplate::create(vm, structure, callback, jsc_data); return globalInternals->currentHandleScope()->createLocal(vm, functionTemplate); } @@ -63,76 +57,9 @@ MaybeLocal FunctionTemplate::GetFunction(Local context) auto& vm = context->vm(); auto* globalObject = context->globalObject(); auto* globalInternals = globalObject->V8GlobalInternals(); - auto* f = Function::create(vm, globalInternals->v8FunctionStructure(globalObject), localToObjectPointer()); + auto* f = shim::Function::create(vm, globalInternals->v8FunctionStructure(globalObject), localToObjectPointer()); return globalInternals->currentHandleScope()->createLocal(vm, f); } -Structure* FunctionTemplate::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) -{ - return Structure::create( - vm, - globalObject, - globalObject->functionPrototype(), - JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), - info()); -} - -template -void FunctionTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) -{ - FunctionTemplate* fn = jsCast(cell); - ASSERT_GC_OBJECT_INHERITS(fn, info()); - Base::visitChildren(fn, visitor); - - visitor.append(fn->__internals.m_data); -} - -DEFINE_VISIT_CHILDREN(FunctionTemplate); - -JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame) -{ - auto* callee = JSC::jsDynamicCast(callFrame->jsCallee()); - auto* functionTemplate = callee->functionTemplate(); - auto* isolate = JSC::jsCast(globalObject)->V8GlobalInternals()->isolate(); - auto& vm = globalObject->vm(); - - WTF::Vector args(callFrame->argumentCount() + 1); - - HandleScope hs(isolate); - Local thisValue = hs.createLocal(vm, callFrame->thisValue()); - args[0] = thisValue.tagged(); - - for (size_t i = 0; i < callFrame->argumentCount(); i++) { - Local argValue = hs.createLocal(vm, callFrame->argument(i)); - args[i + 1] = argValue.tagged(); - } - - Local data = hs.createLocal(vm, functionTemplate->__internals.m_data.get()); - - ImplicitArgs implicit_args = { - .holder = nullptr, - .isolate = isolate, - // set to nullptr to catch any case where this is actually used - .context = nullptr, - .return_value = TaggedPointer(), - // data may be an object - // put it in the handle scope so that it has a map ptr - .target = data.tagged(), - .new_target = nullptr, - }; - - FunctionCallbackInfo info(&implicit_args, args.data() + 1, callFrame->argumentCount()); - - functionTemplate->__internals.m_callback(info); - - if (implicit_args.return_value.type() != TaggedPointer::Type::Smi && implicit_args.return_value.getPtr() == nullptr) { - // callback forgot to set a return value, so return undefined - return JSValue::encode(JSC::jsUndefined()); - } else { - Local local_ret(&implicit_args.return_value); - return JSValue::encode(local_ret->localToJSValue(isolate->globalInternals())); - } -} - -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.h b/src/bun.js/bindings/v8/V8FunctionTemplate.h index b1ba3399b0e0d..28a1688d476c9 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.h +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.h @@ -7,42 +7,17 @@ #include "V8MaybeLocal.h" #include "V8Value.h" #include "V8Signature.h" +#include "V8Template.h" +#include "V8FunctionCallbackInfo.h" +#include "shim/FunctionTemplate.h" namespace v8 { class Function; -struct ImplicitArgs { - // v8-function-callback.h:168 - void* holder; - Isolate* isolate; - Context* context; - // overwritten by the callback - TaggedPointer return_value; - // holds the value passed for data in FunctionTemplate::New - TaggedPointer target; - void* new_target; -}; - -// T = return value -template -class FunctionCallbackInfo { - // V8 treats this as an array of pointers - ImplicitArgs* implicit_args; - // index -1 is this - TaggedPointer* values; - int length; - -public: - FunctionCallbackInfo(ImplicitArgs* implicit_args_, TaggedPointer* values_, int length_) - : implicit_args(implicit_args_) - , values(values_) - , length(length_) - { - } -}; - -using FunctionCallback = void (*)(const FunctionCallbackInfo&); +namespace shim { +class Function; +} enum class ConstructorBehavior { kThrow, @@ -61,12 +36,8 @@ class CFunction { const void* type_info; }; -// If this inherited Template like it does in V8, the layout would be wrong for JSC HeapCell. -// Inheritance shouldn't matter for the ABI. -class FunctionTemplate : public JSC::InternalFunction { +class FunctionTemplate : public Template { public: - using Base = JSC::InternalFunction; - BUN_EXPORT static Local New( Isolate* isolate, FunctionCallback callback = nullptr, @@ -82,74 +53,16 @@ class FunctionTemplate : public JSC::InternalFunction { BUN_EXPORT MaybeLocal GetFunction(Local context); - static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); - - template - static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) - { - if constexpr (mode == JSC::SubspaceAccess::Concurrently) - return nullptr; - return WebCore::subspaceForImpl( - vm, - [](auto& spaces) { return spaces.m_clientSubspaceForFunctionTemplate.get(); }, - [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForFunctionTemplate = std::forward(space); }, - [](auto& spaces) { return spaces.m_subspaceForFunctionTemplate.get(); }, - [](auto& spaces, auto&& space) { spaces.m_subspaceForFunctionTemplate = std::forward(space); }); - } - - DECLARE_INFO; - DECLARE_VISIT_CHILDREN; - - friend class Function; - - FunctionCallback callback() const - { - return __internals.m_callback; - } - private: - class Internals { - private: - FunctionCallback m_callback; - JSC::WriteBarrier m_data; - - Internals(FunctionCallback callback, JSC::VM& vm, FunctionTemplate* owner, JSC::JSValue data) - : m_callback(callback) - , m_data(vm, owner, data) - { - } - - friend class FunctionTemplate; - }; - - // Only access this directly in functions called by JSC code, or on a valid v8::FunctionTemplate - // pointer. In functions called on a Local, use internals() - Internals __internals; - - FunctionTemplate* localToObjectPointer() + shim::FunctionTemplate* localToObjectPointer() { - return reinterpret_cast(this)->localToObjectPointer(); + return Data::localToObjectPointer(); } - const FunctionTemplate* localToObjectPointer() const - { - return reinterpret_cast(this)->localToObjectPointer(); - } - - // Only call this in functions called on a Local. When you have a valid - // v8::FunctionTemplate pointer, use __internals - Internals& internals() - { - return localToObjectPointer()->__internals; - } - - static JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES functionCall(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame); - - FunctionTemplate(JSC::VM& vm, JSC::Structure* structure, FunctionCallback callback, JSC::JSValue data) - : __internals(callback, vm, this, data) - , Base(vm, structure, functionCall, JSC::callHostFunctionAsConstructor) + const shim::FunctionTemplate* localToObjectPointer() const { + return Data::localToObjectPointer(); } }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index cac39fe2d5e07..c00bfcd84665b 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -1,13 +1,15 @@ #include "V8HandleScope.h" -#include "V8GlobalInternals.h" +#include "shim/GlobalInternals.h" namespace v8 { -HandleScope::HandleScope(Isolate* isolate_) - : m_isolate(isolate_) +HandleScope::HandleScope(Isolate* isolate) + : m_isolate(isolate) , m_previousHandleScope(m_isolate->globalInternals()->currentHandleScope()) - , m_buffer(HandleScopeBuffer::create(isolate_->vm(), isolate_->globalInternals()->handleScopeBufferStructure(isolate_->globalObject()))) + , m_buffer(shim::HandleScopeBuffer::create( + isolate->vm(), + isolate->globalInternals()->handleScopeBufferStructure(isolate->globalObject()))) { m_isolate->globalInternals()->setCurrentHandleScope(this); } @@ -28,4 +30,4 @@ uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t val return &newSlot->m_value; } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScope.h b/src/bun.js/bindings/v8/V8HandleScope.h index 00b5be645b805..dd351a50d6784 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.h +++ b/src/bun.js/bindings/v8/V8HandleScope.h @@ -3,8 +3,9 @@ #include "v8.h" #include "V8Isolate.h" #include "v8_internal.h" -#include "V8HandleScopeBuffer.h" -#include "V8GlobalInternals.h" +#include "shim/HandleScopeBuffer.h" +#include "shim/GlobalInternals.h" +#include "shim/Map.h" namespace v8 { @@ -19,9 +20,9 @@ class HandleScope { { // TODO(@190n) handle more types if (value.isString()) { - return Local(m_buffer->createHandle(value.asCell(), &Map::string_map, vm)); + return Local(m_buffer->createHandle(value.asCell(), &shim::Map::string_map, vm)); } else if (value.isCell()) { - return Local(m_buffer->createHandle(value.asCell(), &Map::object_map, vm)); + return Local(m_buffer->createHandle(value.asCell(), &shim::Map::object_map, vm)); } else if (value.isInt32()) { return Local(m_buffer->createSmiHandle(value.asInt32())); } else if (value.isNumber()) { @@ -46,7 +47,7 @@ class HandleScope { // must be 24 bytes to match V8 layout Isolate* m_isolate; HandleScope* m_previousHandleScope; - HandleScopeBuffer* m_buffer; + shim::HandleScopeBuffer* m_buffer; // is protected in v8, which matters on windows BUN_EXPORT static uintptr_t* CreateHandle(internal::Isolate* isolate, uintptr_t value); @@ -54,4 +55,4 @@ class HandleScope { static_assert(sizeof(HandleScope) == 24, "HandleScope has wrong layout"); -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Isolate.cpp b/src/bun.js/bindings/v8/V8Isolate.cpp index b5b548be232cf..03fd2c4eb4c20 100644 --- a/src/bun.js/bindings/v8/V8Isolate.cpp +++ b/src/bun.js/bindings/v8/V8Isolate.cpp @@ -1,6 +1,6 @@ #include "V8Isolate.h" #include "V8HandleScope.h" -#include "V8GlobalInternals.h" +#include "shim/GlobalInternals.h" namespace v8 { @@ -25,7 +25,7 @@ Local Isolate::GetCurrentContext() return currentHandleScope()->createLocal(m_globalObject->vm(), m_globalObject); } -Isolate::Isolate(GlobalInternals* globalInternals) +Isolate::Isolate(shim::GlobalInternals* globalInternals) : m_globalInternals(globalInternals) , m_globalObject(globalInternals->m_globalObject) { @@ -40,4 +40,4 @@ HandleScope* Isolate::currentHandleScope() return m_globalInternals->currentHandleScope(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Isolate.h b/src/bun.js/bindings/v8/V8Isolate.h index 929e76bf79e28..e1e091cf4333d 100644 --- a/src/bun.js/bindings/v8/V8Isolate.h +++ b/src/bun.js/bindings/v8/V8Isolate.h @@ -7,7 +7,10 @@ namespace v8 { class HandleScope; class Context; + +namespace shim { class GlobalInternals; +} // The only fields here are "roots," which are the global locations of V8's versions of nullish and // boolean values. These are computed as offsets from an Isolate pointer in many V8 functions so @@ -21,7 +24,7 @@ class Isolate final { static constexpr int kTrueValueRootIndex = 7; static constexpr int kFalseValueRootIndex = 8; - Isolate(GlobalInternals* globalInternals); + Isolate(shim::GlobalInternals* globalInternals); // Returns the isolate inside which the current thread is running or nullptr. BUN_EXPORT static Isolate* TryGetCurrent(); @@ -33,12 +36,12 @@ class Isolate final { Zig::GlobalObject* globalObject() { return m_globalObject; } JSC::VM& vm() { return globalObject()->vm(); } - GlobalInternals* globalInternals() { return m_globalInternals; } + shim::GlobalInternals* globalInternals() { return m_globalInternals; } HandleScope* currentHandleScope(); TaggedPointer* getRoot(int index) { return &m_roots[index]; } - GlobalInternals* m_globalInternals; + shim::GlobalInternals* m_globalInternals; Zig::GlobalObject* m_globalObject; uintptr_t m_padding[72]; @@ -48,4 +51,4 @@ class Isolate final { static_assert(offsetof(Isolate, m_roots) == 592, "Isolate has wrong layout"); -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Local.h b/src/bun.js/bindings/v8/V8Local.h index 1408d8e12bfa7..24bdfaafa1003 100644 --- a/src/bun.js/bindings/v8/V8Local.h +++ b/src/bun.js/bindings/v8/V8Local.h @@ -2,7 +2,7 @@ #include "root.h" -#include "V8TaggedPointer.h" +#include "shim/TaggedPointer.h" namespace v8 { @@ -39,4 +39,4 @@ class Local final { TaggedPointer* m_location; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Maybe.h b/src/bun.js/bindings/v8/V8Maybe.h index 2c0f52f5ec716..5e71a2502768d 100644 --- a/src/bun.js/bindings/v8/V8Maybe.h +++ b/src/bun.js/bindings/v8/V8Maybe.h @@ -30,4 +30,4 @@ inline Maybe Just(const T& t) return Maybe(t); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8MaybeLocal.h b/src/bun.js/bindings/v8/V8MaybeLocal.h index 82cdad09bc6b5..13b71acee2679 100644 --- a/src/bun.js/bindings/v8/V8MaybeLocal.h +++ b/src/bun.js/bindings/v8/V8MaybeLocal.h @@ -21,4 +21,4 @@ class MaybeLocal { Local m_local; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Number.cpp b/src/bun.js/bindings/v8/V8Number.cpp index 31c6744f2fc0a..dc9c9d8bc8572 100644 --- a/src/bun.js/bindings/v8/V8Number.cpp +++ b/src/bun.js/bindings/v8/V8Number.cpp @@ -13,4 +13,4 @@ double Number::Value() const return localToJSValue(Isolate::GetCurrent()->globalInternals()).asNumber(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Number.h b/src/bun.js/bindings/v8/V8Number.h index 7ec8e6d54cd51..e85c3ae5ec936 100644 --- a/src/bun.js/bindings/v8/V8Number.h +++ b/src/bun.js/bindings/v8/V8Number.h @@ -14,4 +14,4 @@ class Number : public Primitive { BUN_EXPORT double Value() const; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Object.cpp b/src/bun.js/bindings/v8/V8Object.cpp index ba9d9205eb52f..4a085a1e80297 100644 --- a/src/bun.js/bindings/v8/V8Object.cpp +++ b/src/bun.js/bindings/v8/V8Object.cpp @@ -1,5 +1,5 @@ #include "V8Object.h" -#include "V8InternalFieldObject.h" +#include "shim/InternalFieldObject.h" #include "V8HandleScope.h" #include "JavaScriptCore/ConstructData.h" @@ -14,7 +14,7 @@ using JSC::PutPropertySlot; namespace v8 { -using FieldContainer = InternalFieldObject::FieldContainer; +using FieldContainer = shim::InternalFieldObject::FieldContainer; static FieldContainer* getInternalFieldsContainer(Object* object) { @@ -22,7 +22,7 @@ static FieldContainer* getInternalFieldsContainer(Object* object) // TODO(@190n): do we need to unwrap proxies like node-jsc did? - if (auto ifo = JSC::jsDynamicCast(js_object)) { + if (auto ifo = JSC::jsDynamicCast(js_object)) { return ifo->internalFields(); } @@ -88,4 +88,4 @@ Local Object::SlowGetInternalField(int index) return handleScope->createLocal(globalObject->vm(), JSC::jsUndefined()); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Object.h b/src/bun.js/bindings/v8/V8Object.h index 6c74b0f035949..9351f44220bc3 100644 --- a/src/bun.js/bindings/v8/V8Object.h +++ b/src/bun.js/bindings/v8/V8Object.h @@ -22,4 +22,4 @@ class Object : public Value { BUN_EXPORT Local SlowGetInternalField(int index); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp index 46cd993268d24..ce784c22cc878 100644 --- a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp +++ b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp @@ -1,53 +1,19 @@ #include "V8ObjectTemplate.h" -#include "V8InternalFieldObject.h" -#include "V8GlobalInternals.h" +#include "shim/InternalFieldObject.h" +#include "shim/GlobalInternals.h" #include "V8HandleScope.h" -#include "JavaScriptCore/FunctionPrototype.h" -#include "JavaScriptCore/LazyPropertyInlines.h" -#include "JavaScriptCore/VMTrapsInlines.h" - -using JSC::JSGlobalObject; -using JSC::JSValue; -using JSC::LazyProperty; -using JSC::Structure; - namespace v8 { -void ObjectTemplate::finishCreation(JSC::VM& vm) -{ - Base::finishCreation(vm); - __internals.m_objectStructure.initLater([](const LazyProperty::Initializer& init) { - init.set(JSC::Structure::create( - init.vm, - init.owner->globalObject(), - init.owner->globalObject()->objectPrototype(), - JSC::TypeInfo(JSC::ObjectType, InternalFieldObject::StructureFlags), - InternalFieldObject::info())); - }); -} - -// for CREATE_METHOD_TABLE -namespace JSCastingHelpers = JSC::JSCastingHelpers; - -const JSC::ClassInfo ObjectTemplate::s_info = { - "ObjectTemplate"_s, - &Base::s_info, - nullptr, - nullptr, - CREATE_METHOD_TABLE(ObjectTemplate) -}; - Local ObjectTemplate::New(Isolate* isolate, Local constructor) { RELEASE_ASSERT(constructor.IsEmpty()); auto* globalObject = isolate->globalObject(); auto& vm = globalObject->vm(); auto* globalInternals = globalObject->V8GlobalInternals(); - Structure* structure = globalInternals->objectTemplateStructure(globalObject); - auto* objectTemplate = new (NotNull, JSC::allocateCell(vm)) ObjectTemplate(vm, structure); + auto* structure = globalInternals->objectTemplateStructure(globalObject); // TODO pass constructor - objectTemplate->finishCreation(vm); + auto* objectTemplate = shim::ObjectTemplate::create(vm, structure); return globalInternals->currentHandleScope()->createLocal(vm, objectTemplate); } @@ -57,52 +23,19 @@ MaybeLocal ObjectTemplate::NewInstance(Local context) // TODO handle interceptors? auto& vm = context->vm(); - auto thisObj = localToObjectPointer(); - - // get a structure - // must take thisObj because JSC needs the native pointer - auto structure = internals().m_objectStructure.get(thisObj); - - // create object from it - // InternalFieldObject needs a Local, which we can create using the `this` - // pointer as we know this method itself was called through a Local - auto newInstance = InternalFieldObject::create(vm, structure, Local(reinterpret_cast(this))); - - // todo: apply properties - + auto* thisObj = localToObjectPointer(); + auto* newInstance = thisObj->newInstance(); return MaybeLocal(context->currentHandleScope()->createLocal(vm, newInstance)); } -template -void ObjectTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) -{ - ObjectTemplate* fn = jsCast(cell); - ASSERT_GC_OBJECT_INHERITS(fn, info()); - Base::visitChildren(fn, visitor); - - fn->__internals.m_objectStructure.visit(visitor); -} - -DEFINE_VISIT_CHILDREN(ObjectTemplate); - void ObjectTemplate::SetInternalFieldCount(int value) { - internals().m_internalFieldCount = value; + localToObjectPointer()->setInternalFieldCount(value); } int ObjectTemplate::InternalFieldCount() const { - return internals().m_internalFieldCount; + return localToObjectPointer()->internalFieldCount(); } -Structure* ObjectTemplate::createStructure(JSC::VM& vm, JSGlobalObject* globalObject, JSValue prototype) -{ - return Structure::create( - vm, - globalObject, - prototype, - JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), - info()); -} - -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8ObjectTemplate.h b/src/bun.js/bindings/v8/V8ObjectTemplate.h index 4659c624a94fd..0775df72a9a64 100644 --- a/src/bun.js/bindings/v8/V8ObjectTemplate.h +++ b/src/bun.js/bindings/v8/V8ObjectTemplate.h @@ -1,6 +1,5 @@ #pragma once -#include "JavaScriptCore/SubspaceAccess.h" #include "v8.h" #include "V8Context.h" #include "V8Local.h" @@ -9,78 +8,27 @@ #include "V8MaybeLocal.h" #include "V8Object.h" #include "V8Template.h" +#include "shim/ObjectTemplate.h" namespace v8 { -// If this inherited Template like it does in V8, the layout would be wrong for JSC HeapCell. -// Inheritance shouldn't matter for the ABI. -class ObjectTemplate : public JSC::InternalFunction { +class ObjectTemplate : public Template { public: - using Base = JSC::InternalFunction; - - DECLARE_INFO; - BUN_EXPORT static Local New(Isolate* isolate, Local constructor = Local()); BUN_EXPORT MaybeLocal NewInstance(Local context); BUN_EXPORT void SetInternalFieldCount(int value); BUN_EXPORT int InternalFieldCount() const; - static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype); - - template - static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) - { - if constexpr (mode == JSC::SubspaceAccess::Concurrently) - return nullptr; - return WebCore::subspaceForImpl( - vm, - [](auto& spaces) { return spaces.m_clientSubspaceForObjectTemplate.get(); }, - [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForObjectTemplate = std::forward(space); }, - [](auto& spaces) { return spaces.m_subspaceForObjectTemplate.get(); }, - [](auto& spaces, auto&& space) { spaces.m_subspaceForObjectTemplate = std::forward(space); }); - } - - DECLARE_VISIT_CHILDREN; - private: - class Internals { - int m_internalFieldCount = 0; - JSC::LazyProperty m_objectStructure; - friend class ObjectTemplate; - }; - - // Only access this directly in functions called by JSC code, or on a valid v8::ObjectTemplate - // pointer. In functions called on a Local, use internals() - Internals __internals; - - ObjectTemplate* localToObjectPointer() + shim::ObjectTemplate* localToObjectPointer() { - return reinterpret_cast(this)->localToObjectPointer(); + return Data::localToObjectPointer(); } - const ObjectTemplate* localToObjectPointer() const + const shim::ObjectTemplate* localToObjectPointer() const { - return reinterpret_cast(this)->localToObjectPointer(); + return Data::localToObjectPointer(); } - - // Only call this in functions called on a Local. When you have a valid - // v8::ObjectTemplate pointer, use __internals - Internals& internals() - { - return localToObjectPointer()->__internals; - } - - const Internals& internals() const - { - return localToObjectPointer()->__internals; - } - - ObjectTemplate(JSC::VM& vm, JSC::Structure* structure) - : Base(vm, structure, Template::DummyCallback, Template::DummyCallback) - { - } - - void finishCreation(JSC::VM& vm); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Primitive.h b/src/bun.js/bindings/v8/V8Primitive.h index afb9565fe62c7..57791bac71a16 100644 --- a/src/bun.js/bindings/v8/V8Primitive.h +++ b/src/bun.js/bindings/v8/V8Primitive.h @@ -6,4 +6,4 @@ namespace v8 { class Primitive : public Value {}; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Signature.h b/src/bun.js/bindings/v8/V8Signature.h index 91387756535a1..61775b1b1102d 100644 --- a/src/bun.js/bindings/v8/V8Signature.h +++ b/src/bun.js/bindings/v8/V8Signature.h @@ -6,4 +6,4 @@ namespace v8 { class Signature : public Data {}; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8String.cpp b/src/bun.js/bindings/v8/V8String.cpp index 7e6d9965d91d1..a2e943939c688 100644 --- a/src/bun.js/bindings/v8/V8String.cpp +++ b/src/bun.js/bindings/v8/V8String.cpp @@ -164,4 +164,4 @@ int String::Length() const return static_cast(jsString->length()); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8String.h b/src/bun.js/bindings/v8/V8String.h index 4ec76762c7614..6c0ff9eed6419 100644 --- a/src/bun.js/bindings/v8/V8String.h +++ b/src/bun.js/bindings/v8/V8String.h @@ -76,4 +76,4 @@ class String : Primitive { } }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Template.cpp b/src/bun.js/bindings/v8/V8Template.cpp index 5993d74d41098..b5c95a1e284ef 100644 --- a/src/bun.js/bindings/v8/V8Template.cpp +++ b/src/bun.js/bindings/v8/V8Template.cpp @@ -8,4 +8,4 @@ JSC::EncodedJSValue Template::DummyCallback(JSC::JSGlobalObject* globalObject, J return JSC::JSValue::encode(JSC::jsUndefined()); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Template.h b/src/bun.js/bindings/v8/V8Template.h index d6cbdbc66f20c..d6aeb7d90f0e4 100644 --- a/src/bun.js/bindings/v8/V8Template.h +++ b/src/bun.js/bindings/v8/V8Template.h @@ -10,4 +10,4 @@ class Template : public Data { static JSC_HOST_CALL_ATTRIBUTES JSC::EncodedJSValue DummyCallback(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame); }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Value.cpp b/src/bun.js/bindings/v8/V8Value.cpp index f0dcdf0d7c946..afc5a2c18f25e 100644 --- a/src/bun.js/bindings/v8/V8Value.cpp +++ b/src/bun.js/bindings/v8/V8Value.cpp @@ -73,4 +73,4 @@ bool Value::FullIsFalse() const return localToJSValue(Isolate::GetCurrent()->globalInternals()).isFalse(); } -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Value.h b/src/bun.js/bindings/v8/V8Value.h index fe3b7c9f7e6ac..b658e4efaf6a1 100644 --- a/src/bun.js/bindings/v8/V8Value.h +++ b/src/bun.js/bindings/v8/V8Value.h @@ -29,4 +29,4 @@ class Value : public Data { BUN_EXPORT bool FullIsFalse() const; }; -} +} // namespace v8 diff --git a/src/bun.js/bindings/v8/node.cpp b/src/bun.js/bindings/v8/node.cpp index 4d148f286dd15..1df2feebc956d 100644 --- a/src/bun.js/bindings/v8/node.cpp +++ b/src/bun.js/bindings/v8/node.cpp @@ -100,4 +100,4 @@ void node_module_register(void* opaque_mod) globalObject->m_pendingNapiModuleAndExports[1].set(vm, globalObject, object); } -} +} // namespace node diff --git a/src/bun.js/bindings/v8/node.h b/src/bun.js/bindings/v8/node.h index d1b791f7ea3d0..dc467afbef324 100644 --- a/src/bun.js/bindings/v8/node.h +++ b/src/bun.js/bindings/v8/node.h @@ -41,4 +41,4 @@ struct node_module { extern "C" BUN_EXPORT void node_module_register(void* mod); -} +} // namespace node diff --git a/src/bun.js/bindings/v8/shim/Function.cpp b/src/bun.js/bindings/v8/shim/Function.cpp new file mode 100644 index 0000000000000..2ee99becbb002 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/Function.cpp @@ -0,0 +1,63 @@ +#include "Function.h" + +#include "JavaScriptCore/FunctionPrototype.h" + +using JSC::Structure; +using JSC::VM; + +namespace v8 { +namespace shim { + +// for CREATE_METHOD_TABLE +namespace JSCastingHelpers = JSC::JSCastingHelpers; + +const JSC::ClassInfo Function::s_info = { + "Function"_s, + &Base::s_info, + nullptr, + nullptr, + CREATE_METHOD_TABLE(Function) +}; + +Structure* Function::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) +{ + return Structure::create( + vm, + globalObject, + globalObject->functionPrototype(), + JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), + info()); +} + +template +void Function::visitChildrenImpl(JSCell* cell, Visitor& visitor) +{ + Function* fn = jsCast(cell); + ASSERT_GC_OBJECT_INHERITS(fn, info()); + Base::visitChildren(fn, visitor); + + visitor.append(fn->m_functionTemplate); +} + +DEFINE_VISIT_CHILDREN(Function); + +Function* Function::create(VM& vm, Structure* structure, FunctionTemplate* functionTemplate) +{ + auto* function = new (NotNull, JSC::allocateCell(vm)) Function(vm, structure); + function->finishCreation(vm, functionTemplate); + return function; +} + +void Function::finishCreation(VM& vm, FunctionTemplate* functionTemplate) +{ + Base::finishCreation(vm, 0, "Function"_s); + m_functionTemplate.set(vm, this, functionTemplate); +} + +void Function::setName(JSC::JSString* name) +{ + m_originalName.set(globalObject()->vm(), this, name); +} + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/Function.h b/src/bun.js/bindings/v8/shim/Function.h new file mode 100644 index 0000000000000..895dd2b2df7cb --- /dev/null +++ b/src/bun.js/bindings/v8/shim/Function.h @@ -0,0 +1,49 @@ +#pragma once + +#include "../v8.h" +#include "FunctionTemplate.h" + +namespace v8 { +namespace shim { + +class Function : public JSC::InternalFunction { +public: + using Base = JSC::InternalFunction; + + static Function* create(JSC::VM& vm, JSC::Structure* structure, FunctionTemplate* functionTemplate); + + DECLARE_INFO; + DECLARE_VISIT_CHILDREN; + + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + if constexpr (mode == JSC::SubspaceAccess::Concurrently) + return nullptr; + return WebCore::subspaceForImpl( + vm, + [](auto& spaces) { return spaces.m_clientSubspaceForV8Function.get(); }, + [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForV8Function = std::forward(space); }, + [](auto& spaces) { return spaces.m_subspaceForV8Function.get(); }, + [](auto& spaces, auto&& space) { spaces.m_subspaceForV8Function = std::forward(space); }); + } + + FunctionTemplate* functionTemplate() const { return m_functionTemplate.get(); } + + void setName(JSC::JSString* name); + +private: + JSC::WriteBarrier m_functionTemplate; + + Function(JSC::VM& vm, JSC::Structure* structure) + : Base(vm, structure, FunctionTemplate::functionCall) + { + } + + void finishCreation(JSC::VM& vm, FunctionTemplate* functionTemplate); +}; + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp new file mode 100644 index 0000000000000..8b02503264722 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp @@ -0,0 +1,101 @@ +#include "FunctionTemplate.h" +#include "Function.h" +#include "../V8HandleScope.h" +#include "../V8Data.h" + +#include "JavaScriptCore/FunctionPrototype.h" + +using JSC::JSValue; +using JSC::Structure; + +namespace v8 { +namespace shim { + +// for CREATE_METHOD_TABLE +namespace JSCastingHelpers = JSC::JSCastingHelpers; + +const JSC::ClassInfo FunctionTemplate::s_info = { + "FunctionTemplate"_s, + &Base::s_info, + nullptr, + nullptr, + CREATE_METHOD_TABLE(FunctionTemplate) +}; + +FunctionTemplate* FunctionTemplate::create(JSC::VM& vm, JSC::Structure* structure, FunctionCallback callback, JSC::JSValue data) +{ + auto* functionTemplate = new (NotNull, JSC::allocateCell(vm)) FunctionTemplate( + vm, structure, callback, data); + functionTemplate->finishCreation(vm); + return functionTemplate; +} + +Structure* FunctionTemplate::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) +{ + return Structure::create( + vm, + globalObject, + globalObject->functionPrototype(), + JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), + info()); +} + +template +void FunctionTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) +{ + FunctionTemplate* fn = jsCast(cell); + ASSERT_GC_OBJECT_INHERITS(fn, info()); + Base::visitChildren(fn, visitor); + + visitor.append(fn->m_data); +} + +DEFINE_VISIT_CHILDREN(FunctionTemplate); + +JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame) +{ + auto* callee = JSC::jsDynamicCast(callFrame->jsCallee()); + auto* functionTemplate = callee->functionTemplate(); + auto* isolate = JSC::jsCast(globalObject)->V8GlobalInternals()->isolate(); + auto& vm = globalObject->vm(); + + WTF::Vector args(callFrame->argumentCount() + 1); + + HandleScope hs(isolate); + Local thisValue = hs.createLocal(vm, callFrame->thisValue()); + args[0] = thisValue.tagged(); + + for (size_t i = 0; i < callFrame->argumentCount(); i++) { + Local argValue = hs.createLocal(vm, callFrame->argument(i)); + args[i + 1] = argValue.tagged(); + } + + Local data = hs.createLocal(vm, functionTemplate->m_data.get()); + + ImplicitArgs implicit_args = { + .holder = nullptr, + .isolate = isolate, + // set to nullptr to catch any case where this is actually used + .context = nullptr, + .return_value = TaggedPointer(), + // data may be an object + // put it in the handle scope so that it has a map ptr + .target = data.tagged(), + .new_target = nullptr, + }; + + FunctionCallbackInfo info(&implicit_args, args.data() + 1, callFrame->argumentCount()); + + functionTemplate->m_callback(info); + + if (implicit_args.return_value.type() != TaggedPointer::Type::Smi && implicit_args.return_value.getPtr() == nullptr) { + // callback forgot to set a return value, so return undefined + return JSValue::encode(JSC::jsUndefined()); + } else { + Local local_ret(&implicit_args.return_value); + return JSValue::encode(local_ret->localToJSValue(isolate->globalInternals())); + } +} + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.h b/src/bun.js/bindings/v8/shim/FunctionTemplate.h new file mode 100644 index 0000000000000..29027bd3fc0f9 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.h @@ -0,0 +1,51 @@ +#pragma once + +#include "../v8.h" +#include "../V8FunctionCallbackInfo.h" + +namespace v8 { + +class FunctionTemplate; + +namespace shim { + +class FunctionTemplate : public JSC::InternalFunction { +public: + using Base = JSC::InternalFunction; + + static FunctionTemplate* create(JSC::VM& vm, JSC::Structure* structure, FunctionCallback callback, JSC::JSValue data); + + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + if constexpr (mode == JSC::SubspaceAccess::Concurrently) + return nullptr; + return WebCore::subspaceForImpl( + vm, + [](auto& spaces) { return spaces.m_clientSubspaceForFunctionTemplate.get(); }, + [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForFunctionTemplate = std::forward(space); }, + [](auto& spaces) { return spaces.m_subspaceForFunctionTemplate.get(); }, + [](auto& spaces, auto&& space) { spaces.m_subspaceForFunctionTemplate = std::forward(space); }); + } + + DECLARE_INFO; + DECLARE_VISIT_CHILDREN; + + static JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES functionCall(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame); + +private: + FunctionCallback m_callback; + JSC::WriteBarrier m_data; + + FunctionTemplate(JSC::VM& vm, JSC::Structure* structure, FunctionCallback callback, JSC::JSValue data) + : Base(vm, structure, functionCall, JSC::callHostFunctionAsConstructor) + , m_callback(callback) + , m_data(vm, this, data) + { + } +}; + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8GlobalInternals.cpp b/src/bun.js/bindings/v8/shim/GlobalInternals.cpp similarity index 91% rename from src/bun.js/bindings/v8/V8GlobalInternals.cpp rename to src/bun.js/bindings/v8/shim/GlobalInternals.cpp index 1e71ccee4aa2f..452c90b21dcaf 100644 --- a/src/bun.js/bindings/v8/V8GlobalInternals.cpp +++ b/src/bun.js/bindings/v8/shim/GlobalInternals.cpp @@ -1,10 +1,10 @@ -#include "V8GlobalInternals.h" +#include "GlobalInternals.h" -#include "V8ObjectTemplate.h" -#include "V8InternalFieldObject.h" -#include "V8HandleScopeBuffer.h" -#include "V8FunctionTemplate.h" -#include "V8Function.h" +#include "../V8ObjectTemplate.h" +#include "InternalFieldObject.h" +#include "HandleScopeBuffer.h" +#include "../V8FunctionTemplate.h" +#include "../V8Function.h" #include "JavaScriptCore/FunctionPrototype.h" #include "JavaScriptCore/LazyClassStructureInlines.h" @@ -17,6 +17,7 @@ using JSC::Structure; using JSC::VM; namespace v8 { +namespace shim { // for CREATE_METHOD_TABLE namespace JSCastingHelpers = JSC::JSCastingHelpers; @@ -67,4 +68,5 @@ void GlobalInternals::visitChildrenImpl(JSCell* cell, Visitor& visitor) DEFINE_VISIT_CHILDREN_WITH_MODIFIER(JS_EXPORT_PRIVATE, GlobalInternals); -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8GlobalInternals.h b/src/bun.js/bindings/v8/shim/GlobalInternals.h similarity index 95% rename from src/bun.js/bindings/v8/V8GlobalInternals.h rename to src/bun.js/bindings/v8/shim/GlobalInternals.h index 734a2e76b7f1b..872063c9a2a2e 100644 --- a/src/bun.js/bindings/v8/V8GlobalInternals.h +++ b/src/bun.js/bindings/v8/shim/GlobalInternals.h @@ -2,12 +2,15 @@ #include "BunClientData.h" -#include "V8Isolate.h" -#include "V8Oddball.h" +#include "../V8Isolate.h" +#include "Oddball.h" namespace v8 { class HandleScope; + +namespace shim { + class HandleScopeBuffer; class GlobalInternals : public JSC::JSCell { @@ -73,9 +76,8 @@ class GlobalInternals : public JSC::JSCell { DECLARE_INFO; DECLARE_VISIT_CHILDREN_WITH_MODIFIER(JS_EXPORT_PRIVATE); - friend struct Roots; - friend class Isolate; - friend class Context; + friend class ::v8::Isolate; + friend class ::v8::Context; private: Zig::GlobalObject* m_globalObject; @@ -107,4 +109,5 @@ class GlobalInternals : public JSC::JSCell { } }; -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Handle.h b/src/bun.js/bindings/v8/shim/Handle.h similarity index 97% rename from src/bun.js/bindings/v8/V8Handle.h rename to src/bun.js/bindings/v8/shim/Handle.h index 6b19617485d80..ceed92ccfe43c 100644 --- a/src/bun.js/bindings/v8/V8Handle.h +++ b/src/bun.js/bindings/v8/shim/Handle.h @@ -1,8 +1,10 @@ #pragma once -#include "V8Map.h" +#include "Map.h" +#include namespace v8 { +namespace shim { class ObjectLayout { private: @@ -131,4 +133,5 @@ struct Handle { ObjectLayout m_object; }; -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp similarity index 95% rename from src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp rename to src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp index 8cee152cad401..50b3dd4f7ee1c 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp @@ -1,8 +1,9 @@ -#include "V8HandleScopeBuffer.h" -#include "V8GlobalInternals.h" -#include "V8Isolate.h" +#include "HandleScopeBuffer.h" +#include "GlobalInternals.h" +#include "../V8Isolate.h" namespace v8 { +namespace shim { // for CREATE_METHOD_TABLE namespace JSCastingHelpers = JSC::JSCastingHelpers; @@ -108,4 +109,5 @@ void HandleScopeBuffer::clear() m_storage.clear(); } -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.h similarity index 90% rename from src/bun.js/bindings/v8/V8HandleScopeBuffer.h rename to src/bun.js/bindings/v8/shim/HandleScopeBuffer.h index ffc86c18e8a90..e51e59e9e8072 100644 --- a/src/bun.js/bindings/v8/V8HandleScopeBuffer.h +++ b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.h @@ -1,13 +1,17 @@ #pragma once -#include "v8.h" -#include "V8TaggedPointer.h" -#include "V8Map.h" -#include "V8Handle.h" +#include "../v8.h" +#include "TaggedPointer.h" +#include "Map.h" +#include "Handle.h" +#include namespace v8 { class Isolate; +class EscapableHandleScopeBase; + +namespace shim { // An array used by HandleScope to store the items. Must keep pointer stability when resized, since // v8::Locals point inside this array. @@ -53,7 +57,7 @@ class HandleScopeBuffer : public JSC::JSCell { DECLARE_INFO; DECLARE_VISIT_CHILDREN; - friend class EscapableHandleScopeBase; + friend class ::v8::EscapableHandleScopeBase; private: WTF::Lock m_gcLock; @@ -67,4 +71,5 @@ class HandleScopeBuffer : public JSC::JSCell { } }; -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8InternalFieldObject.cpp b/src/bun.js/bindings/v8/shim/InternalFieldObject.cpp similarity index 80% rename from src/bun.js/bindings/v8/V8InternalFieldObject.cpp rename to src/bun.js/bindings/v8/shim/InternalFieldObject.cpp index 09bbdc4d720e9..ca3f7bd2ee214 100644 --- a/src/bun.js/bindings/v8/V8InternalFieldObject.cpp +++ b/src/bun.js/bindings/v8/shim/InternalFieldObject.cpp @@ -1,7 +1,8 @@ -#include "V8InternalFieldObject.h" +#include "InternalFieldObject.h" +#include "ObjectTemplate.h" namespace v8 { - +namespace shim { // for CREATE_METHOD_TABLE namespace JSCastingHelpers = JSC::JSCastingHelpers; @@ -13,11 +14,11 @@ const JSC::ClassInfo InternalFieldObject::s_info = { CREATE_METHOD_TABLE(InternalFieldObject) }; -InternalFieldObject* InternalFieldObject::create(JSC::VM& vm, JSC::Structure* structure, Local objectTemplate) +InternalFieldObject* InternalFieldObject::create(JSC::VM& vm, JSC::Structure* structure, int internalFieldCount) { // TODO figure out how this works with __internals // maybe pass a Local - auto object = new (NotNull, JSC::allocateCell(vm)) InternalFieldObject(vm, structure, objectTemplate->InternalFieldCount()); + auto object = new (NotNull, JSC::allocateCell(vm)) InternalFieldObject(vm, structure, internalFieldCount); object->finishCreation(vm); return object; } @@ -36,4 +37,5 @@ void InternalFieldObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) DEFINE_VISIT_CHILDREN(InternalFieldObject); -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8InternalFieldObject.h b/src/bun.js/bindings/v8/shim/InternalFieldObject.h similarity index 91% rename from src/bun.js/bindings/v8/V8InternalFieldObject.h rename to src/bun.js/bindings/v8/shim/InternalFieldObject.h index 2d483d1a4e478..1453c9448ef01 100644 --- a/src/bun.js/bindings/v8/V8InternalFieldObject.h +++ b/src/bun.js/bindings/v8/shim/InternalFieldObject.h @@ -1,9 +1,13 @@ #pragma once -#include "V8ObjectTemplate.h" +#include "BunClientData.h" namespace v8 { +namespace shim { + +class ObjectTemplate; + class InternalFieldObject : public JSC::JSDestructibleObject { public: using Base = JSC::JSDestructibleObject; @@ -27,7 +31,7 @@ class InternalFieldObject : public JSC::JSDestructibleObject { using FieldContainer = WTF::FixedVector>; FieldContainer* internalFields() { return &m_fields; } - static InternalFieldObject* create(JSC::VM& vm, JSC::Structure* structure, Local objectTemplate); + static InternalFieldObject* create(JSC::VM& vm, JSC::Structure* structure, int internalFieldCount); DECLARE_VISIT_CHILDREN; @@ -42,4 +46,5 @@ class InternalFieldObject : public JSC::JSDestructibleObject { FieldContainer m_fields; }; -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Map.cpp b/src/bun.js/bindings/v8/shim/Map.cpp similarity index 81% rename from src/bun.js/bindings/v8/V8Map.cpp rename to src/bun.js/bindings/v8/shim/Map.cpp index feeacf8b66c17..53d1f363e01fb 100644 --- a/src/bun.js/bindings/v8/V8Map.cpp +++ b/src/bun.js/bindings/v8/shim/Map.cpp @@ -1,6 +1,7 @@ -#include "V8Map.h" +#include "Map.h" namespace v8 { +namespace shim { // TODO give these more appropriate instance types const Map Map::map_map(InstanceType::Object); @@ -9,4 +10,5 @@ const Map Map::oddball_map(InstanceType::Oddball); const Map Map::string_map(InstanceType::String); const Map Map::heap_number_map(InstanceType::HeapNumber); -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Map.h b/src/bun.js/bindings/v8/shim/Map.h similarity index 96% rename from src/bun.js/bindings/v8/V8Map.h rename to src/bun.js/bindings/v8/shim/Map.h index 4bd5b12a657bb..95a9d14f6daa4 100644 --- a/src/bun.js/bindings/v8/V8Map.h +++ b/src/bun.js/bindings/v8/shim/Map.h @@ -1,8 +1,9 @@ #pragma once -#include "V8TaggedPointer.h" +#include "TaggedPointer.h" namespace v8 { +namespace shim { enum class InstanceType : uint16_t { // v8-internal.h:787, kFirstNonstringType is 0x80 @@ -58,4 +59,5 @@ static_assert(sizeof(Map) == 16, "Map has wrong layout"); static_assert(offsetof(Map, m_metaMap) == 0, "Map has wrong layout"); static_assert(offsetof(Map, m_instanceType) == 12, "Map has wrong layout"); -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/ObjectTemplate.cpp b/src/bun.js/bindings/v8/shim/ObjectTemplate.cpp new file mode 100644 index 0000000000000..6fb50513b8542 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/ObjectTemplate.cpp @@ -0,0 +1,76 @@ +#include "ObjectTemplate.h" + +#include "JavaScriptCore/FunctionPrototype.h" +#include "JavaScriptCore/LazyPropertyInlines.h" +#include "JavaScriptCore/VMTrapsInlines.h" + +using JSC::LazyProperty; +using JSC::Structure; + +namespace v8 { +namespace shim { + +// for CREATE_METHOD_TABLE +namespace JSCastingHelpers = JSC::JSCastingHelpers; + +const JSC::ClassInfo ObjectTemplate::s_info = { + "ObjectTemplate"_s, + &Base::s_info, + nullptr, + nullptr, + CREATE_METHOD_TABLE(ObjectTemplate) +}; + +ObjectTemplate* ObjectTemplate::create(JSC::VM& vm, JSC::Structure* structure) +{ + // TODO take a constructor + auto* objectTemplate = new (NotNull, JSC::allocateCell(vm)) ObjectTemplate(vm, structure); + objectTemplate->finishCreation(vm); + return objectTemplate; +} + +void ObjectTemplate::finishCreation(JSC::VM& vm) +{ + Base::finishCreation(vm); + m_objectStructure.initLater([](const LazyProperty::Initializer& init) { + init.set(JSC::Structure::create( + init.vm, + init.owner->globalObject(), + init.owner->globalObject()->objectPrototype(), + JSC::TypeInfo(JSC::ObjectType, InternalFieldObject::StructureFlags), + InternalFieldObject::info())); + }); +} + +template +void ObjectTemplate::visitChildrenImpl(JSCell* cell, Visitor& visitor) +{ + ObjectTemplate* tmp = jsCast(cell); + ASSERT_GC_OBJECT_INHERITS(tmp, info()); + Base::visitChildren(tmp, visitor); + + tmp->m_objectStructure.visit(visitor); +} + +DEFINE_VISIT_CHILDREN(ObjectTemplate); + +Structure* ObjectTemplate::createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype) +{ + return Structure::create( + vm, + globalObject, + prototype, + JSC::TypeInfo(JSC::InternalFunctionType, StructureFlags), + info()); +} + +InternalFieldObject* ObjectTemplate::newInstance() +{ + auto* structure = m_objectStructure.get(this); + auto* newInstance = InternalFieldObject::create(globalObject()->vm(), structure, m_internalFieldCount); + // todo: apply properties + return newInstance; +} + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/ObjectTemplate.h b/src/bun.js/bindings/v8/shim/ObjectTemplate.h new file mode 100644 index 0000000000000..85be1790e8f85 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/ObjectTemplate.h @@ -0,0 +1,61 @@ +#pragma once + +#include "JavaScriptCore/SubspaceAccess.h" +#include "../v8/V8Template.h" +#include "InternalFieldObject.h" + +namespace v8 { + +class ObjectTemplate; + +namespace shim { + +class ObjectTemplate : public JSC::InternalFunction { +public: + using Base = JSC::InternalFunction; + + DECLARE_INFO; + + static ObjectTemplate* create(JSC::VM& vm, JSC::Structure* structure); + + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype); + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + if constexpr (mode == JSC::SubspaceAccess::Concurrently) + return nullptr; + return WebCore::subspaceForImpl( + vm, + [](auto& spaces) { return spaces.m_clientSubspaceForObjectTemplate.get(); }, + [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForObjectTemplate = std::forward(space); }, + [](auto& spaces) { return spaces.m_subspaceForObjectTemplate.get(); }, + [](auto& spaces, auto&& space) { spaces.m_subspaceForObjectTemplate = std::forward(space); }); + } + + DECLARE_VISIT_CHILDREN; + + InternalFieldObject* newInstance(); + + int internalFieldCount() const { return m_internalFieldCount; } + + void setInternalFieldCount(int newInternalFieldCount) { m_internalFieldCount = newInternalFieldCount; } + +private: + // Number of internal fields to allocate space for on objects created by this template + int m_internalFieldCount = 0; + // Structure used to allocate objects with this template (different than + // GlobalInternals::m_objectTemplateStructure, which is the structure used to allocate object + // templates themselves) + JSC::LazyProperty m_objectStructure; + + ObjectTemplate(JSC::VM& vm, JSC::Structure* structure) + : Base(vm, structure, Template::DummyCallback, Template::DummyCallback) + { + } + + void finishCreation(JSC::VM& vm); +}; + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Oddball.h b/src/bun.js/bindings/v8/shim/Oddball.h similarity index 80% rename from src/bun.js/bindings/v8/V8Oddball.h rename to src/bun.js/bindings/v8/shim/Oddball.h index 2ca1d869d5678..b8b3e9ed1f854 100644 --- a/src/bun.js/bindings/v8/V8Oddball.h +++ b/src/bun.js/bindings/v8/shim/Oddball.h @@ -1,9 +1,10 @@ #pragma once -#include "V8TaggedPointer.h" -#include "V8Map.h" +#include "TaggedPointer.h" +#include "Map.h" namespace v8 { +namespace shim { struct Oddball { enum class Kind : int { @@ -25,4 +26,5 @@ struct Oddball { } }; -} +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/V8TaggedPointer.h b/src/bun.js/bindings/v8/shim/TaggedPointer.h similarity index 91% rename from src/bun.js/bindings/v8/V8TaggedPointer.h rename to src/bun.js/bindings/v8/shim/TaggedPointer.h index 2ad827f25f2da..2f4a9156a6995 100644 --- a/src/bun.js/bindings/v8/V8TaggedPointer.h +++ b/src/bun.js/bindings/v8/shim/TaggedPointer.h @@ -1,8 +1,14 @@ #pragma once -#include "v8.h" +#include "wtf/Assertions.h" +#include + +namespace JSC { +class JSCell; +} namespace v8 { +namespace shim { struct TaggedPointer { uintptr_t m_value; @@ -75,4 +81,8 @@ struct TaggedPointer { } }; -} +} // namespace shim + +using shim::TaggedPointer; + +} // namespace v8 diff --git a/src/bun.js/bindings/v8/v8.h b/src/bun.js/bindings/v8/v8.h index 461c653f5173f..6bbe46fbef5ac 100644 --- a/src/bun.js/bindings/v8/v8.h +++ b/src/bun.js/bindings/v8/v8.h @@ -12,5 +12,11 @@ extern "C" Zig::GlobalObject* Bun__getDefaultGlobalObject(); +// Use only for types and functions that are exposed in the public V8 API namespace v8 { + +// Use for types added to Bun to support V8 APIs that aren't used in the actual V8 API +namespace shim { } + +} // namespace v8 diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index 75a4abd6b2053..a4ffbda7afd62 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -1,9 +1,10 @@ #include "v8_api_internal.h" #include "V8Isolate.h" -#include "V8HandleScopeBuffer.h" -#include "V8GlobalInternals.h" +#include "shim/HandleScopeBuffer.h" +#include "shim/GlobalInternals.h" namespace v8 { + namespace api_internal { void ToLocalEmpty() @@ -25,5 +26,5 @@ void DisposeGlobal(uintptr_t* location) (void)location; } -} -} +} // namespace api_internal +} // namespace v8 diff --git a/src/bun.js/bindings/v8/v8_api_internal.h b/src/bun.js/bindings/v8/v8_api_internal.h index ec3fa7347b5f7..8d84e0ab3db51 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.h +++ b/src/bun.js/bindings/v8/v8_api_internal.h @@ -10,5 +10,5 @@ BUN_EXPORT void ToLocalEmpty(); BUN_EXPORT uintptr_t* GlobalizeReference(v8::internal::Isolate* isolate, uintptr_t address); BUN_EXPORT void DisposeGlobal(uintptr_t* location); -} -} +} // namespace api_internal +} // namespace v8 diff --git a/src/bun.js/bindings/v8/v8_internal.cpp b/src/bun.js/bindings/v8/v8_internal.cpp index 4f4ce03da4565..784433c844251 100644 --- a/src/bun.js/bindings/v8/v8_internal.cpp +++ b/src/bun.js/bindings/v8/v8_internal.cpp @@ -9,5 +9,5 @@ Isolate* IsolateFromNeverReadOnlySpaceObject(uintptr_t obj) return nullptr; } -} -} +} // namespace internal +} // namespace v8 diff --git a/src/bun.js/bindings/v8/v8_internal.h b/src/bun.js/bindings/v8/v8_internal.h index 98c511f5804fc..472ae4c17fbac 100644 --- a/src/bun.js/bindings/v8/v8_internal.h +++ b/src/bun.js/bindings/v8/v8_internal.h @@ -10,5 +10,5 @@ class Isolate {}; BUN_EXPORT Isolate* IsolateFromNeverReadOnlySpaceObject(uintptr_t obj); -} -} +} // namespace internal +} // namespace v8 From 64b3aeac84c4d30b0be66673b917c707b58b313f Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 16:47:14 -0700 Subject: [PATCH 11/30] Remove duplicate destructor call in EscapableHandleScope --- src/bun.js/bindings/v8/V8EscapableHandleScope.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp index 6208c077c1f82..091db0724b25f 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp @@ -9,7 +9,6 @@ EscapableHandleScope::EscapableHandleScope(Isolate* isolate) EscapableHandleScope::~EscapableHandleScope() { - EscapableHandleScopeBase::~EscapableHandleScopeBase(); } } // namespace v8 From 5177532d959f74e1a51809f5ba49b1ad2050213a Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 18:14:26 -0700 Subject: [PATCH 12/30] Delete v8::Data::localToPointer --- src/bun.js/bindings/v8/V8Data.h | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index ea12cfa986c00..f4c6bf33bca10 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -14,18 +14,12 @@ class Data { // 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(); - } - // Recover a JSCell pointer out of a v8::Local JSC::JSCell* localToCell() { - return reinterpret_cast(localToPointer()); + TaggedPointer tagged = localToTagged(); + RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi); + return tagged.getPtr(); } // Recover a pointer to a JSCell subclass out of a v8::Local @@ -65,18 +59,12 @@ class Data { } } - // 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(); - } - // Recover a JSCell pointer out of a v8::Local const JSC::JSCell* localToCell() const { - return reinterpret_cast(localToPointer()); + TaggedPointer tagged = localToTagged(); + RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi); + return tagged.getPtr(); } // Recover a pointer to a JSCell subclass out of a v8::Local From e4e365c9314432423b09d3501d7c298b6f274e3d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 3 Sep 2024 18:40:19 -0700 Subject: [PATCH 13/30] Clean up many v8::Data functions --- src/bun.js/bindings/v8/V8Boolean.cpp | 9 ++- src/bun.js/bindings/v8/V8Data.h | 63 +++++++++---------- src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 2 +- src/bun.js/bindings/v8/V8Number.cpp | 2 +- src/bun.js/bindings/v8/V8Object.cpp | 6 +- src/bun.js/bindings/v8/V8Value.cpp | 22 +++---- .../bindings/v8/shim/FunctionTemplate.cpp | 2 +- src/bun.js/bindings/v8/shim/Oddball.h | 22 +++++++ 8 files changed, 75 insertions(+), 53 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Boolean.cpp b/src/bun.js/bindings/v8/V8Boolean.cpp index 05374dbc980df..42ad98b82b076 100644 --- a/src/bun.js/bindings/v8/V8Boolean.cpp +++ b/src/bun.js/bindings/v8/V8Boolean.cpp @@ -5,7 +5,14 @@ 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::New(Isolate* isolate, bool value) diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index f4c6bf33bca10..9295db2093701 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -17,9 +17,9 @@ class Data { // Recover a JSCell pointer out of a v8::Local JSC::JSCell* localToCell() { - TaggedPointer tagged = localToTagged(); - RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi); - return tagged.getPtr(); + TaggedPointer root = localToTagged(); + RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi); + return root.getPtr()->asCell(); } // Recover a pointer to a JSCell subclass out of a v8::Local @@ -30,31 +30,33 @@ class Data { return JSC::jsDynamicCast(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(); + RELEASE_ASSERT( + oddball->m_map.getPtr()->m_instanceType == shim::InstanceType::Oddball); + return oddball->toJSValue(); + } + // Get this as a JSValue when this is a v8::Local - JSC::JSValue localToJSValue(shim::GlobalInternals* globalInternals) const + JSC::JSValue localToJSValue() const { - TaggedPointer root = *reinterpret_cast(this); + TaggedPointer root = localToTagged(); if (root.type() == TaggedPointer::Type::Smi) { return JSC::jsNumber(root.getSmiUnchecked()); } else { - void* raw_ptr = root.getPtr(); - // check if this pointer is identical to the fixed locations where these primitive - // values are stored - if (raw_ptr == globalInternals->undefinedSlot()->getPtr()) { - return JSC::jsUndefined(); - } else if (raw_ptr == globalInternals->nullSlot()->getPtr()) { - return JSC::jsNull(); - } else if (raw_ptr == globalInternals->trueSlot()->getPtr()) { - return JSC::jsBoolean(true); - } else if (raw_ptr == globalInternals->falseSlot()->getPtr()) { - return JSC::jsBoolean(false); - } + using shim::InstanceType; + auto* v8_object = root.getPtr(); - shim::ObjectLayout* v8_object = reinterpret_cast(raw_ptr); - if (v8_object->map()->m_instanceType == shim::InstanceType::HeapNumber) { + switch (v8_object->map()->m_instanceType) { + case InstanceType::Oddball: + return reinterpret_cast(v8_object)->toJSValue(); + case InstanceType::HeapNumber: return JSC::jsDoubleNumber(v8_object->asDouble()); - } else { - return JSC::JSValue(v8_object->asCell()); + default: + return v8_object->asCell(); } } } @@ -62,9 +64,9 @@ class Data { // Recover a JSCell pointer out of a v8::Local const JSC::JSCell* localToCell() const { - TaggedPointer tagged = localToTagged(); - RELEASE_ASSERT(tagged.type() != TaggedPointer::Type::Smi); - return tagged.getPtr(); + TaggedPointer root = localToTagged(); + RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi); + return root.getPtr()->asCell(); } // Recover a pointer to a JSCell subclass out of a v8::Local @@ -76,19 +78,10 @@ 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(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. - shim::ObjectLayout* v8_object = root.getPtr(); - return TaggedPointer(v8_object->asCell()); - } + return *reinterpret_cast(this); } }; diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index 1cbe29ef61615..d8743ed867984 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -44,7 +44,7 @@ Local FunctionTemplate::New( auto globalObject = isolate->globalObject(); auto& vm = globalObject->vm(); auto* globalInternals = globalObject->V8GlobalInternals(); - JSValue jsc_data = data.IsEmpty() ? JSC::jsUndefined() : data->localToJSValue(globalInternals); + JSValue jsc_data = data.IsEmpty() ? JSC::jsUndefined() : data->localToJSValue(); Structure* structure = globalInternals->functionTemplateStructure(globalObject); auto* functionTemplate = shim::FunctionTemplate::create(vm, structure, callback, jsc_data); diff --git a/src/bun.js/bindings/v8/V8Number.cpp b/src/bun.js/bindings/v8/V8Number.cpp index dc9c9d8bc8572..4e20336d83d70 100644 --- a/src/bun.js/bindings/v8/V8Number.cpp +++ b/src/bun.js/bindings/v8/V8Number.cpp @@ -10,7 +10,7 @@ Local Number::New(Isolate* isolate, double value) double Number::Value() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).asNumber(); + return localToJSValue().asNumber(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Object.cpp b/src/bun.js/bindings/v8/V8Object.cpp index 4a085a1e80297..07dd1c3f9c50c 100644 --- a/src/bun.js/bindings/v8/V8Object.cpp +++ b/src/bun.js/bindings/v8/V8Object.cpp @@ -39,8 +39,8 @@ Maybe Object::Set(Local context, Local key, Local v { Zig::GlobalObject* globalObject = context->globalObject(); JSObject* object = localToObjectPointer(); - JSValue k = key->localToJSValue(globalObject->V8GlobalInternals()); - JSValue v = value->localToJSValue(globalObject->V8GlobalInternals()); + JSValue k = key->localToJSValue(); + JSValue v = value->localToJSValue(); auto& vm = globalObject->vm(); auto scope = DECLARE_CATCH_SCOPE(vm); @@ -67,7 +67,7 @@ void Object::SetInternalField(int index, Local data) RELEASE_ASSERT(index >= 0 && index < fields->size(), "internal field index is out of bounds"); JSObject* js_object = localToObjectPointer(); auto* globalObject = JSC::jsDynamicCast(js_object->globalObject()); - fields->at(index).set(globalObject->vm(), localToCell(), data->localToJSValue(globalObject->V8GlobalInternals())); + fields->at(index).set(globalObject->vm(), localToCell(), data->localToJSValue()); } Local Object::GetInternalField(int index) diff --git a/src/bun.js/bindings/v8/V8Value.cpp b/src/bun.js/bindings/v8/V8Value.cpp index afc5a2c18f25e..e1acb50a9db90 100644 --- a/src/bun.js/bindings/v8/V8Value.cpp +++ b/src/bun.js/bindings/v8/V8Value.cpp @@ -5,37 +5,37 @@ namespace v8 { bool Value::IsBoolean() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isBoolean(); + return localToJSValue().isBoolean(); } bool Value::IsObject() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isObject(); + return localToJSValue().isObject(); } bool Value::IsNumber() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isNumber(); + return localToJSValue().isNumber(); } bool Value::IsUint32() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isUInt32(); + return localToJSValue().isUInt32(); } bool Value::IsUndefined() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isUndefined(); + return localToJSValue().isUndefined(); } bool Value::IsNull() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isNull(); + return localToJSValue().isNull(); } bool Value::IsNullOrUndefined() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isUndefinedOrNull(); + return localToJSValue().isUndefinedOrNull(); } bool Value::IsTrue() const @@ -50,12 +50,12 @@ bool Value::IsFalse() const bool Value::IsString() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isString(); + return localToJSValue().isString(); } Maybe Value::Uint32Value(Local context) const { - auto js_value = localToJSValue(context->globalObject()->V8GlobalInternals()); + auto js_value = localToJSValue(); uint32_t value; if (js_value.getUInt32(value)) { return Just(value); @@ -65,12 +65,12 @@ Maybe Value::Uint32Value(Local context) const bool Value::FullIsTrue() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isTrue(); + return localToJSValue().isTrue(); } bool Value::FullIsFalse() const { - return localToJSValue(Isolate::GetCurrent()->globalInternals()).isFalse(); + return localToJSValue().isFalse(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp index 8b02503264722..1071bb111bb7c 100644 --- a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp @@ -93,7 +93,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb return JSValue::encode(JSC::jsUndefined()); } else { Local local_ret(&implicit_args.return_value); - return JSValue::encode(local_ret->localToJSValue(isolate->globalInternals())); + return JSValue::encode(local_ret->localToJSValue()); } } diff --git a/src/bun.js/bindings/v8/shim/Oddball.h b/src/bun.js/bindings/v8/shim/Oddball.h index b8b3e9ed1f854..5f2dfbf51937f 100644 --- a/src/bun.js/bindings/v8/shim/Oddball.h +++ b/src/bun.js/bindings/v8/shim/Oddball.h @@ -2,6 +2,7 @@ #include "TaggedPointer.h" #include "Map.h" +#include "JavaScriptCore/JSCJSValue.h" namespace v8 { namespace shim { @@ -24,6 +25,27 @@ struct Oddball { , m_kind(TaggedPointer(static_cast(kind))) { } + + Kind kind() const + { + return (Kind)m_kind.getSmiUnchecked(); + } + + JSC::JSValue toJSValue() const + { + switch (kind()) { + case Kind::kUndefined: + return JSC::jsUndefined(); + case Kind::kNull: + return JSC::jsNull(); + case Kind::kTrue: + return JSC::jsBoolean(true); + case Kind::kFalse: + return JSC::jsBoolean(false); + default: + RELEASE_ASSERT_NOT_REACHED(); + } + } }; } // namespace shim From 6d9ec16629d396c642ec58f4c22ccc9034752400 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 4 Sep 2024 11:23:35 -0700 Subject: [PATCH 14/30] Make V8 garbage collection test more strict --- test/v8/v8-module/main.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 0a909242a0147..376c62ea3045e 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -382,14 +382,19 @@ static Local setup_object_with_string_field(Isolate *isolate, return ehs.Escape(o); } -static void examine_object_fields(Isolate *isolate, Local o) { +static void examine_object_fields(Isolate *isolate, Local o, + int expected_field0, int expected_field1) { char buf[16]; HandleScope hs(isolate); o->GetInternalField(0).As()->WriteUtf8(isolate, buf); + assert(atoi(buf) == expected_field0); - Local field1 = o->GetInternalField(1); - if (field1.As()->IsString()) { + Local field1 = o->GetInternalField(1).As(); + if (field1->IsString()) { field1.As()->WriteUtf8(isolate, buf); + assert(atoi(buf) == expected_field1); + } else { + assert(field1->IsUndefined()); } } @@ -425,7 +430,7 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { // this should cause GC to start looking for objects to free // after each big string allocation, we try reading all of the strings we // created above to ensure they are still alive - constexpr size_t num_strings = 100; + constexpr size_t num_strings = 50; constexpr size_t string_size = 20 * 1000 * 1000; auto string_data = new char[string_size]; @@ -442,10 +447,12 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { for (size_t j = 0; j < num_small_allocs; j++) { char buf[16]; mini_strings[j]->WriteUtf8(isolate, buf); + assert(atoi(buf) == (int)j); } for (size_t j = 0; j < num_small_allocs; j++) { - examine_object_fields(isolate, objects[j]); + examine_object_fields(isolate, objects[j], j + num_small_allocs, + j + 2 * num_small_allocs); } if (i == 1) { @@ -464,6 +471,14 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { } } + memset(string_data, 0, string_size); + for (size_t i = 0; i < num_strings; i++) { + huge_strings[i]->WriteUtf8(isolate, string_data); + for (size_t j = 0; j < string_size - 1; j++) { + assert(string_data[j] == (char)(i + 1)); + } + } + delete[] string_data; } From 6efb86a78bc8f597d8b872e1104a2854d08cc050 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 4 Sep 2024 11:40:21 -0700 Subject: [PATCH 15/30] Turn some internal V8 types into structs and reduce diret member access --- .../bindings/v8/V8EscapableHandleScopeBase.cpp | 2 +- src/bun.js/bindings/v8/V8HandleScope.cpp | 2 +- src/bun.js/bindings/v8/shim/Handle.h | 13 ++++++++++++- src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp | 12 ++++++------ src/bun.js/bindings/v8/shim/TaggedPointer.h | 9 +++++++++ src/bun.js/bindings/v8/v8_api_internal.cpp | 2 +- 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index cb24b65e19af2..c679b37e6f629 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -21,7 +21,7 @@ uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value) m_isolate, m_escapeSlot); m_escapeSlot = nullptr; - return &newHandle->m_value; + return newHandle->asRawPtr(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index c00bfcd84665b..78e9d483379ac 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -27,7 +27,7 @@ uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t val auto* handleScope = isolate->globalInternals()->currentHandleScope(); TaggedPointer* newSlot = handleScope->m_buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate); // basically a reinterpret - return &newSlot->m_value; + return newSlot->asRawPtr(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/Handle.h b/src/bun.js/bindings/v8/shim/Handle.h index ceed92ccfe43c..d73d3a6ef421c 100644 --- a/src/bun.js/bindings/v8/shim/Handle.h +++ b/src/bun.js/bindings/v8/shim/Handle.h @@ -6,7 +6,7 @@ namespace v8 { namespace shim { -class ObjectLayout { +struct ObjectLayout { private: // these two fields are laid out so that V8 can find the map TaggedPointer m_taggedMap; @@ -128,6 +128,17 @@ struct Handle { } } + TaggedPointer* slot() + { + return &m_toV8Object; + } + + JSC::WriteBarrier asCell() const + { + return m_object.m_contents.cell; + } + +private: // if not SMI, holds &this->map so that V8 can see what kind of object this is TaggedPointer m_toV8Object; ObjectLayout m_object; diff --git a/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp index 50b3dd4f7ee1c..f588210ca1e35 100644 --- a/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp @@ -34,7 +34,7 @@ void HandleScopeBuffer::visitChildrenImpl(JSCell* cell, Visitor& visitor) for (auto& handle : thisObject->m_storage) { if (handle.isCell()) { - visitor.append(handle.m_object.m_contents.cell); + visitor.append(handle.asCell()); } } } @@ -52,21 +52,21 @@ TaggedPointer* HandleScopeBuffer::createHandle(JSCell* ptr, const Map* map, JSC: { auto& handle = createEmptyHandle(); handle = Handle(map, ptr, vm, this); - return &handle.m_toV8Object; + return handle.slot(); } TaggedPointer* HandleScopeBuffer::createSmiHandle(int32_t smi) { auto& handle = createEmptyHandle(); handle = Handle(smi); - return &handle.m_toV8Object; + return handle.slot(); } TaggedPointer* HandleScopeBuffer::createDoubleHandle(double value) { auto& handle = createEmptyHandle(); handle = Handle(value); - return &handle.m_toV8Object; + return handle.slot(); } TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer address, Isolate* isolate, Handle* reuseHandle) @@ -75,7 +75,7 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a if (address.getSmi(smi)) { if (reuseHandle) { *reuseHandle = Handle(smi); - return &reuseHandle->m_toV8Object; + return reuseHandle->slot(); } else { return createSmiHandle(smi); } @@ -92,7 +92,7 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a } if (reuseHandle) { *reuseHandle = Handle(v8_object->map(), v8_object->asCell(), vm(), this); - return &reuseHandle->m_toV8Object; + return reuseHandle->slot(); } else { return createHandle(v8_object->asCell(), v8_object->map(), vm()); } diff --git a/src/bun.js/bindings/v8/shim/TaggedPointer.h b/src/bun.js/bindings/v8/shim/TaggedPointer.h index 2f4a9156a6995..46ea98701386c 100644 --- a/src/bun.js/bindings/v8/shim/TaggedPointer.h +++ b/src/bun.js/bindings/v8/shim/TaggedPointer.h @@ -11,8 +11,10 @@ namespace v8 { namespace shim { struct TaggedPointer { +private: uintptr_t m_value; +public: enum class Type : uint8_t { Smi, StrongPointer, @@ -36,6 +38,7 @@ struct TaggedPointer { { } + // Convert the raw integer representation of a tagged pointer into a TaggedPointer struct static TaggedPointer fromRaw(uintptr_t raw) { TaggedPointer tagged; @@ -43,6 +46,12 @@ struct TaggedPointer { return tagged; } + // Get a pointer to where this TaggedPointer is located + uintptr_t* asRawPtr() + { + return &m_value; + } + Type type() const { switch (m_value & 3) { diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index a4ffbda7afd62..2153b11f3c201 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -17,7 +17,7 @@ uintptr_t* GlobalizeReference(internal::Isolate* i_isolate, uintptr_t address) auto* isolate = reinterpret_cast(i_isolate); auto* globalHandles = isolate->globalInternals()->globalHandles(); TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), isolate); - return &newSlot->m_value; + return newSlot->asRawPtr(); } void DisposeGlobal(uintptr_t* location) From 87d4add863c12283d7d35249ae59fb704eed2a04 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 4 Sep 2024 11:55:48 -0700 Subject: [PATCH 16/30] Clean up roots handling --- src/bun.js/bindings/v8/V8HandleScope.h | 8 ++++---- src/bun.js/bindings/v8/V8Isolate.h | 8 +++++++- src/bun.js/bindings/v8/shim/GlobalInternals.h | 8 -------- .../bindings/v8/shim/HandleScopeBuffer.cpp | 20 +++++++++++++------ 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/bun.js/bindings/v8/V8HandleScope.h b/src/bun.js/bindings/v8/V8HandleScope.h index dd351a50d6784..8813525e39886 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.h +++ b/src/bun.js/bindings/v8/V8HandleScope.h @@ -28,13 +28,13 @@ class HandleScope { } else if (value.isNumber()) { return Local(m_buffer->createDoubleHandle(value.asNumber())); } else if (value.isUndefined()) { - return Local(m_isolate->globalInternals()->undefinedSlot()); + return Local(m_isolate->undefinedSlot()); } else if (value.isNull()) { - return Local(m_isolate->globalInternals()->nullSlot()); + return Local(m_isolate->nullSlot()); } else if (value.isTrue()) { - return Local(m_isolate->globalInternals()->trueSlot()); + return Local(m_isolate->trueSlot()); } else if (value.isFalse()) { - return Local(m_isolate->globalInternals()->falseSlot()); + return Local(m_isolate->falseSlot()); } else { V8_UNIMPLEMENTED(); return Local(); diff --git a/src/bun.js/bindings/v8/V8Isolate.h b/src/bun.js/bindings/v8/V8Isolate.h index e1e091cf4333d..e368844e380e1 100644 --- a/src/bun.js/bindings/v8/V8Isolate.h +++ b/src/bun.js/bindings/v8/V8Isolate.h @@ -39,7 +39,13 @@ class Isolate final { shim::GlobalInternals* globalInternals() { return m_globalInternals; } HandleScope* currentHandleScope(); - TaggedPointer* getRoot(int index) { return &m_roots[index]; } + TaggedPointer* undefinedSlot() { return &m_roots[Isolate::kUndefinedValueRootIndex]; } + + TaggedPointer* nullSlot() { return &m_roots[Isolate::kNullValueRootIndex]; } + + TaggedPointer* trueSlot() { return &m_roots[Isolate::kTrueValueRootIndex]; } + + TaggedPointer* falseSlot() { return &m_roots[Isolate::kFalseValueRootIndex]; } shim::GlobalInternals* m_globalInternals; Zig::GlobalObject* m_globalObject; diff --git a/src/bun.js/bindings/v8/shim/GlobalInternals.h b/src/bun.js/bindings/v8/shim/GlobalInternals.h index 872063c9a2a2e..04a47ed1131ef 100644 --- a/src/bun.js/bindings/v8/shim/GlobalInternals.h +++ b/src/bun.js/bindings/v8/shim/GlobalInternals.h @@ -63,14 +63,6 @@ class GlobalInternals : public JSC::JSCell { void setCurrentHandleScope(HandleScope* handleScope) { m_currentHandleScope = handleScope; } - TaggedPointer* undefinedSlot() { return m_isolate.getRoot(Isolate::kUndefinedValueRootIndex); } - - TaggedPointer* nullSlot() { return m_isolate.getRoot(Isolate::kNullValueRootIndex); } - - TaggedPointer* trueSlot() { return m_isolate.getRoot(Isolate::kTrueValueRootIndex); } - - TaggedPointer* falseSlot() { return m_isolate.getRoot(Isolate::kFalseValueRootIndex); } - Isolate* isolate() { return &m_isolate; } DECLARE_INFO; diff --git a/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp index f588210ca1e35..9f814d90a763a 100644 --- a/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp +++ b/src/bun.js/bindings/v8/shim/HandleScopeBuffer.cpp @@ -81,14 +81,22 @@ TaggedPointer* HandleScopeBuffer::createHandleFromExistingObject(TaggedPointer a } } else { auto* v8_object = address.getPtr(); - if (v8_object->m_taggedMap.getPtr()->m_instanceType == InstanceType::Oddball) { + if (v8_object->map()->m_instanceType == InstanceType::Oddball) { + using Kind = Oddball::Kind; // find which oddball this is - for (auto& root : isolate->m_roots) { - if (root == address) { - return &root; - } + switch (reinterpret_cast(v8_object)->kind()) { + case Kind::kNull: + return isolate->nullSlot(); + case Kind::kUndefined: + return isolate->undefinedSlot(); + case Kind::kTrue: + return isolate->trueSlot(); + case Kind::kFalse: + return isolate->falseSlot(); + default: + RELEASE_ASSERT_NOT_REACHED("HandleScopeBuffer::createHandleFromExistingObject passed an unknown Oddball kind: %d", + reinterpret_cast(v8_object)->kind()); } - RELEASE_ASSERT_NOT_REACHED("HandleScopeBuffer::createHandleFromExistingObject passed an Oddball which does not exist in Roots"); } if (reuseHandle) { *reuseHandle = Handle(v8_object->map(), v8_object->asCell(), vm(), this); From cd71965c579020cc282a9e87b13d97aa03164ad1 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 4 Sep 2024 13:05:53 -0700 Subject: [PATCH 17/30] Support creating internalized V8 strings --- src/bun.js/bindings/v8/V8String.cpp | 32 ++++++++++++++---- test/v8/v8-module/main.cpp | 51 +++++++++++++++++++---------- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/src/bun.js/bindings/v8/V8String.cpp b/src/bun.js/bindings/v8/V8String.cpp index a2e943939c688..7dba41a93faa3 100644 --- a/src/bun.js/bindings/v8/V8String.cpp +++ b/src/bun.js/bindings/v8/V8String.cpp @@ -9,8 +9,6 @@ namespace v8 { MaybeLocal String::NewFromUtf8(Isolate* isolate, char const* data, NewStringType type, int signed_length) { - // TODO(@190n) maybe use JSC::AtomString instead of ignoring type - (void)type; size_t length = 0; if (signed_length < 0) { length = strlen(data); @@ -25,15 +23,26 @@ MaybeLocal String::NewFromUtf8(Isolate* isolate, char const* data, NewSt auto& vm = isolate->vm(); std::span span(reinterpret_cast(data), length); + JSString* jsString = nullptr; // ReplacingInvalidSequences matches how v8 behaves here auto string = WTF::String::fromUTF8ReplacingInvalidSequences(span); - JSString* jsString = JSC::jsString(vm, string); + switch (type) { + case NewStringType::kNormal: + jsString = JSC::jsString(vm, string); + break; + + case NewStringType::kInternalized: + // don't create AtomString directly from the characters, as that gives an empty string + // instead of replacing invalid UTF-8 sequences + WTF::AtomString atom_string(string); + jsString = JSC::jsString(vm, atom_string); + break; + } return MaybeLocal(isolate->currentHandleScope()->createLocal(vm, jsString)); } MaybeLocal String::NewFromOneByte(Isolate* isolate, const uint8_t* data, NewStringType type, int signed_length) { - (void)type; size_t length = 0; if (signed_length < 0) { length = strlen(reinterpret_cast(data)); @@ -48,8 +57,19 @@ MaybeLocal String::NewFromOneByte(Isolate* isolate, const uint8_t* data, auto& vm = isolate->vm(); std::span span(data, length); - WTF::String string(span); - JSString* jsString = JSC::jsString(vm, string); + JSString* jsString = nullptr; + switch (type) { + case NewStringType::kNormal: { + WTF::String string(span); + jsString = JSC::jsString(vm, string); + break; + } + case NewStringType::kInternalized: { + WTF::AtomString atom_string(span); + jsString = JSC::jsString(vm, atom_string); + break; + } + } return MaybeLocal(isolate->currentHandleScope()->createLocal(vm, jsString)); } diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 376c62ea3045e..6188860cb290a 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -155,40 +155,55 @@ static void perform_string_test(const FunctionCallbackInfo &info, return ok(info); } +template +void perform_string_test_normal_and_internalized( + const FunctionCallbackInfo &info, const T *string_literal, + bool latin1 = false) { + Isolate *isolate = info.GetIsolate(); + + if (latin1) { + const uint8_t *string = reinterpret_cast(string_literal); + perform_string_test( + info, String::NewFromOneByte(isolate, string, NewStringType::kNormal) + .ToLocalChecked()); + perform_string_test(info, String::NewFromOneByte( + isolate, string, NewStringType::kInternalized) + .ToLocalChecked()); + + } else { + const char *string = reinterpret_cast(string_literal); + perform_string_test( + info, String::NewFromUtf8(isolate, string, NewStringType::kNormal) + .ToLocalChecked()); + perform_string_test( + info, String::NewFromUtf8(isolate, string, NewStringType::kInternalized) + .ToLocalChecked()); + } +} + void test_v8_string_ascii(const FunctionCallbackInfo &info) { - auto string = - String::NewFromUtf8(info.GetIsolate(), "hello world").ToLocalChecked(); - perform_string_test(info, string); + perform_string_test_normal_and_internalized(info, "hello world"); } void test_v8_string_utf8(const FunctionCallbackInfo &info) { const unsigned char trans_flag_unsigned[] = {240, 159, 143, 179, 239, 184, 143, 226, 128, 141, 226, 154, 167, 239, 184, 143, 0}; - const char *trans_flag = reinterpret_cast(trans_flag_unsigned); - auto string = - String::NewFromUtf8(info.GetIsolate(), trans_flag).ToLocalChecked(); - perform_string_test(info, string); + perform_string_test_normal_and_internalized(info, trans_flag_unsigned); } void test_v8_string_invalid_utf8(const FunctionCallbackInfo &info) { const unsigned char mixed_sequence_unsigned[] = {'o', 'h', ' ', 0xc0, 'n', 'o', 0xc2, '!', 0xf5, 0}; - const char *mixed_sequence = - reinterpret_cast(mixed_sequence_unsigned); - auto string = - String::NewFromUtf8(info.GetIsolate(), mixed_sequence).ToLocalChecked(); - perform_string_test(info, string); + perform_string_test_normal_and_internalized(info, mixed_sequence_unsigned); } void test_v8_string_latin1(const FunctionCallbackInfo &info) { const unsigned char latin1[] = {0xa1, 'b', 'u', 'n', '!', 0}; - auto string = - String::NewFromOneByte(info.GetIsolate(), latin1).ToLocalChecked(); - perform_string_test(info, string); - string = String::NewFromOneByte(info.GetIsolate(), latin1, - NewStringType::kNormal, 1) - .ToLocalChecked(); + perform_string_test_normal_and_internalized(info, latin1, true); + auto string = String::NewFromOneByte(info.GetIsolate(), latin1, + NewStringType::kNormal, 1) + .ToLocalChecked(); perform_string_test(info, string); } From 3358383d98345d352883968b4b7d69f2727deecd Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 6 Sep 2024 10:43:48 -0700 Subject: [PATCH 18/30] Increase GC level when running V8 tests --- test/v8/v8-module/main.cpp | 27 +++++++++++++++++++++++++-- test/v8/v8.test.ts | 15 +++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 6188860cb290a..2cf6f568e4342 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -382,6 +382,25 @@ void test_many_v8_locals(const FunctionCallbackInfo &info) { } } +void print_cell_location(Local v8_value, const char *fmt, ...) { + (void)v8_value; + (void)fmt; + // va_list ap; + // va_start(ap, fmt); + // vprintf(fmt, ap); + // va_end(ap); + + // uintptr_t *slot = *reinterpret_cast(&v8_value); + // uintptr_t tagged = *slot; + // uintptr_t addr = tagged & ~3; + // struct ObjectLayout { + // uintptr_t map; + // void *cell; + // }; + // void *cell = reinterpret_cast(addr)->cell; + // printf(" = %p\n", cell); +} + static Local setup_object_with_string_field(Isolate *isolate, Local context, Local tmp, @@ -389,9 +408,10 @@ static Local setup_object_with_string_field(Isolate *isolate, const std::string &str) { EscapableHandleScope ehs(isolate); Local o = tmp->NewInstance(context).ToLocalChecked(); - // print_cell_location(o, "objects[%5d] ", i); + print_cell_location(o, "objects[%3d] ", i); Local value = String::NewFromUtf8(isolate, str.c_str()).ToLocalChecked(); + print_cell_location(value, "objects[%3d]->0", i); o->SetInternalField(0, value); return ehs.Escape(o); @@ -418,19 +438,22 @@ void test_handle_scope_gc(const FunctionCallbackInfo &info) { Local context = isolate->GetCurrentContext(); // allocate a ton of objects - constexpr size_t num_small_allocs = 10000; + constexpr size_t num_small_allocs = 500; Local mini_strings[num_small_allocs]; for (size_t i = 0; i < num_small_allocs; i++) { std::string cpp_str = std::to_string(i); mini_strings[i] = String::NewFromUtf8(isolate, cpp_str.c_str()).ToLocalChecked(); + print_cell_location(mini_strings[i], "mini_strings[%3d]", i); } // allocate some objects with internal fields, to check that those are // traced Local tmp = ObjectTemplate::New(isolate); tmp->SetInternalFieldCount(2); + print_cell_location(tmp, "object template"); + print_cell_location(context, "context"); Local objects[num_small_allocs]; for (size_t i = 0; i < num_small_allocs; i++) { diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index 623bc77fcb8f5..105e4b946858f 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -247,7 +247,14 @@ function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, jsArgs: : directories.bunRelease; const exe = runtime == Runtime.node ? "node" : bunExe(); - const cmd = [exe, join(baseDir, "main.js"), testName, JSON.stringify(jsArgs), JSON.stringify(thisValue ?? null)]; + const cmd = [ + exe, + ...(runtime == Runtime.bun ? ["--smol"] : []), + join(baseDir, "main.js"), + testName, + JSON.stringify(jsArgs), + JSON.stringify(thisValue ?? null), + ]; if (buildMode == BuildMode.debug) { cmd.push("debug"); } @@ -255,7 +262,11 @@ function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, jsArgs: const exec = spawnSync({ cmd, cwd: baseDir, - env: bunEnv, + env: { + ...bunEnv, + BUN_GARBAGE_COLLECTOR_LEVEL: "2", + BUN_JSC_randomIntegrityAuditRate: "1.0", + }, }); const errs = exec.stderr.toString(); const crashMsg = `test ${testName} crashed under ${Runtime[runtime]} in ${BuildMode[buildMode]} mode`; From f2d76b39487c9ce38d30981dce252f6379bf34fb Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 6 Sep 2024 10:44:06 -0700 Subject: [PATCH 19/30] Fix GlobalInternals type info --- src/bun.js/bindings/v8/shim/GlobalInternals.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/v8/shim/GlobalInternals.h b/src/bun.js/bindings/v8/shim/GlobalInternals.h index 04a47ed1131ef..e55ac4af20f5d 100644 --- a/src/bun.js/bindings/v8/shim/GlobalInternals.h +++ b/src/bun.js/bindings/v8/shim/GlobalInternals.h @@ -21,7 +21,7 @@ class GlobalInternals : public JSC::JSCell { static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) { - return JSC::Structure::create(vm, globalObject, JSC::jsNull(), JSC::TypeInfo(JSC::ObjectType, StructureFlags), info(), 0, 0); + return JSC::Structure::create(vm, globalObject, JSC::jsNull(), JSC::TypeInfo(JSC::CellType, StructureFlags), info(), 0, 0); } template From f0a9081a7f04fe275aac6199d6fdade3dabc3726 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 11 Sep 2024 17:25:29 -0700 Subject: [PATCH 20/30] Fix build error --- cmake/targets/BuildBun.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/targets/BuildBun.cmake b/cmake/targets/BuildBun.cmake index 43ca8076c4e39..9bdfd0658e079 100644 --- a/cmake/targets/BuildBun.cmake +++ b/cmake/targets/BuildBun.cmake @@ -510,6 +510,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 ${BUN_USOCKETS_SOURCE}/src/crypto/*.cpp ${BUN_DEPS_SOURCE}/*.cpp ) From 8c60f2a9f7952f7009f750445ae70da4620335c1 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 13 Sep 2024 14:02:59 -0700 Subject: [PATCH 21/30] Move iso subspaces for V8 types alongside other Bun-specific iso subspaces --- src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h | 13 ++++++------- src/bun.js/bindings/webcore/DOMIsoSubspaces.h | 13 ++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h index 0059dd14013ed..8c057a501869a 100644 --- a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h @@ -50,6 +50,12 @@ class DOMClientIsoSubspaces { std::unique_ptr m_clientSubspaceForNAPIFunction; std::unique_ptr m_clientSubspaceForTTYWrapObject; std::unique_ptr m_clientSubspaceForNapiHandleScopeImpl; + std::unique_ptr m_clientSubspaceForObjectTemplate; + std::unique_ptr m_clientSubspaceForInternalFieldObject; + std::unique_ptr m_clientSubspaceForV8GlobalInternals; + std::unique_ptr m_clientSubspaceForHandleScopeBuffer; + std::unique_ptr m_clientSubspaceForFunctionTemplate; + std::unique_ptr m_clientSubspaceForV8Function; #include "ZigGeneratedClasses+DOMClientIsoSubspaces.h" /* --- bun --- */ @@ -907,12 +913,5 @@ class DOMClientIsoSubspaces { std::unique_ptr m_clientSubspaceForEventListener; std::unique_ptr m_clientSubspaceForEventTarget; std::unique_ptr m_clientSubspaceForEventEmitter; - // todo(@190n) move these up or move these elsewhere - std::unique_ptr m_clientSubspaceForObjectTemplate; - std::unique_ptr m_clientSubspaceForInternalFieldObject; - std::unique_ptr m_clientSubspaceForV8GlobalInternals; - std::unique_ptr m_clientSubspaceForHandleScopeBuffer; - std::unique_ptr m_clientSubspaceForFunctionTemplate; - std::unique_ptr m_clientSubspaceForV8Function; }; } // namespace WebCore diff --git a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h index 03cd9e07f9b91..93781dae55b39 100644 --- a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h @@ -50,6 +50,12 @@ class DOMIsoSubspaces { std::unique_ptr m_subspaceForNAPIFunction; std::unique_ptr m_subspaceForTTYWrapObject; std::unique_ptr m_subspaceForNapiHandleScopeImpl; + std::unique_ptr m_subspaceForObjectTemplate; + std::unique_ptr m_subspaceForInternalFieldObject; + std::unique_ptr m_subspaceForV8GlobalInternals; + std::unique_ptr m_subspaceForHandleScopeBuffer; + std::unique_ptr m_subspaceForFunctionTemplate; + std::unique_ptr m_subspaceForV8Function; #include "ZigGeneratedClasses+DOMIsoSubspaces.h" /*-- BUN --*/ @@ -911,13 +917,6 @@ class DOMIsoSubspaces { // std::unique_ptr m_subspaceForDOMFormData; // std::unique_ptr m_subspaceForDOMFormDataIterator; std::unique_ptr m_subspaceForDOMURL; - // todo(@190n) move up - std::unique_ptr m_subspaceForObjectTemplate; - std::unique_ptr m_subspaceForInternalFieldObject; - std::unique_ptr m_subspaceForV8GlobalInternals; - std::unique_ptr m_subspaceForHandleScopeBuffer; - std::unique_ptr m_subspaceForFunctionTemplate; - std::unique_ptr m_subspaceForV8Function; }; } // namespace WebCore From 0f1ddb425259434623b23832ecadf8eb1a3380cd Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 13 Sep 2024 14:03:23 -0700 Subject: [PATCH 22/30] Run compiles for V8 test module in parallel but without interleaving outputs --- test/v8/v8.test.ts | 66 +++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index e7c983f341c35..7ed1568d3320f 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -3,7 +3,17 @@ import { beforeAll, describe, expect, it } from "bun:test"; import { bunEnv, bunExe, tmpdirSync } from "harness"; import assert from "node:assert"; import fs from "node:fs/promises"; -import { join } from "path"; +import { join, basename } from "path"; + +enum Runtime { + node, + bun, +} + +enum BuildMode { + debug, + release, +} // clang-cl does not work on Windows with node-gyp 10.2.0, so we should not let that affect the // test environment @@ -43,7 +53,12 @@ async function install(srcDir: string, tmpDir: string, runtime: Runtime): Promis } } -async function build(srcDir: string, tmpDir: string, runtime: Runtime, buildMode: BuildMode): Promise { +async function build( + srcDir: string, + tmpDir: string, + runtime: Runtime, + buildMode: BuildMode, +): Promise<{ out: string; err: string; description: string }> { const build = spawn({ cmd: runtime == Runtime.bun @@ -52,13 +67,21 @@ async function build(srcDir: string, tmpDir: string, runtime: Runtime, buildMode cwd: tmpDir, env: bunEnv, stdin: "inherit", - stdout: "inherit", - stderr: "inherit", + stdout: "pipe", + stderr: "pipe", }); await build.exited; + const out = await new Response(build.stdout).text(); + const err = await new Response(build.stderr).text(); if (build.exitCode != 0) { + console.error(err); throw new Error("build failed"); } + return { + out, + err, + description: `build ${basename(srcDir)} with ${Runtime[runtime]} in ${BuildMode[buildMode]} mode`, + }; } beforeAll(async () => { @@ -73,10 +96,18 @@ beforeAll(async () => { await install(srcDir, directories.node, Runtime.node); await install(join(__dirname, "bad-modules"), directories.badModules, Runtime.node); - await build(srcDir, directories.bunRelease, Runtime.bun, BuildMode.release); - await build(srcDir, directories.bunDebug, Runtime.bun, BuildMode.debug); - await build(srcDir, directories.node, Runtime.node, BuildMode.release); - await build(join(__dirname, "bad-modules"), directories.badModules, Runtime.node, BuildMode.release); + const results = await Promise.all([ + build(srcDir, directories.bunRelease, Runtime.bun, BuildMode.release), + build(srcDir, directories.bunDebug, Runtime.bun, BuildMode.debug), + build(srcDir, directories.node, Runtime.node, BuildMode.release), + build(join(__dirname, "bad-modules"), directories.badModules, Runtime.node, BuildMode.release), + ]); + for (const r of results) { + console.log(r.description, "stdout:"); + console.log(r.out); + console.log(r.description, "stderr:"); + console.log(r.err); + } }); describe("module lifecycle", () => { @@ -196,25 +227,6 @@ describe("EscapableHandleScope", () => { }); }); -afterAll(async () => { - await Promise.all([ - fs.rm(directories.bunRelease, { recursive: true, force: true }), - fs.rm(directories.bunDebug, { recursive: true, force: true }), - fs.rm(directories.node, { recursive: true, force: true }), - fs.rm(directories.badModules, { recursive: true, force: true }), - ]); -}); - -enum Runtime { - node, - bun, -} - -enum BuildMode { - debug, - release, -} - function checkSameOutput(testName: string, args: any[], thisValue?: any) { const nodeResult = runOn(Runtime.node, BuildMode.release, testName, args, thisValue).trim(); let bunReleaseResult = runOn(Runtime.bun, BuildMode.release, testName, args, thisValue); From 08a3ffd6a151cc477bdba7a000d210e3bfe89231 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 13 Sep 2024 17:19:47 -0700 Subject: [PATCH 23/30] Give proper this value in v8::Function --- src/bun.js/bindings/v8/V8Function.cpp | 27 +++++++++++++++++-- src/bun.js/bindings/v8/V8Function.h | 12 +-------- src/bun.js/bindings/v8/V8Value.cpp | 5 ++++ src/bun.js/bindings/v8/V8Value.h | 1 + .../bindings/v8/shim/FunctionTemplate.cpp | 15 +++++++++-- src/napi/napi.zig | 4 +++ src/symbols.def | 2 ++ src/symbols.dyn | 2 ++ src/symbols.txt | 2 ++ test/v8/v8-module/main.cpp | 25 ++++++++++++++++- test/v8/v8-module/main.js | 10 ++++++- test/v8/v8-module/module.js | 19 +++++++++++++ test/v8/v8.test.ts | 7 ++++- 13 files changed, 113 insertions(+), 18 deletions(-) diff --git a/src/bun.js/bindings/v8/V8Function.cpp b/src/bun.js/bindings/v8/V8Function.cpp index 37bee7fb16738..8f6e1fed7d1ef 100644 --- a/src/bun.js/bindings/v8/V8Function.cpp +++ b/src/bun.js/bindings/v8/V8Function.cpp @@ -1,13 +1,36 @@ #include "V8Function.h" #include "shim/Function.h" +#include "V8HandleScope.h" namespace v8 { void Function::SetName(Local name) { - auto* thisObj = localToObjectPointer(); - thisObj->setName(name->localToJSString()); + if (auto* jsFunction = localToObjectPointer()) { + jsFunction->setFunctionName(jsFunction->globalObject(), name->localToJSString()); + } else if (auto* v8Function = localToObjectPointer()) { + v8Function->setName(name->localToJSString()); + } else { + RELEASE_ASSERT_NOT_REACHED("v8::Function::SetName called on invalid type"); + } +} + +Local Function::GetName() const +{ + WTF::String wtfString; + if (auto* jsFunction = localToObjectPointer()) { + wtfString = const_cast(jsFunction)->name(jsFunction->globalObject()->vm()); + } else if (auto* internalFunction = localToObjectPointer()) { + wtfString = const_cast(internalFunction)->name(); + } else { + RELEASE_ASSERT_NOT_REACHED("v8::Function::GetName called on invalid type"); + } + + auto* globalObject = jsCast(localToObjectPointer()->globalObject()); + auto* handleScope = globalObject->V8GlobalInternals()->currentHandleScope(); + auto* jsString = JSC::jsString(globalObject->vm(), wtfString); + return handleScope->createLocal(globalObject->vm(), jsString); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Function.h b/src/bun.js/bindings/v8/V8Function.h index f4d801a329c3b..bf781e959d19e 100644 --- a/src/bun.js/bindings/v8/V8Function.h +++ b/src/bun.js/bindings/v8/V8Function.h @@ -11,17 +11,7 @@ namespace v8 { class Function : public Object { public: BUN_EXPORT void SetName(Local name); - -private: - shim::Function* localToObjectPointer() - { - return Data::localToObjectPointer(); - } - - const shim::Function* localToObjectPointer() const - { - return Data::localToObjectPointer(); - } + BUN_EXPORT Local GetName() const; }; } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Value.cpp b/src/bun.js/bindings/v8/V8Value.cpp index e1acb50a9db90..0077bc8d0a814 100644 --- a/src/bun.js/bindings/v8/V8Value.cpp +++ b/src/bun.js/bindings/v8/V8Value.cpp @@ -53,6 +53,11 @@ bool Value::IsString() const return localToJSValue().isString(); } +bool Value::IsFunction() const +{ + return JSC::jsTypeofIsFunction(defaultGlobalObject(), localToJSValue()); +} + Maybe Value::Uint32Value(Local context) const { auto js_value = localToJSValue(); diff --git a/src/bun.js/bindings/v8/V8Value.h b/src/bun.js/bindings/v8/V8Value.h index b658e4efaf6a1..9cf7a1b86c3da 100644 --- a/src/bun.js/bindings/v8/V8Value.h +++ b/src/bun.js/bindings/v8/V8Value.h @@ -13,6 +13,7 @@ class Value : public Data { BUN_EXPORT bool IsObject() const; BUN_EXPORT bool IsNumber() const; BUN_EXPORT bool IsUint32() const; + BUN_EXPORT bool IsFunction() const; BUN_EXPORT Maybe Uint32Value(Local context) const; // usually inlined: diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp index 1071bb111bb7c..14ce908afe3ff 100644 --- a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp @@ -9,6 +9,9 @@ using JSC::JSValue; using JSC::Structure; namespace v8 { + +class Object; + namespace shim { // for CREATE_METHOD_TABLE @@ -62,8 +65,16 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb WTF::Vector args(callFrame->argumentCount() + 1); HandleScope hs(isolate); - Local thisValue = hs.createLocal(vm, callFrame->thisValue()); - args[0] = thisValue.tagged(); + + // V8 function calls always run in "sloppy mode," even if the JS side is in strict mode. So if + // `this` is null or undefined, we use globalThis instead; otherwise, we convert `this` to an + // object. + JSC::JSObject* jscThis = globalObject->globalThis(); + if (!callFrame->thisValue().isUndefinedOrNull()) { + jscThis = callFrame->thisValue().toObject(globalObject); + } + Local thisObject = hs.createLocal(vm, jscThis); + args[0] = thisObject.tagged(); for (size_t i = 0; i < callFrame->argumentCount(); i++) { Local argValue = hs.createLocal(vm, callFrame->argument(i)); diff --git a/src/napi/napi.zig b/src/napi/napi.zig index 088960a4b4a51..8228e77734232 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -1838,6 +1838,8 @@ const V8API = if (!bun.Environment.isWindows) struct { pub extern fn _ZNK2v86String19ContainsOnlyOneByteEv() *anyopaque; pub extern fn _ZN2v812api_internal18GlobalizeReferenceEPNS_8internal7IsolateEm() *anyopaque; pub extern fn _ZN2v812api_internal13DisposeGlobalEPm() *anyopaque; + pub extern fn _ZNK2v88Function7GetNameEv() *anyopaque; + pub extern fn _ZNK2v85Value10IsFunctionEv() *anyopaque; } else struct { // MSVC name mangling is different than it is on unix. // To make this easier to deal with, I have provided a script to generate the list of functions. @@ -1904,6 +1906,8 @@ const V8API = if (!bun.Environment.isWindows) struct { pub extern fn @"?ContainsOnlyOneByte@String@v8@@QEBA_NXZ"() *anyopaque; pub extern fn @"?GlobalizeReference@api_internal@v8@@YAPEA_KPEAVIsolate@internal@2@_K@Z"() *anyopaque; pub extern fn @"?DisposeGlobal@api_internal@v8@@YAXPEA_K@Z"() *anyopaque; + pub extern fn @"?GetName@Function@v8@@QEBA?AV?$Local@VValue@v8@@@2@XZ"() *anyopaque; + pub extern fn @"?IsFunction@Value@v8@@QEBA_NXZ"() *anyopaque; }; pub fn fixDeadCodeElimination() void { diff --git a/src/symbols.def b/src/symbols.def index 71f031ae9b34d..4c45040d116f6 100644 --- a/src/symbols.def +++ b/src/symbols.def @@ -626,3 +626,5 @@ EXPORTS ?ContainsOnlyOneByte@String@v8@@QEBA_NXZ ?GlobalizeReference@api_internal@v8@@YAPEA_KPEAVIsolate@internal@2@_K@Z ?DisposeGlobal@api_internal@v8@@YAXPEA_K@Z + ?GetName@Function@v8@@QEBA?AV?$Local@VValue@v8@@@2@XZ + ?IsFunction@Value@v8@@QEBA_NXZ diff --git a/src/symbols.dyn b/src/symbols.dyn index e49d1f06eb606..19bc55fe1c9d7 100644 --- a/src/symbols.dyn +++ b/src/symbols.dyn @@ -213,4 +213,6 @@ __ZNK2v86String19ContainsOnlyOneByteEv; __ZN2v812api_internal18GlobalizeReferenceEPNS_8internal7IsolateEm; __ZN2v812api_internal13DisposeGlobalEPm; + __ZNK2v88Function7GetNameEv; + __ZNK2v85Value10IsFunctionEv; }; diff --git a/src/symbols.txt b/src/symbols.txt index 6d692d24e6944..758c8eb265ae2 100644 --- a/src/symbols.txt +++ b/src/symbols.txt @@ -212,3 +212,5 @@ __ZNK2v86String9IsOneByteEv __ZNK2v86String19ContainsOnlyOneByteEv __ZN2v812api_internal18GlobalizeReferenceEPNS_8internal7IsolateEm __ZN2v812api_internal13DisposeGlobalEPm +__ZNK2v88Function7GetNameEv +__ZNK2v85Value10IsFunctionEv diff --git a/test/v8/v8-module/main.cpp b/test/v8/v8-module/main.cpp index 2cf6f568e4342..a47e950f806fa 100644 --- a/test/v8/v8-module/main.cpp +++ b/test/v8/v8-module/main.cpp @@ -24,6 +24,12 @@ using namespace v8; namespace v8tests { +static void run_gc(const FunctionCallbackInfo &info) { + auto *isolate = info.GetIsolate(); + auto context = isolate->GetCurrentContext(); + (void)info[0].As()->Call(context, Null(isolate), 0, nullptr); +} + static void log_buffer(const char *buf, int len) { for (int i = 0; i < len; i++) { printf("buf[%d] = 0x%02x\n", i, buf[i]); @@ -46,6 +52,14 @@ static std::string describe(Isolate *isolate, Local value) { result += buf; result += "\""; return result; + } else if (value->IsFunction()) { + char buf[1024] = {0}; + value.As()->GetName().As()->WriteUtf8(isolate, buf, + sizeof(buf) - 1); + std::string result = "function "; + result += buf; + result += "()"; + return result; } else if (value->IsObject()) { return "[object Object]"; } else if (value->IsNumber()) { @@ -324,6 +338,9 @@ void create_function_with_data(const FunctionCallbackInfo &info) { Local tmp = FunctionTemplate::New(isolate, return_data_callback, s); Local f = tmp->GetFunction(context).ToLocalChecked(); + Local name = + String::NewFromUtf8(isolate, "function_with_data").ToLocalChecked(); + f->SetName(name); info.GetReturnValue().Set(f); } @@ -337,6 +354,10 @@ void print_values_from_js(const FunctionCallbackInfo &info) { return ok(info); } +void return_this(const FunctionCallbackInfo &info) { + info.GetReturnValue().Set(info.This()); +} + class GlobalTestWrapper { public: static void set(const FunctionCallbackInfo &info); @@ -356,7 +377,8 @@ void GlobalTestWrapper::set(const FunctionCallbackInfo &info) { } else { info.GetReturnValue().Set(value.Get(isolate)); } - value.Reset(isolate, info[0]); + const auto new_value = info[0]; + value.Reset(isolate, new_value); } void GlobalTestWrapper::get(const FunctionCallbackInfo &info) { @@ -580,6 +602,7 @@ void initialize(Local exports, Local module, NODE_SET_METHOD(exports, "create_function_with_data", create_function_with_data); NODE_SET_METHOD(exports, "print_values_from_js", print_values_from_js); + NODE_SET_METHOD(exports, "return_this", return_this); NODE_SET_METHOD(exports, "global_get", GlobalTestWrapper::get); NODE_SET_METHOD(exports, "global_set", GlobalTestWrapper::set); NODE_SET_METHOD(exports, "test_many_v8_locals", test_many_v8_locals); diff --git a/test/v8/v8-module/main.js b/test/v8/v8-module/main.js index 1fdb23342f9b6..ad5d9afc69ad0 100644 --- a/test/v8/v8-module/main.js +++ b/test/v8/v8-module/main.js @@ -1,3 +1,4 @@ +"use strict"; // usage: bun/node main.js [JSON array of arguments] [JSON `this` value] [debug] const buildMode = process.argv[5]; @@ -8,11 +9,18 @@ const testName = process.argv[2]; const args = JSON.parse(process.argv[3] ?? "[]"); const thisValue = JSON.parse(process.argv[4] ?? "null"); +function runGC() { + if (typeof Bun !== "undefined") { + Bun.gc(true); + } +} + const fn = tests[testName]; if (typeof fn !== "function") { throw new Error("Unknown test:", testName); } -const result = fn.apply(thisValue, args); +const result = fn.apply(thisValue, [runGC, ...args]); if (result) { + console.log(result == global); throw new Error(result); } diff --git a/test/v8/v8-module/module.js b/test/v8/v8-module/module.js index fcbd23231c87b..91a7fe9b78dc3 100644 --- a/test/v8/v8-module/module.js +++ b/test/v8/v8-module/module.js @@ -26,5 +26,24 @@ module.exports = debugMode => { } console.log(f()); }, + + print_native_function() { + nativeModule.print_values_from_js(nativeModule.create_function_with_data()); + }, + + call_function_with_weird_this_values() { + for (const thisValue of [null, undefined, 5, "abc"]) { + const ret = nativeModule.return_this.call(thisValue); + console.log("typeof =", typeof ret); + if (ret == globalThis) { + console.log("returned globalThis"); + } else if (ret instanceof String) { + console.log("returned boxed String:", ret.toString()); + } else { + console.log("returned", ret); + } + console.log("constructor is", ret.constructor.name); + } + }, }; }; diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index 7ed1568d3320f..02b192fba2843 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -188,7 +188,12 @@ describe("FunctionTemplate", () => { describe("Function", () => { it("correctly receives all its arguments from JS", () => { - checkSameOutput("print_values_from_js", [5.0, true, null, false, "meow", {}], {}); + checkSameOutput("print_values_from_js", [5.0, true, null, false, "meow", {}]); + checkSameOutput("print_native_function", []); + }); + + it("correctly receives the this value from JS", () => { + checkSameOutput("call_function_with_weird_this_values", []); }); }); From d6770836aed2b12889c3d8e83c881a14761e5070 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 13 Sep 2024 17:36:24 -0700 Subject: [PATCH 24/30] Use it.skipIf for uv_os_* tests --- test/v8/v8.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index 82754fc1a52af..73998e108d8e5 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -1,6 +1,6 @@ import { spawn, spawnSync } from "bun"; import { beforeAll, describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, tmpdirSync } from "harness"; +import { bunEnv, bunExe, tmpdirSync, isWindows } from "harness"; import assert from "node:assert"; import fs from "node:fs/promises"; import { join, basename } from "path"; @@ -232,16 +232,14 @@ describe("EscapableHandleScope", () => { }); }); -const itIfNotWindows = process.platform == "win32" ? it.skip : it; - describe("uv_os_getpid", () => { - itIfNotWindows("returns the same result as getpid on POSIX", () => { + it.skipIf(isWindows)("returns the same result as getpid on POSIX", () => { checkSameOutput("test_uv_os_getpid", []); }); }); describe("uv_os_getppid", () => { - itIfNotWindows("returns the same result as getppid on POSIX", () => { + it.skipIf(isWindows)("returns the same result as getppid on POSIX", () => { checkSameOutput("test_uv_os_getppid", []); }); }); From c28c25fc3c6c0d34190d4c855212f3e44965f60d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 20 Sep 2024 10:03:17 -0700 Subject: [PATCH 25/30] Restore environment variables in V8 test --- test/v8/v8.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index 73998e108d8e5..a1c595649e6de 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -15,6 +15,8 @@ enum BuildMode { release, } +console.log(bunEnv); + // clang-cl does not work on Windows with node-gyp 10.2.0, so we should not let that affect the // test environment delete bunEnv.CC; @@ -287,11 +289,7 @@ function runOn(runtime: Runtime, buildMode: BuildMode, testName: string, jsArgs: const exec = spawnSync({ cmd, cwd: baseDir, - env: { - ...bunEnv, - BUN_GARBAGE_COLLECTOR_LEVEL: "2", - BUN_JSC_randomIntegrityAuditRate: "1.0", - }, + env: bunEnv, }); const errs = exec.stderr.toString(); const crashMsg = `test ${testName} crashed under ${Runtime[runtime]} in ${BuildMode[buildMode]} mode`; From 01b51bee92a7d194ed8327baf2da80015f1dfeb0 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 20 Sep 2024 10:08:05 -0700 Subject: [PATCH 26/30] Don't log the environment variables --- test/v8/v8.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/v8/v8.test.ts b/test/v8/v8.test.ts index a1c595649e6de..ad7b2b1a3d92b 100644 --- a/test/v8/v8.test.ts +++ b/test/v8/v8.test.ts @@ -15,8 +15,6 @@ enum BuildMode { release, } -console.log(bunEnv); - // clang-cl does not work on Windows with node-gyp 10.2.0, so we should not let that affect the // test environment delete bunEnv.CC; From ffd1e18d2c6460fc336a794e5edb9f6c37f9a151 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 25 Sep 2024 18:33:33 -0700 Subject: [PATCH 27/30] Add static assertions that size, alignment, offsets, and constant values match V8 --- cmake/targets/BuildBun.cmake | 3 + src/bun.js/bindings/v8/V8Array.cpp | 3 + src/bun.js/bindings/v8/V8Boolean.cpp | 3 + src/bun.js/bindings/v8/V8Context.cpp | 3 + .../bindings/v8/V8EscapableHandleScope.cpp | 3 + .../v8/V8EscapableHandleScopeBase.cpp | 3 + src/bun.js/bindings/v8/V8External.cpp | 4 +- src/bun.js/bindings/v8/V8Function.cpp | 4 +- .../bindings/v8/V8FunctionCallbackInfo.cpp | 23 ++++++++ .../bindings/v8/V8FunctionCallbackInfo.h | 6 +- src/bun.js/bindings/v8/V8FunctionTemplate.cpp | 10 ++++ src/bun.js/bindings/v8/V8FunctionTemplate.h | 2 + src/bun.js/bindings/v8/V8HandleScope.cpp | 7 ++- src/bun.js/bindings/v8/V8Isolate.cpp | 17 ++++++ src/bun.js/bindings/v8/V8Isolate.h | 2 - src/bun.js/bindings/v8/V8Local.cpp | 5 ++ src/bun.js/bindings/v8/V8Maybe.cpp | 6 ++ src/bun.js/bindings/v8/V8Number.cpp | 3 + src/bun.js/bindings/v8/V8Object.cpp | 4 +- src/bun.js/bindings/v8/V8ObjectTemplate.cpp | 3 + src/bun.js/bindings/v8/V8String.cpp | 13 ++++- src/bun.js/bindings/v8/V8Template.cpp | 3 + src/bun.js/bindings/v8/V8Value.cpp | 3 + src/bun.js/bindings/v8/node.cpp | 6 +- src/bun.js/bindings/v8/real_v8.h | 7 +++ .../bindings/v8/shim/FunctionTemplate.cpp | 5 +- src/bun.js/bindings/v8/shim/Handle.cpp | 53 ++++++++++++++++++ src/bun.js/bindings/v8/shim/Handle.h | 56 +++---------------- src/bun.js/bindings/v8/shim/Map.cpp | 19 +++++++ src/bun.js/bindings/v8/shim/Oddball.cpp | 18 ++++++ src/bun.js/bindings/v8/shim/Oddball.h | 4 +- .../bindings/v8/v8_compatibility_assertions.h | 50 +++++++++++++++++ src/bun.js/bindings/v8/v8config.h | 3 + 33 files changed, 291 insertions(+), 63 deletions(-) create mode 100644 src/bun.js/bindings/v8/V8FunctionCallbackInfo.cpp create mode 100644 src/bun.js/bindings/v8/V8Local.cpp create mode 100644 src/bun.js/bindings/v8/V8Maybe.cpp create mode 100644 src/bun.js/bindings/v8/real_v8.h create mode 100644 src/bun.js/bindings/v8/shim/Handle.cpp create mode 100644 src/bun.js/bindings/v8/shim/Oddball.cpp create mode 100644 src/bun.js/bindings/v8/v8_compatibility_assertions.h create mode 100644 src/bun.js/bindings/v8/v8config.h diff --git a/cmake/targets/BuildBun.cmake b/cmake/targets/BuildBun.cmake index 45f3e6f298eaa..6e498009ff094 100644 --- a/cmake/targets/BuildBun.cmake +++ b/cmake/targets/BuildBun.cmake @@ -613,6 +613,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} @@ -690,6 +691,7 @@ 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 @@ -697,6 +699,7 @@ target_include_directories(${bun} PRIVATE ${CODEGEN_PATH} ${VENDOR_PATH} ${VENDOR_PATH}/picohttpparser + ${NODEJS_HEADERS_PATH}/include ) # --- C/C++ Definitions --- diff --git a/src/bun.js/bindings/v8/V8Array.cpp b/src/bun.js/bindings/v8/V8Array.cpp index 0af06548d4201..27f4265ee0ce2 100644 --- a/src/bun.js/bindings/v8/V8Array.cpp +++ b/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; diff --git a/src/bun.js/bindings/v8/V8Boolean.cpp b/src/bun.js/bindings/v8/V8Boolean.cpp index 42ad98b82b076..5e5a5e6d4c1e1 100644 --- a/src/bun.js/bindings/v8/V8Boolean.cpp +++ b/src/bun.js/bindings/v8/V8Boolean.cpp @@ -1,5 +1,8 @@ #include "V8Boolean.h" #include "V8HandleScope.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Boolean) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8Context.cpp b/src/bun.js/bindings/v8/V8Context.cpp index 2019de4eaec2c..35a1ec09545da 100644 --- a/src/bun.js/bindings/v8/V8Context.cpp +++ b/src/bun.js/bindings/v8/V8Context.cpp @@ -1,4 +1,7 @@ #include "V8Context.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Context) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp index 091db0724b25f..5135aeb8e8e7e 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScope.cpp +++ b/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 { diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index c679b37e6f629..1aa8e117b6079 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/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 { diff --git a/src/bun.js/bindings/v8/V8External.cpp b/src/bun.js/bindings/v8/V8External.cpp index 7d21c67ed153c..51023c986356d 100644 --- a/src/bun.js/bindings/v8/V8External.cpp +++ b/src/bun.js/bindings/v8/V8External.cpp @@ -1,8 +1,10 @@ #include "V8External.h" #include "V8HandleScope.h" - +#include "v8_compatibility_assertions.h" #include "napi_external.h" +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::External) + namespace v8 { Local External::New(Isolate* isolate, void* value) diff --git a/src/bun.js/bindings/v8/V8Function.cpp b/src/bun.js/bindings/v8/V8Function.cpp index 8f6e1fed7d1ef..ad2823e4ab6c3 100644 --- a/src/bun.js/bindings/v8/V8Function.cpp +++ b/src/bun.js/bindings/v8/V8Function.cpp @@ -1,7 +1,9 @@ #include "V8Function.h" - #include "shim/Function.h" #include "V8HandleScope.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Function) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8FunctionCallbackInfo.cpp b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.cpp new file mode 100644 index 0000000000000..f819282b5b757 --- /dev/null +++ b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.cpp @@ -0,0 +1,23 @@ +#include "V8FunctionCallbackInfo.h" +#include "real_v8.h" +#include "v8_compatibility_assertions.h" + +// Check that the offset of a field in our ImplicitArgs struct matches the array index +// that V8 uses to access that field +#define CHECK_IMPLICIT_ARG(BUN_NAME, V8_NAME) \ + static_assert(offsetof(v8::ImplicitArgs, BUN_NAME) \ + == sizeof(void*) * real_v8::FunctionCallbackInfo::V8_NAME, \ + "Position of `" #BUN_NAME "` in implicit arguments does not match V8"); + +CHECK_IMPLICIT_ARG(holder, kHolderIndex) +CHECK_IMPLICIT_ARG(isolate, kIsolateIndex) +CHECK_IMPLICIT_ARG(unused, kUnusedIndex) +CHECK_IMPLICIT_ARG(return_value, kReturnValueIndex) +CHECK_IMPLICIT_ARG(data, kDataIndex) +CHECK_IMPLICIT_ARG(new_target, kNewTargetIndex) + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::FunctionCallbackInfo) + +ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::FunctionCallbackInfo, implicit_args, implicit_args_) +ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::FunctionCallbackInfo, values, values_) +ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::FunctionCallbackInfo, length, length_) diff --git a/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h index 194af693619fd..7e9f6f9177321 100644 --- a/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h +++ b/src/bun.js/bindings/v8/V8FunctionCallbackInfo.h @@ -12,24 +12,24 @@ struct ImplicitArgs { // v8-function-callback.h:168 void* holder; Isolate* isolate; - Context* context; + void* unused; // overwritten by the callback TaggedPointer return_value; // holds the value passed for data in FunctionTemplate::New - TaggedPointer target; + TaggedPointer data; void* new_target; }; // T = return value template class FunctionCallbackInfo { +public: // V8 treats this as an array of pointers ImplicitArgs* implicit_args; // index -1 is this TaggedPointer* values; int length; -public: FunctionCallbackInfo(ImplicitArgs* implicit_args_, TaggedPointer* values_, int length_) : implicit_args(implicit_args_) , values(values_) diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp index d8743ed867984..36fdf022a17d0 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.cpp @@ -2,6 +2,16 @@ #include "V8Function.h" #include "V8HandleScope.h" #include "shim/FunctionTemplate.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::FunctionTemplate) + +ASSERT_V8_ENUM_MATCHES(ConstructorBehavior, kThrow) +ASSERT_V8_ENUM_MATCHES(ConstructorBehavior, kAllow) + +ASSERT_V8_ENUM_MATCHES(SideEffectType, kHasSideEffect) +ASSERT_V8_ENUM_MATCHES(SideEffectType, kHasNoSideEffect) +ASSERT_V8_ENUM_MATCHES(SideEffectType, kHasSideEffectToReceiver) using JSC::JSCell; using JSC::JSValue; diff --git a/src/bun.js/bindings/v8/V8FunctionTemplate.h b/src/bun.js/bindings/v8/V8FunctionTemplate.h index 28a1688d476c9..873b45a0daa23 100644 --- a/src/bun.js/bindings/v8/V8FunctionTemplate.h +++ b/src/bun.js/bindings/v8/V8FunctionTemplate.h @@ -30,6 +30,8 @@ enum class SideEffectType { kHasSideEffectToReceiver, }; +// Only used by v8 fast API calls, which Node.js doesn't seem to intend to support +// (v8-fast-api-calls.h is not in the headers distribution) class CFunction { private: const void* address; diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index 78e9d483379ac..10053311afd68 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -1,6 +1,11 @@ #include "V8HandleScope.h" - #include "shim/GlobalInternals.h" +#include "v8_compatibility_assertions.h" + +// I haven't found an inlined function which accesses HandleScope fields, so I'm assuming the field +// offsets do *not* need to match (also, our fields have different types and meanings anyway). +// But the size must match, because if our HandleScope is too big it'll clobber other stack variables. +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::HandleScope) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8Isolate.cpp b/src/bun.js/bindings/v8/V8Isolate.cpp index 547ca7c19e277..f6385c53b99f1 100644 --- a/src/bun.js/bindings/v8/V8Isolate.cpp +++ b/src/bun.js/bindings/v8/V8Isolate.cpp @@ -2,6 +2,23 @@ #include "V8HandleScope.h" #include "shim/GlobalInternals.h" #include "ZigGlobalObject.h" +#include "real_v8.h" +#include "v8_compatibility_assertions.h" + +static_assert(offsetof(v8::Isolate, m_roots) == real_v8::internal::Internals::kIsolateRootsOffset, + "Isolate roots array is at wrong offset"); + +#define CHECK_ROOT_INDEX(NAME) \ + static_assert(v8::Isolate::NAME == real_v8::internal::Internals::NAME, \ + "Isolate root index " #NAME " does not match between Bun and V8"); \ + static_assert(v8::Isolate::NAME < std::tuple_size_v, \ + "Bun v8::Isolate roots array is too small for index " #NAME); + +CHECK_ROOT_INDEX(kUndefinedValueRootIndex) +CHECK_ROOT_INDEX(kTheHoleValueRootIndex) +CHECK_ROOT_INDEX(kNullValueRootIndex) +CHECK_ROOT_INDEX(kTrueValueRootIndex) +CHECK_ROOT_INDEX(kFalseValueRootIndex) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8Isolate.h b/src/bun.js/bindings/v8/V8Isolate.h index e368844e380e1..de603e0690023 100644 --- a/src/bun.js/bindings/v8/V8Isolate.h +++ b/src/bun.js/bindings/v8/V8Isolate.h @@ -55,6 +55,4 @@ class Isolate final { std::array m_roots; }; -static_assert(offsetof(Isolate, m_roots) == 592, "Isolate has wrong layout"); - } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8Local.cpp b/src/bun.js/bindings/v8/V8Local.cpp new file mode 100644 index 0000000000000..a2360b17e0ea3 --- /dev/null +++ b/src/bun.js/bindings/v8/V8Local.cpp @@ -0,0 +1,5 @@ +#include "V8Local.h" +#include "V8Data.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Local) diff --git a/src/bun.js/bindings/v8/V8Maybe.cpp b/src/bun.js/bindings/v8/V8Maybe.cpp new file mode 100644 index 0000000000000..ff06921249d05 --- /dev/null +++ b/src/bun.js/bindings/v8/V8Maybe.cpp @@ -0,0 +1,6 @@ +#include "V8Maybe.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Maybe) +ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::Maybe, m_hasValue, has_value_) +ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::Maybe, m_value, value_) diff --git a/src/bun.js/bindings/v8/V8Number.cpp b/src/bun.js/bindings/v8/V8Number.cpp index 4e20336d83d70..c870d1ef38769 100644 --- a/src/bun.js/bindings/v8/V8Number.cpp +++ b/src/bun.js/bindings/v8/V8Number.cpp @@ -1,5 +1,8 @@ #include "V8Number.h" #include "V8HandleScope.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Number) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8Object.cpp b/src/bun.js/bindings/v8/V8Object.cpp index 07dd1c3f9c50c..e69031ec2de14 100644 --- a/src/bun.js/bindings/v8/V8Object.cpp +++ b/src/bun.js/bindings/v8/V8Object.cpp @@ -1,9 +1,11 @@ #include "V8Object.h" #include "shim/InternalFieldObject.h" #include "V8HandleScope.h" - #include "JavaScriptCore/ConstructData.h" #include "JavaScriptCore/ObjectConstructor.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Object) using JSC::Identifier; using JSC::JSFinalObject; diff --git a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp index ce784c22cc878..3eb5b5c7a5aa2 100644 --- a/src/bun.js/bindings/v8/V8ObjectTemplate.cpp +++ b/src/bun.js/bindings/v8/V8ObjectTemplate.cpp @@ -2,6 +2,9 @@ #include "shim/InternalFieldObject.h" #include "shim/GlobalInternals.h" #include "V8HandleScope.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Object) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8String.cpp b/src/bun.js/bindings/v8/V8String.cpp index 7dba41a93faa3..fc792cc7e4388 100644 --- a/src/bun.js/bindings/v8/V8String.cpp +++ b/src/bun.js/bindings/v8/V8String.cpp @@ -1,7 +1,18 @@ #include "V8String.h" - #include "V8HandleScope.h" #include "wtf/SIMDUTF.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::String) + +ASSERT_V8_ENUM_MATCHES(NewStringType, kNormal) +ASSERT_V8_ENUM_MATCHES(NewStringType, kInternalized) + +ASSERT_V8_ENUM_MATCHES(String::WriteOptions, NO_OPTIONS) +ASSERT_V8_ENUM_MATCHES(String::WriteOptions, HINT_MANY_WRITES_EXPECTED) +ASSERT_V8_ENUM_MATCHES(String::WriteOptions, NO_NULL_TERMINATION) +ASSERT_V8_ENUM_MATCHES(String::WriteOptions, PRESERVE_ONE_BYTE_NULL) +ASSERT_V8_ENUM_MATCHES(String::WriteOptions, REPLACE_INVALID_UTF8) using JSC::JSString; diff --git a/src/bun.js/bindings/v8/V8Template.cpp b/src/bun.js/bindings/v8/V8Template.cpp index b5c95a1e284ef..3d269fdfa5272 100644 --- a/src/bun.js/bindings/v8/V8Template.cpp +++ b/src/bun.js/bindings/v8/V8Template.cpp @@ -1,4 +1,7 @@ #include "V8Template.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Template) namespace v8 { diff --git a/src/bun.js/bindings/v8/V8Value.cpp b/src/bun.js/bindings/v8/V8Value.cpp index 0077bc8d0a814..68499be9b51ee 100644 --- a/src/bun.js/bindings/v8/V8Value.cpp +++ b/src/bun.js/bindings/v8/V8Value.cpp @@ -1,5 +1,8 @@ #include "V8Value.h" #include "V8Isolate.h" +#include "v8_compatibility_assertions.h" + +ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::Value) namespace v8 { diff --git a/src/bun.js/bindings/v8/node.cpp b/src/bun.js/bindings/v8/node.cpp index a4ef550efa2ce..f9e8f84f8de93 100644 --- a/src/bun.js/bindings/v8/node.cpp +++ b/src/bun.js/bindings/v8/node.cpp @@ -3,7 +3,11 @@ #include "JavaScriptCore/ObjectConstructor.h" #include "CommonJSModuleRecord.h" -#include + +#include "node/node_version.h" + +static_assert(REPORTED_NODEJS_ABI_VERSION == NODE_MODULE_VERSION, + "Bun's Node.js ABI version is not the same as in the reported version of Node.js"); using v8::Context; using v8::HandleScope; diff --git a/src/bun.js/bindings/v8/real_v8.h b/src/bun.js/bindings/v8/real_v8.h new file mode 100644 index 0000000000000..a052929388180 --- /dev/null +++ b/src/bun.js/bindings/v8/real_v8.h @@ -0,0 +1,7 @@ +#pragma once + +#define v8 real_v8 +#define private public +#include "node/v8.h" +#undef private +#undef v8 diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp index 14ce908afe3ff..e46a552d59d10 100644 --- a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp @@ -86,12 +86,11 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb ImplicitArgs implicit_args = { .holder = nullptr, .isolate = isolate, - // set to nullptr to catch any case where this is actually used - .context = nullptr, + .unused = nullptr, .return_value = TaggedPointer(), // data may be an object // put it in the handle scope so that it has a map ptr - .target = data.tagged(), + .data = data.tagged(), .new_target = nullptr, }; diff --git a/src/bun.js/bindings/v8/shim/Handle.cpp b/src/bun.js/bindings/v8/shim/Handle.cpp new file mode 100644 index 0000000000000..d7b92cec04ba8 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/Handle.cpp @@ -0,0 +1,53 @@ +#include "Handle.h" +#include "real_v8.h" + +static_assert(offsetof(v8::shim::ObjectLayout, m_taggedMap) == real_v8::internal::Internals::kHeapObjectMapOffset, + "ObjectLayout map pointer is at the wrong offset"); +static_assert(offsetof(v8::shim::Handle, m_toV8Object) == 0, + "Handle object pointer is at wrong offset"); + +namespace v8 { +namespace shim { + +Handle::Handle(const Map* map, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner) + : m_toV8Object(&this->m_object) + , m_object(map, cell, vm, owner) +{ +} + +Handle::Handle(double number) + : m_toV8Object(&this->m_object) + , m_object(number) +{ +} + +Handle::Handle(int32_t smi) + : m_toV8Object(smi) + , m_object() +{ +} + +Handle::Handle(const Handle& that) +{ + *this = that; +} + +Handle::Handle(const ObjectLayout* that) + : m_toV8Object(&this->m_object) +{ + m_object = *that; +} + +Handle& Handle::operator=(const Handle& that) +{ + m_object = that.m_object; + if (that.m_toV8Object.type() == TaggedPointer::Type::Smi) { + m_toV8Object = that.m_toV8Object; + } else { + m_toV8Object = &this->m_object; + } + return *this; +} + +} // namespace shim +} // namespace v8 diff --git a/src/bun.js/bindings/v8/shim/Handle.h b/src/bun.js/bindings/v8/shim/Handle.h index d73d3a6ef421c..eb15f14917bba 100644 --- a/src/bun.js/bindings/v8/shim/Handle.h +++ b/src/bun.js/bindings/v8/shim/Handle.h @@ -1,21 +1,19 @@ #pragma once -#include "Map.h" #include +#include "Map.h" namespace v8 { namespace shim { struct ObjectLayout { -private: - // these two fields are laid out so that V8 can find the map + // this field must be at the start so that V8 can find the map TaggedPointer m_taggedMap; union { JSC::WriteBarrier cell; double number; } m_contents; -public: ObjectLayout() // using a smi value for map is most likely to catch bugs as almost every access will expect // map to be a pointer (and even if the assertion is bypassed, it'll be a null pointer) @@ -41,9 +39,6 @@ struct ObjectLayout { double asDouble() const { return m_contents.number; } JSC::JSCell* asCell() const { return m_contents.cell.get(); } - - friend class Handle; - friend class HandleScopeBuffer; }; // A handle stored in a HandleScope with layout suitable for V8's inlined functions: @@ -60,49 +55,15 @@ struct ObjectLayout { // the third field to get the actual object (either a JSCell* or a void*, depending on whether map // points to Map::object_map or Map::raw_ptr_map). struct Handle { - static_assert(offsetof(ObjectLayout, m_taggedMap) == 0, "ObjectLayout is wrong"); - static_assert(offsetof(ObjectLayout, m_contents) == 8, "ObjectLayout is wrong"); - static_assert(sizeof(ObjectLayout) == 16, "ObjectLayout is wrong"); + Handle(const Map* map, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner); - Handle(const Map* map, JSC::JSCell* cell, JSC::VM& vm, const JSC::JSCell* owner) - : m_toV8Object(&this->m_object) - , m_object(map, cell, vm, owner) - { - } + Handle(double number); - Handle(double number) - : m_toV8Object(&this->m_object) - , m_object(number) - { - } + Handle(int32_t smi); - Handle(int32_t smi) - : m_toV8Object(smi) - , m_object() - { - } + Handle(const Handle& that); - Handle(const Handle& that) - { - *this = that; - } - - Handle(const ObjectLayout* that) - : m_toV8Object(&this->m_object) - { - m_object = *that; - } - - Handle& operator=(const Handle& that) - { - m_object = that.m_object; - if (that.m_toV8Object.type() == TaggedPointer::Type::Smi) { - m_toV8Object = that.m_toV8Object; - } else { - m_toV8Object = &this->m_object; - } - return *this; - } + Handle(const ObjectLayout* that); Handle() : m_toV8Object(0) @@ -110,6 +71,8 @@ struct Handle { { } + Handle& operator=(const Handle& that); + bool isCell() const { if (m_toV8Object.type() == TaggedPointer::Type::Smi) { @@ -138,7 +101,6 @@ struct Handle { return m_object.m_contents.cell; } -private: // if not SMI, holds &this->map so that V8 can see what kind of object this is TaggedPointer m_toV8Object; ObjectLayout m_object; diff --git a/src/bun.js/bindings/v8/shim/Map.cpp b/src/bun.js/bindings/v8/shim/Map.cpp index 53d1f363e01fb..0502ac6b717bd 100644 --- a/src/bun.js/bindings/v8/shim/Map.cpp +++ b/src/bun.js/bindings/v8/shim/Map.cpp @@ -1,4 +1,23 @@ #include "Map.h" +#include "real_v8.h" + +static_assert(offsetof(v8::shim::Map, m_metaMap) == real_v8::internal::Internals::kHeapObjectMapOffset, + "v8::Map map pointer is at wrong offset"); +static_assert(offsetof(v8::shim::Map, m_instanceType) == real_v8::internal::Internals::kMapInstanceTypeOffset, + "v8::Map instance type is at wrong offset"); + +static_assert((int)v8::shim::InstanceType::String < real_v8::internal::Internals::kFirstNonstringType, + "String instance type is not a string"); +static_assert((int)v8::shim::InstanceType::Oddball == real_v8::internal::Internals::kOddballType, + "Oddball instance type does not match V8"); +static_assert((int)v8::shim::InstanceType::Object >= real_v8::internal::Internals::kFirstNonstringType, + "Objects are strings"); +static_assert((int)v8::shim::InstanceType::HeapNumber >= real_v8::internal::Internals::kFirstNonstringType, + "HeapNumbers are strings"); + +static_assert(real_v8::internal::Internals::CanHaveInternalField((int)v8::shim::InstanceType::Object) == false, + "Object instance type appears compatible with internal fields" + "(so V8 will use direct pointer offsets instead of calling the slow path)"); namespace v8 { namespace shim { diff --git a/src/bun.js/bindings/v8/shim/Oddball.cpp b/src/bun.js/bindings/v8/shim/Oddball.cpp new file mode 100644 index 0000000000000..55e50512aff74 --- /dev/null +++ b/src/bun.js/bindings/v8/shim/Oddball.cpp @@ -0,0 +1,18 @@ +#include "Oddball.h" +#include "real_v8.h" + +#define CHECK_ODDBALL_KIND(KIND) \ + static_assert((int)v8::shim::Oddball::Kind::KIND == real_v8::internal::Internals::KIND##OddballKind, \ + "Oddball kind " #KIND " does not match V8"); + +// true and false are unchecked, as those are only defined by class v8::internal::Oddball in +// src/objects/oddball.h which is not included in the API headers. I haven't seen a case where an +// inline function relies on those values. For now, we intentionally *don't* match V8's kind values +// for true and false so that an error will be apparent if V8 ever does rely on them. +CHECK_ODDBALL_KIND(kNull) +CHECK_ODDBALL_KIND(kUndefined) + +static_assert(offsetof(v8::shim::Oddball, m_map) == real_v8::internal::Internals::kHeapObjectMapOffset, + "Oddball map field is at wrong offset"); + +static_assert(offsetof(v8::shim::Oddball, m_kind) == real_v8::internal::Internals::kOddballKindOffset); diff --git a/src/bun.js/bindings/v8/shim/Oddball.h b/src/bun.js/bindings/v8/shim/Oddball.h index 5f2dfbf51937f..e445e5ead8066 100644 --- a/src/bun.js/bindings/v8/shim/Oddball.h +++ b/src/bun.js/bindings/v8/shim/Oddball.h @@ -12,8 +12,8 @@ struct Oddball { kUndefined = 4, kNull = 3, kInvalid = 255, - kTrue = 1, - kFalse = 0, + kTrue = 99, + kFalse = 98, }; TaggedPointer m_map; diff --git a/src/bun.js/bindings/v8/v8_compatibility_assertions.h b/src/bun.js/bindings/v8/v8_compatibility_assertions.h new file mode 100644 index 0000000000000..2f10791493416 --- /dev/null +++ b/src/bun.js/bindings/v8/v8_compatibility_assertions.h @@ -0,0 +1,50 @@ +#pragma once + +#include "real_v8.h" + +#define V8_TYPE_ASSERTIONS_NAMESPACE_NAME_INDIRECT(LINE) V8TypeAssertions__##LINE + +#define V8_TYPE_ASSERTIONS_NAMESPACE_NAME(LINE) \ + V8_TYPE_ASSERTIONS_NAMESPACE_NAME_INDIRECT(LINE) + +// usage: [*outside* namespace v8] ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::SomeType) +#define ASSERT_V8_TYPE_LAYOUT_MATCHES(TYPENAME) \ + namespace V8_TYPE_ASSERTIONS_NAMESPACE_NAME(__LINE__) { \ + namespace DeclareBunType { \ + namespace v8 = ::v8; \ + using BunType = TYPENAME; \ + } \ + \ + namespace DeclareV8Type { \ + namespace v8 = ::real_v8; \ + using V8Type = TYPENAME; \ + } \ + \ + static_assert(sizeof(DeclareBunType::BunType) == sizeof(DeclareV8Type::V8Type), \ + "size of " #TYPENAME " does not match between Bun and V8"); \ + static_assert(alignof(DeclareBunType::BunType) == alignof(DeclareV8Type::V8Type), \ + "alignment of " #TYPENAME " does not match between Bun and V8"); \ + } + +// usage: [*outside* namespace v8] ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(v8::Maybe, m_hasValue, has_value_) +#define ASSERT_V8_TYPE_FIELD_OFFSET_MATCHES(TYPENAME, BUN_FIELD_NAME, V8_FIELD_NAME) \ + namespace V8_TYPE_ASSERTIONS_NAMESPACE_NAME(__LINE__) { \ + namespace DeclareBunType { \ + namespace v8 = ::v8; \ + using BunType = TYPENAME; \ + } \ + \ + namespace DeclareV8Type { \ + namespace v8 = ::real_v8; \ + using V8Type = TYPENAME; \ + } \ + \ + static_assert(offsetof(DeclareBunType::BunType, BUN_FIELD_NAME) \ + == offsetof(DeclareV8Type::V8Type, V8_FIELD_NAME), \ + "offset of " #TYPENAME "::" #BUN_FIELD_NAME " does not match between Bun and V8"); \ + } + +// usage: [*outside* namespace v8] ASSERT_V8_ENUM_MATCHES(ConstructorBehavior, kAllow) +#define ASSERT_V8_ENUM_MATCHES(ENUM_NAME, ENUMERATOR_NAME) \ + static_assert((int)::v8::ENUM_NAME::ENUMERATOR_NAME == (int)::real_v8::ENUM_NAME::ENUMERATOR_NAME, \ + "enumerator " #ENUM_NAME "::" #ENUMERATOR_NAME " does not match between Bun and V8"); diff --git a/src/bun.js/bindings/v8/v8config.h b/src/bun.js/bindings/v8/v8config.h new file mode 100644 index 0000000000000..f32a4d66359fd --- /dev/null +++ b/src/bun.js/bindings/v8/v8config.h @@ -0,0 +1,3 @@ +// This only exists so that files from the real V8 implementation can include it, without us having +// to add vendor/nodejs/include/node to the include path (as that would conflict with our V8 functions) +#include "node/v8config.h" From 216417624d84713d9633d530ae0f1b8a9b92324d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 26 Sep 2024 09:58:01 -0700 Subject: [PATCH 28/30] Add comments to real_v8.h and v8_compatibility_assertions.h --- src/bun.js/bindings/v8/real_v8.h | 5 +++++ src/bun.js/bindings/v8/v8_compatibility_assertions.h | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/v8/real_v8.h b/src/bun.js/bindings/v8/real_v8.h index a052929388180..14618941bcb69 100644 --- a/src/bun.js/bindings/v8/real_v8.h +++ b/src/bun.js/bindings/v8/real_v8.h @@ -1,5 +1,10 @@ #pragma once +// This file includes the actual V8 headers, with the namespace renamed from v8 to real_v8. To +// minimize potential conflicts between V8 and Bun's implementation, it should only be included by +// files from Bun's V8 implementation, and it should only be included in source files (not +// header files). + #define v8 real_v8 #define private public #include "node/v8.h" diff --git a/src/bun.js/bindings/v8/v8_compatibility_assertions.h b/src/bun.js/bindings/v8/v8_compatibility_assertions.h index 2f10791493416..0abfff025d12f 100644 --- a/src/bun.js/bindings/v8/v8_compatibility_assertions.h +++ b/src/bun.js/bindings/v8/v8_compatibility_assertions.h @@ -1,5 +1,9 @@ #pragma once +// This file defines macros to check compatibility between types from V8 and types from Bun's V8 +// implementation. The same warning as in real_v8.h applies: only include this in source files in +// the v8 directory. + #include "real_v8.h" #define V8_TYPE_ASSERTIONS_NAMESPACE_NAME_INDIRECT(LINE) V8TypeAssertions__##LINE @@ -7,7 +11,7 @@ #define V8_TYPE_ASSERTIONS_NAMESPACE_NAME(LINE) \ V8_TYPE_ASSERTIONS_NAMESPACE_NAME_INDIRECT(LINE) -// usage: [*outside* namespace v8] ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::SomeType) +// usage: [*outside* namespace v8] ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::SomeTemplate) #define ASSERT_V8_TYPE_LAYOUT_MATCHES(TYPENAME) \ namespace V8_TYPE_ASSERTIONS_NAMESPACE_NAME(__LINE__) { \ namespace DeclareBunType { \ From fa1e6710bdb8705d41c6dd7675655cb78b2c7f50 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 26 Sep 2024 10:38:37 -0700 Subject: [PATCH 29/30] Try to please Microsoft --- src/bun.js/bindings/v8/real_v8.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/bun.js/bindings/v8/real_v8.h b/src/bun.js/bindings/v8/real_v8.h index 14618941bcb69..b8d139c00343c 100644 --- a/src/bun.js/bindings/v8/real_v8.h +++ b/src/bun.js/bindings/v8/real_v8.h @@ -5,6 +5,27 @@ // files from Bun's V8 implementation, and it should only be included in source files (not // header files). +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + #define v8 real_v8 #define private public #include "node/v8.h" From 12305f501e7b050a4d2b29b5f3e7b6903e0a3835 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 26 Sep 2024 16:07:32 -0700 Subject: [PATCH 30/30] TaggedPointer assertions and refactoring --- src/bun.js/bindings/v8/V8Data.h | 6 +-- .../v8/V8EscapableHandleScopeBase.cpp | 2 +- src/bun.js/bindings/v8/V8HandleScope.cpp | 2 +- src/bun.js/bindings/v8/real_v8.h | 5 ++ .../bindings/v8/shim/FunctionTemplate.cpp | 2 +- src/bun.js/bindings/v8/shim/Handle.cpp | 2 +- src/bun.js/bindings/v8/shim/Handle.h | 2 +- src/bun.js/bindings/v8/shim/TaggedPointer.cpp | 8 +++ src/bun.js/bindings/v8/shim/TaggedPointer.h | 49 ++++++++++--------- src/bun.js/bindings/v8/v8_api_internal.cpp | 2 +- 10 files changed, 47 insertions(+), 33 deletions(-) create mode 100644 src/bun.js/bindings/v8/shim/TaggedPointer.cpp diff --git a/src/bun.js/bindings/v8/V8Data.h b/src/bun.js/bindings/v8/V8Data.h index 9295db2093701..f3ebc81aafe9e 100644 --- a/src/bun.js/bindings/v8/V8Data.h +++ b/src/bun.js/bindings/v8/V8Data.h @@ -18,7 +18,7 @@ class Data { JSC::JSCell* localToCell() { TaggedPointer root = localToTagged(); - RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi); + RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi); return root.getPtr()->asCell(); } @@ -44,7 +44,7 @@ class Data { JSC::JSValue localToJSValue() const { TaggedPointer root = localToTagged(); - if (root.type() == TaggedPointer::Type::Smi) { + if (root.tag() == TaggedPointer::Tag::Smi) { return JSC::jsNumber(root.getSmiUnchecked()); } else { using shim::InstanceType; @@ -65,7 +65,7 @@ class Data { const JSC::JSCell* localToCell() const { TaggedPointer root = localToTagged(); - RELEASE_ASSERT(root.type() != TaggedPointer::Type::Smi); + RELEASE_ASSERT(root.tag() != TaggedPointer::Tag::Smi); return root.getPtr()->asCell(); } diff --git a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp index 1aa8e117b6079..e9c3c01968801 100644 --- a/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp +++ b/src/bun.js/bindings/v8/V8EscapableHandleScopeBase.cpp @@ -24,7 +24,7 @@ uintptr_t* EscapableHandleScopeBase::EscapeSlot(uintptr_t* escape_value) m_isolate, m_escapeSlot); m_escapeSlot = nullptr; - return newHandle->asRawPtr(); + return newHandle->asRawPtrLocation(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/V8HandleScope.cpp b/src/bun.js/bindings/v8/V8HandleScope.cpp index 10053311afd68..9071d9621c5c5 100644 --- a/src/bun.js/bindings/v8/V8HandleScope.cpp +++ b/src/bun.js/bindings/v8/V8HandleScope.cpp @@ -32,7 +32,7 @@ uintptr_t* HandleScope::CreateHandle(internal::Isolate* i_isolate, uintptr_t val auto* handleScope = isolate->globalInternals()->currentHandleScope(); TaggedPointer* newSlot = handleScope->m_buffer->createHandleFromExistingObject(TaggedPointer::fromRaw(value), isolate); // basically a reinterpret - return newSlot->asRawPtr(); + return newSlot->asRawPtrLocation(); } } // namespace v8 diff --git a/src/bun.js/bindings/v8/real_v8.h b/src/bun.js/bindings/v8/real_v8.h index b8d139c00343c..dec9fa2b1a97a 100644 --- a/src/bun.js/bindings/v8/real_v8.h +++ b/src/bun.js/bindings/v8/real_v8.h @@ -5,6 +5,11 @@ // files from Bun's V8 implementation, and it should only be included in source files (not // header files). +// Microsoft's C++ headers cause a compiler error if `private` has been redefined, like we do below. +// These are all the standard library headers included by V8. We include them here so that they +// are included while `private` is not redefined yet, and then when V8 includes them they will get +// skipped by include guards. + #include #include #include diff --git a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp index e46a552d59d10..db04da9fc27aa 100644 --- a/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp +++ b/src/bun.js/bindings/v8/shim/FunctionTemplate.cpp @@ -98,7 +98,7 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb functionTemplate->m_callback(info); - if (implicit_args.return_value.type() != TaggedPointer::Type::Smi && implicit_args.return_value.getPtr() == nullptr) { + if (implicit_args.return_value.isEmpty()) { // callback forgot to set a return value, so return undefined return JSValue::encode(JSC::jsUndefined()); } else { diff --git a/src/bun.js/bindings/v8/shim/Handle.cpp b/src/bun.js/bindings/v8/shim/Handle.cpp index d7b92cec04ba8..3aa33c7880c7e 100644 --- a/src/bun.js/bindings/v8/shim/Handle.cpp +++ b/src/bun.js/bindings/v8/shim/Handle.cpp @@ -41,7 +41,7 @@ Handle::Handle(const ObjectLayout* that) Handle& Handle::operator=(const Handle& that) { m_object = that.m_object; - if (that.m_toV8Object.type() == TaggedPointer::Type::Smi) { + if (that.m_toV8Object.tag() == TaggedPointer::Tag::Smi) { m_toV8Object = that.m_toV8Object; } else { m_toV8Object = &this->m_object; diff --git a/src/bun.js/bindings/v8/shim/Handle.h b/src/bun.js/bindings/v8/shim/Handle.h index eb15f14917bba..bd87f417f0dff 100644 --- a/src/bun.js/bindings/v8/shim/Handle.h +++ b/src/bun.js/bindings/v8/shim/Handle.h @@ -75,7 +75,7 @@ struct Handle { bool isCell() const { - if (m_toV8Object.type() == TaggedPointer::Type::Smi) { + if (m_toV8Object.tag() == TaggedPointer::Tag::Smi) { return false; } const Map* map_ptr = m_object.map(); diff --git a/src/bun.js/bindings/v8/shim/TaggedPointer.cpp b/src/bun.js/bindings/v8/shim/TaggedPointer.cpp new file mode 100644 index 0000000000000..eb0e786899cda --- /dev/null +++ b/src/bun.js/bindings/v8/shim/TaggedPointer.cpp @@ -0,0 +1,8 @@ +#include "TaggedPointer.h" +#include "../real_v8.h" + +static_assert(sizeof(v8::shim::TaggedPointer) == sizeof(real_v8::internal::Address), + "TaggedPointer has wrong size"); + +static_assert(alignof(v8::shim::TaggedPointer) == alignof(real_v8::internal::Address), + "TaggedPointer has wrong alignment"); diff --git a/src/bun.js/bindings/v8/shim/TaggedPointer.h b/src/bun.js/bindings/v8/shim/TaggedPointer.h index 46ea98701386c..a11cac1bd784d 100644 --- a/src/bun.js/bindings/v8/shim/TaggedPointer.h +++ b/src/bun.js/bindings/v8/shim/TaggedPointer.h @@ -15,12 +15,15 @@ struct TaggedPointer { uintptr_t m_value; public: - enum class Type : uint8_t { - Smi, - StrongPointer, - WeakPointer, + enum class Tag : uint8_t { + Smi = 0, + StrongPointer = 1, + WeakPointer = 3, }; + static constexpr uintptr_t TagMask = 0b11; + + // empty TaggedPointer() : TaggedPointer(nullptr) {}; TaggedPointer(const TaggedPointer&) = default; @@ -28,13 +31,14 @@ struct TaggedPointer { bool operator==(const TaggedPointer& other) const { return m_value == other.m_value; } TaggedPointer(void* ptr, bool weak = false) - : m_value(reinterpret_cast(ptr) | (weak ? 3 : 1)) + : m_value(reinterpret_cast(ptr) | static_cast(weak ? Tag::WeakPointer : Tag::StrongPointer)) { - RELEASE_ASSERT((reinterpret_cast(ptr) & 3) == 0); + // check original pointer was aligned + RELEASE_ASSERT((reinterpret_cast(ptr) & TagMask) == 0); } TaggedPointer(int32_t smi) - : m_value(static_cast(smi) << 32) + : m_value((static_cast(smi) << 32) | static_cast(Tag::Smi)) { } @@ -46,37 +50,34 @@ struct TaggedPointer { return tagged; } - // Get a pointer to where this TaggedPointer is located - uintptr_t* asRawPtr() + bool isEmpty() const + { + return *this == TaggedPointer(); + } + + // Get a pointer to where this TaggedPointer is located (use ->asRawPtrLocation() to reinterpret + // TaggedPointer* as uintptr_t*) + uintptr_t* asRawPtrLocation() { return &m_value; } - Type type() const + Tag tag() const { - switch (m_value & 3) { - case 0: - return Type::Smi; - case 1: - return Type::StrongPointer; - case 3: - return Type::WeakPointer; - default: - RELEASE_ASSERT_NOT_REACHED(); - } + return static_cast(m_value & TagMask); } template T* getPtr() const { - if (type() == Type::Smi) { + if (tag() == Tag::Smi) { return nullptr; } - return reinterpret_cast(m_value & ~3ull); + return reinterpret_cast(m_value & ~TagMask); } bool getSmi(int32_t& smi) const { - if (type() != Type::Smi) { + if (tag() != Tag::Smi) { return false; } smi = static_cast(m_value >> 32); @@ -85,7 +86,7 @@ struct TaggedPointer { int32_t getSmiUnchecked() const { - ASSERT(type() == Type::Smi); + ASSERT(tag() == Tag::Smi); return static_cast(m_value >> 32); } }; diff --git a/src/bun.js/bindings/v8/v8_api_internal.cpp b/src/bun.js/bindings/v8/v8_api_internal.cpp index 2153b11f3c201..860129c777a0b 100644 --- a/src/bun.js/bindings/v8/v8_api_internal.cpp +++ b/src/bun.js/bindings/v8/v8_api_internal.cpp @@ -17,7 +17,7 @@ uintptr_t* GlobalizeReference(internal::Isolate* i_isolate, uintptr_t address) auto* isolate = reinterpret_cast(i_isolate); auto* globalHandles = isolate->globalInternals()->globalHandles(); TaggedPointer* newSlot = globalHandles->createHandleFromExistingObject(TaggedPointer::fromRaw(address), isolate); - return newSlot->asRawPtr(); + return newSlot->asRawPtrLocation(); } void DisposeGlobal(uintptr_t* location)