Skip to content

Rust JSG: static member named name/length/prototype aborts in FunctionTemplate instantiation #7967

Open

Description

@piscisaureus

A Rust JSG resource with a static member named name, length or
prototype aborts the process when its constructor is first
instantiated. The abort happens when the member is a static method (a
#[jsg_method] with no receiver) and also when it is a
#[jsg_static_constant]. There is no compile error and no JS exception.
The process dies with SIGTRAP (exit code 133) and prints nothing, because
V8's release CHECK has no message. Under lldb:

stop reason = EXC_BREAKPOINT
frame #0:  v8::internal::DescriptorArray::Append(v8::internal::Descriptor*) (.cold.2)
frame #2:  v8::internal::Map::CopyAddDescriptor(...)
frame #3:  v8::internal::Map::CopyWithField(...)
frame #4:  v8::internal::Map::TransitionToDataProperty(...)
frame #6:  v8::internal::Object::TransitionAndWriteDataProperty(...)
frame #7:  v8::internal::(anonymous namespace)::DefineDataProperty(...)
frame #8:  v8::internal::(anonymous namespace)::InstantiateFunction(...)
frame #10: v8::FunctionTemplate::GetFunction(v8::Local<v8::Context>)
frame #11: workerd::rust::jsg::function_template_get_function(...)
frame #17: jsg::resource::function_template_of::<...>

Minimal repro (in src/rust/jsg-test/tests/):

#[jsg_resource]
struct Foo;

#[jsg_resource]
impl Foo {
    #[jsg_method]
    pub fn name() -> String {
        "static".to_owned()
    }
}

#[test]
fn static_method_named_name() {
    let harness = crate::Harness::new();
    harness.run_in_context(|lock, ctx| {
        // aborts here
        let ctor = jsg::resource::function_template_of::<Foo>(lock);
        ctx.set_global(lock, "Foo", ctor);
        Ok(())
    });
}

The same thing happens with #[jsg_method(name = "length")],
#[jsg_method(name = "prototype")] and
#[jsg_static_constant(name = "name" | "length" | "prototype")]. Two
static members with the same JS name also abort the same way, for example
a static method and a static constant that are both named dup.

These cases work fine:

  • instance methods named name, length or prototype, because they go
    on the prototype template
  • static methods named arguments or caller
  • static methods with any other name

On origin/main (67e852e, V8 15.7.112.5), create_resource_template in
src/rust/jsg/ffi.c++ installs static methods (line 1114) and static
constants (line 1233) with constructor->Set(name, ...) on the
constructor FunctionTemplate. When the template is instantiated
(function_template_get_function, line 1245, which calls
FunctionTemplate::GetFunction), V8 starts from a function map that
already has length, name and prototype descriptors (AccessorInfo).
It then adds each template property with DefineDataProperty
(src/api/api-natives.cc:102). That function only looks for an existing
property under #ifdef DEBUG, where it throws "Object template has
duplicate property '%'". In release builds it goes straight to
Object::AddDataProperty, and the name-collision
CHECK_EQ(LinearSearch(...), NotFound()) in DescriptorArray::Append
(src/objects/descriptor-array-inl.h:346) fails. Duplicate template keys
fail the same check.

C++ JSG does the same thing and has no guard either.
registerStaticMethod calls constructor->Set(v8StrIntern(isolate, name), functionTemplate) (src/workerd/jsg/resource.h:1458 and :1471), and
registerStaticConstant calls constructor->Set(v8Name, v8Value, attrs)
(resource.h:1667). So JSG_STATIC_METHOD_NAMED(name, ...),
JSG_STATIC_METHOD(length) and similar would hit the same CHECK. No
existing C++ API uses these names, which is probably why nobody has run
into it. On the Rust side the name comes from the snake_case-to-camelCase
conversion of the function name, so a natural fn name() or fn length()
on a resource is enough to trigger it.

Suggested fix: reject these names when the macro expands. In
collect_method_registrations / collect_constant_registrations
(src/rust/jsg-macros/resource.rs), emit a compile_error! when a static
member's JS name is name, length or prototype. Also reject duplicate
JS names among the static members of an impl block. Static members come
from a runtime members(flags) list, so a debug assertion (or a
KJ_REQUIRE in create_resource_template) that checks the static names
against the reserved set and against each other would also catch
duplicates the macro can't see. Either way, the failure becomes a clear
error instead of a silent SIGTRAP. Adding the same check to C++
registerStaticMethod / registerStaticConstant would be cheap.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    jsgrustPull requests that update rust code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions