From 19031b25ce3daeece23917e56ed91b9486ed37d0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 27 Mar 2025 10:44:03 +0800 Subject: [PATCH 1/3] Add RemoteObject subtype Allow Zig structures to define a `sub_type` which will be included in the RemoteObject's subtype field (and will generate an empty description field) External PersitedObjects now wrap our own ExternalEntry wrapper, which includes a pointer to the Zig instance (what we used to have before). This approach allows us to associate metadata with a v8.Value. We currently add the subtype if available. --- build.zig | 2 +- src/engines/v8/generate.zig | 30 ++++++++++----------- src/engines/v8/v8.zig | 52 +++++++++++++++++++++++++++++++++++++ src/reflect.zig | 7 +++++ src/refs.zig | 9 +++---- 5 files changed, 78 insertions(+), 22 deletions(-) diff --git a/build.zig b/build.zig index cc23936..876425c 100644 --- a/build.zig +++ b/build.zig @@ -182,7 +182,7 @@ pub fn packages(comptime vendor_path: []const u8) type { mod.addIncludePath(b.path(vendor ++ "/zig-v8/src")); const build_opts = b.addOptions(); - build_opts.addOption(bool, "inspector_subtype", true); + build_opts.addOption(bool, "inspector_subtype", false); mod.addOptions("default_exports", build_opts); return mod; } diff --git a/src/engines/v8/generate.zig b/src/engines/v8/generate.zig index ea41b30..062dee4 100644 --- a/src/engines/v8/generate.zig +++ b/src/engines/v8/generate.zig @@ -34,6 +34,7 @@ const jsToObject = @import("types_primitives.zig").jsToObject; const TPL = @import("v8.zig").TPL; const JSObject = @import("v8.zig").JSObject; const JSObjectID = @import("v8.zig").JSObjectID; +const ExternalEntry = @import("v8.zig").ExternalEntry; // Utils functions // --------------- @@ -435,20 +436,17 @@ fn bindObjectNativeToJS( const pers = PersistentObject.init(isolate, js_obj); const js_obj_pers = pers.castToObject(); - // bind the native object pointer to the JS object - var ext: v8.External = undefined; - if (comptime T_refl.is_mem_guarantied()) { - - // store directly the object pointer - ext = v8.External.init(isolate, nat_obj); - } else { + const entry = try alloc.create(ExternalEntry); + entry.* = .{ + .ptr = nat_obj, + .sub_type = T_refl.sub_type, + }; - // use the refs mechanism - const int_ptr = try alloc.create(usize); - int_ptr.* = @intFromPtr(nat_obj); - ext = v8.External.init(isolate, int_ptr); - try nat_ctx.nat_objs.put(alloc, int_ptr.*, T_refl.index); + // bind the native object pointer to the JS object + if (comptime T_refl.is_mem_guarantied() == false) { + try nat_ctx.nat_objs.put(alloc, @intFromPtr(nat_obj), T_refl.index); } + const ext = v8.External.init(isolate, entry); js_obj_pers.setInternalField(0, ext); return js_obj_pers; } @@ -824,12 +822,14 @@ fn getNativeObject( // TODO ensure the js object corresponds to the expected native type. - const ext = js_obj.getInternalField(0).castTo(v8.External).get().?; + const external_data = js_obj.getInternalField(0).castTo(v8.External).get().?; + const external_entry: *ExternalEntry = @alignCast(@ptrCast(external_data)); + const opaque_instance_ptr = external_entry.ptr; if (comptime T_refl.is_mem_guarantied()) { // memory is fixed // ensure the pointer is aligned (no-op at runtime) // as External is a ?*anyopaque (ie. *void) with alignment 1 - const ptr: *align(@alignOf(T_refl.Self())) anyopaque = @alignCast(ext); + const ptr: *align(@alignOf(T_refl.Self())) anyopaque = @alignCast(opaque_instance_ptr); if (@hasDecl(T_refl.T, "protoCast")) { // T_refl provides a function to cast the pointer from high level Type obj_ptr = @call(.auto, @field(T_refl.T, "protoCast"), .{ptr}); @@ -841,7 +841,7 @@ fn getNativeObject( } } else { // use the refs mechanism to retrieve from high level Type - obj_ptr = try refs.getObject(nat_ctx.nat_objs, T, gen.Types, ext); + obj_ptr = try refs.getObject(nat_ctx.nat_objs, T, gen.Types, opaque_instance_ptr); } } return obj_ptr; diff --git a/src/engines/v8/v8.zig b/src/engines/v8/v8.zig index 4021698..32328d7 100644 --- a/src/engines/v8/v8.zig +++ b/src/engines/v8/v8.zig @@ -698,3 +698,55 @@ pub const Inspector = struct { return self.session.dispatchProtocolMessage(env.isolate, msg); } }; + +// When we return a Zig instance to V8, we wrap it in a v8.Object. That wrapping +// happens by: +// - Assigning our instance to a v8.External (which just holds an *anyopaque) +// - Creating a v8.PersistentObject and assigning the external to the +// PersistentObject's internalField #0 +// - Casting the v8.PersistentObject to a v8.Object +// +// Now, instead of assigning the instance directly into the v8.External we +// allocate and assign this ExternalEntry, which allows us to hold the ptr to +// the Zig instance, as well as meta data that we'll need. +pub const ExternalEntry = struct { + // Ptr to the Zig instance + ptr: *anyopaque, + + // When we're asked to describe an object via the Inspector, we _must_ include + // the proper subtype (and description) fields in the returned JSON. + // V8 will give us a Value and ask us for the subtype. Hence, we store it + // here. + sub_type: ?[:0]const u8, +}; + +// See above for documentation for the ExternalEntry's sub_type field. +pub export fn v8_inspector__Client__IMPL__valueSubtype( + _: *v8.c.InspectorClientImpl, + c_value: *const v8.C_Value, +) callconv(.C) [*c]const u8 { + const external_entry = getExternalEntry(.{ .handle = c_value }) orelse return null; + return if (external_entry.sub_type) |st| st else null; +} + +pub export fn v8_inspector__Client__IMPL__descriptionForValueSubtype( + _: *v8.c.InspectorClientImpl, + context: *const v8.C_Context, + c_value: *const v8.C_Value, +) callconv(.C) [*c]const u8 { + _ = context; + + // We _must_ include a non-null description in order for the subtype value + // to be included. Besides that, I don't know if the value has any meaning + const external_entry = getExternalEntry(.{ .handle = c_value }) orelse return null; + return if (external_entry.sub_type == null) null else ""; +} + +fn getExternalEntry(value: v8.Value) ?*ExternalEntry { + if (value.isObject() == false) { + return null; + } + const obj = value.castTo(Object); + const external_data = obj.getInternalField(0).castTo(v8.External).get().?; + return @alignCast(@ptrCast(external_data)); +} diff --git a/src/reflect.zig b/src/reflect.zig index ddb0c64..a68912a 100644 --- a/src/reflect.zig +++ b/src/reflect.zig @@ -785,6 +785,7 @@ pub const Struct = struct { // nested types nested: []const StructNested, + sub_type: ?[:0]const u8, pub fn Self(comptime self: Struct) type { comptime { @@ -1246,6 +1247,11 @@ pub const Struct = struct { mem_guarantied = @typeInfo(T).@"struct".layout != .auto; } + var sub_type: ?[:0]const u8 = null; + if (@hasDecl(T, "sub_type")) { + sub_type = @field(T, "sub_type"); + } + // nested types var nested_nb: usize = 0; // first iteration to retrieve the number of nested structs @@ -1430,6 +1436,7 @@ pub const Struct = struct { // nested types .nested = &cnested, + .sub_type = sub_type, }; } }; diff --git a/src/refs.zig b/src/refs.zig index b4a1cb2..d71cb18 100644 --- a/src/refs.zig +++ b/src/refs.zig @@ -26,11 +26,8 @@ const refl = internal.refl; pub const Map = std.AutoHashMapUnmanaged(usize, usize); pub fn getObject(map: Map, comptime T: type, comptime types: []const refl.Struct, ptr: anytype) !*T { - - // use the object pointer (key) to retrieve the API index (value) in the map - const ptr_aligned: *align(@alignOf(usize)) anyopaque = @alignCast(ptr); - const key: *usize = @ptrCast(ptr_aligned); - const T_index = map.get(key.*); + const key: usize = @intFromPtr(ptr); + const T_index = map.get(key); if (T_index == null) { return error.NullReference; } @@ -42,7 +39,7 @@ pub fn getObject(map: Map, comptime T: type, comptime types: []const refl.Struct if (!T_refl.isEmpty()) { // stage1: condition is needed for empty structs // go through the "proto" object chain // to retrieve the good object corresponding to T - const target_ptr: *T_refl.Self() = @ptrFromInt(key.*); + const target_ptr: *T_refl.Self() = @ptrFromInt(key); return try getRealObject(T, target_ptr); } } From 8ff4406ae5859ad08b3ff92d2a17b611ba79c3c7 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 27 Mar 2025 16:41:23 +0800 Subject: [PATCH 2/3] Make sure the object we're looking at has an internalField before accessing it --- src/engines/v8/v8.zig | 4 ++++ vendor/zig-v8 | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/engines/v8/v8.zig b/src/engines/v8/v8.zig index 32328d7..f52413d 100644 --- a/src/engines/v8/v8.zig +++ b/src/engines/v8/v8.zig @@ -747,6 +747,10 @@ fn getExternalEntry(value: v8.Value) ?*ExternalEntry { return null; } const obj = value.castTo(Object); + if (obj.internalFieldCount() == 0) { + return null; + } + const external_data = obj.getInternalField(0).castTo(v8.External).get().?; return @alignCast(@ptrCast(external_data)); } diff --git a/vendor/zig-v8 b/vendor/zig-v8 index 3cb2368..c9c1ba1 160000 --- a/vendor/zig-v8 +++ b/vendor/zig-v8 @@ -1 +1 @@ -Subproject commit 3cb236820d18da6234b08606de4d4a6e7ccba83b +Subproject commit c9c1ba161c9a6db00aad789fc6fe70cdb2d6799c From c65f7ddb067c6cda464da317120c44e5de36e9d0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sun, 30 Mar 2025 15:23:19 +0800 Subject: [PATCH 3/3] Store sub_type as a [*c]const u8 Makes the @sizeOf(ExternalEntry) 16, instead of 24 and works just as well with the string literal we expect it to be set to. --- src/engines/v8/v8.zig | 2 +- src/reflect.zig | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/engines/v8/v8.zig b/src/engines/v8/v8.zig index f52413d..b41b38b 100644 --- a/src/engines/v8/v8.zig +++ b/src/engines/v8/v8.zig @@ -717,7 +717,7 @@ pub const ExternalEntry = struct { // the proper subtype (and description) fields in the returned JSON. // V8 will give us a Value and ask us for the subtype. Hence, we store it // here. - sub_type: ?[:0]const u8, + sub_type: ?[*c]const u8, }; // See above for documentation for the ExternalEntry's sub_type field. diff --git a/src/reflect.zig b/src/reflect.zig index a68912a..e6bbaee 100644 --- a/src/reflect.zig +++ b/src/reflect.zig @@ -785,7 +785,7 @@ pub const Struct = struct { // nested types nested: []const StructNested, - sub_type: ?[:0]const u8, + sub_type: ?[*c]const u8, pub fn Self(comptime self: Struct) type { comptime { @@ -1247,7 +1247,7 @@ pub const Struct = struct { mem_guarantied = @typeInfo(T).@"struct".layout != .auto; } - var sub_type: ?[:0]const u8 = null; + var sub_type: ?[*c]const u8 = null; if (@hasDecl(T, "sub_type")) { sub_type = @field(T, "sub_type"); }