godotengine / godotengine/godot-cpp
method binding on non-MSVC compilers relies on undefined behavior
- Dominant language
- C++
- Stars
- 2.7k
- Forks
- 809
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 8
Description
### Godot version
all versions since at least 3 years ago (it's as far back as I looked)
### godot-cpp version
all versions since at least 3 years ago
### System information
non-MSVC compilers
### Issue description
Method binding in godot-cpp relies on undefined behavior if `TYPED_METHOD_BIND` is not defined. It seems that it's only enabled with the MSVC compiler.
Here, `reinterpret_cast` is used on an unrelated type to cast to `MB_T`:
https://github.com/godotengine/godot-cpp/blob/4131b7f95f5a4ec660484431b175e15d95aed8db/include/godot_cpp/core/method_bind.hpp#L319-L326
Here, `reinterpret_cast` is used on unrelated member function pointers:
https://github.com/godotengine/godot-cpp/blob/4131b7f95f5a4ec660484431b175e15d95aed8db/include/godot_cpp/core/method_bind.hpp#L342-L351
`reinterpret_cast`ing here causes UB as the resulting member to function pointer cannot be used safely, as you are not casting to `unsigned char *` or something like that. Moreover, casting to a member function pointer of a non-base class is also UB. Member function pointers are implementation defined, so honestly, there is no way to rely on what the compiler will do here.
It seems like MSVC actually errors on the UB here which is really nice. I think that switching to `TYPED_METHOD_BIND` would be the right thing to do here.
If y'all are open to it, I could work on a fix. I think it would just be enabling that switch on for all builds.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.