Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempting to use _input will crash Godot #189

Closed
lilizoey opened this issue Mar 20, 2023 · 5 comments · Fixed by #204
Closed

Attempting to use _input will crash Godot #189

lilizoey opened this issue Mar 20, 2023 · 5 comments · Fixed by #204
Labels
bug c: ffi Low-level components and interaction with GDExtension API ub Undefined behavior

Comments

@lilizoey
Copy link
Member

lilizoey commented Mar 20, 2023

Attempting to use _input will crash Godot, because we attempt to construct a Gd<InputEvent> directly from the pointer Godot sends us in the virtual call. However the pointer Godot sends us is actually a Ref<InputEvent>, which needs to be dereferenced using ref_get_object first.

A function defined on Gd like this does that properly

pub unsafe fn from_ref_sys(ptr: sys::GDExtensionConstRefPtr) -> Self {
    let object_ptr = interface_fn!(ref_get_object)(ptr);
    Self::from_obj_sys(object_ptr)
}

My guess is that this is true for all objects inheriting from RefCounted in virtual methods. But it doesn't seem like there's a simple way to check this beyond just manually going through every virtual method and checking. The extension_api.json doesn't say anything.

There isn't a simple hotfix for this beyond just expanding the macros and manually replacing the relevant call for _input. However doing so does fix the issue.

@Bromeon Bromeon added bug c: ffi Low-level components and interaction with GDExtension API labels Mar 20, 2023
@Bromeon
Copy link
Member

Bromeon commented Mar 20, 2023

Thanks! Do you know where that sys::GDExtensionConstRefPtr comes from?

The only place where this type appears in the FFI header is ref_get_object itself:

GDExtensionObjectPtr (*ref_get_object)(GDExtensionConstRefPtr p_ref);

@lilizoey
Copy link
Member Author

Here:

typedef const struct TagExtensionRef *GDExtensionConstRefPtr;

@Bromeon
Copy link
Member

Bromeon commented Mar 20, 2023

Sorry, I meant not the type but the pointer value. You suggested Gd::from_ref_sys() -- but we don't have a pointer of type sys::GDExtensionConstRefPtr anywhere around, and there's no FFI function that returns one.

So I guess we would need to transmute a pointer from a different type (sys::GDExtensionTypePtr)...

@lilizoey
Copy link
Member Author

yeah, just like how we currently just cast a pointer to ObjectPtr in the virtual method calls.

@lilizoey
Copy link
Member Author

Actually it seems like we need to do this for all object pointers

@bors bors bot closed this as completed in afbd2cd Apr 12, 2023
@Bromeon Bromeon added the ub Undefined behavior label Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: ffi Low-level components and interaction with GDExtension API ub Undefined behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants