godot-rust / godot-rust/gdext

It is UB to use most of the library before init code is run

Open
#1,046 19 comments 2 reactions 0 assignees View on GitHub
bug c: ffi ub
Dominant language
Rust
Stars
5.2k
Forks
312
Avg merge
12h 59m
Merged PRs (30d)
9

Description

Most of the library relies on accessing the Godot binding, which is only initialized when the initialization code is run. But in most cases we only check if the initialization code is run when debug assertions are turned on.

This means that if you for instance make a unit test that accesses these APIs and run it in release mode, you'll hit UB.

## Some possible solutions

1. Check when debug assertions are turned off as well.

This would make everything safe, but would likely also incur performance penalties in situations where the initialization code will guaranteed be run, such as when actually being called by Godot.

The performance issues is why we only check when debug assertions are turned on in the first place.

2. Track when code has been run using some kind of token in code.

This is hard to weave through every relevant part of the code, especially in a way that doesn't impact ergonomics. Additionally it will still require at least one check every time Godot calls the rust part of the code.

3. Have an unsafe compile time flag for when the initialization code is run.

I am not entirely sure how we should decide when this flag is enabled or not. Initially i thought we could enable it when we're compiling as a `cdylib` but we cant actually determine that very easily, and it likely wouldn't work as expected if we could. As if you list several crate types in the `Cargo.toml` it'll compile against all of them at the same time.

So if for instance we had `crate-type = ["rlib", "cdylib"]` then anything behind `cfg(crate-type = "cdylib")` would also get compiled for the `rlib` target anyway.

See also this issue: https://github.com/rust-lang/rust/issues/20267

We could also just require the user to explicitly enable this flag somehow, say a feature. But that means that suddenly people's code will become slower and it'd be an extra hurdle for making code run fast. But maybe we could add some useful warnings for when people are running the initialization code in release mode? since if the initialization code gets run in release mode they're likely in a situation where they'd want this feature enabled. We might also be able to add some other useful heuristics to give information to the user when they may be using it wrong.

4. Make compiling in release mode unsafe

I.e we basically just say "you need to make sure you don't access the Godot API before the initialization code is run when you compile code in release mode".

This is very similar to the previous point but we basically just use `debug_assertions` as the relevant flag (or rather it's the inverse of the previous point, as _not_ using this flag is what is unsafe).

However i think this is very strange from a Rust-perspective. In Rust it is generally assumed that `--release` wont make anything unsound, but will merely make the compiled code faster, compilation slower, and debug information worse. For beginners to the library it may be helpful, but in the long run i suspect it may be more confusing than anything else, leading to some subtle bugs.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.