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

Using Rust enums to represent C enums can result in undefined behavior #667

Closed
mjbshaw opened this issue Apr 26, 2017 · 5 comments
Closed

Comments

@mjbshaw
Copy link

mjbshaw commented Apr 26, 2017

This is similar to #225, but different in that this is about C enums that are used as actual enums (not bit flags).

Consider, for example, the following C code:

typedef enum {
  STATUS_OK,
  STATUS_BAD_ARG,
  STATUS_NO_MEMORY,
} Status;

This gets translated by bindgen into:

#[repr(u32)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum Status { STATUS_OK = 0, STATUS_BAD_ARG = 1, STATUS_NO_MEMORY = 2, }

The problem is that the C project could add enum fields (which is often considered to not be an API/ABI breaking change in the C world) to the end (e.g. a new STATUS_BAD_AUTH = 3 field), at which point using the (old) Rust enum would result in undefined behavior due to Rust's strict enum range requirements. If a function in the C API ever returned STATUS_BAD_AUTH then the Rust code would have undefined behavior. Static linking the C library will help mitigate this, but if the library is dynamically linked then the Rust code really can't be considered safe.

Here are my current thoughts on how to solve this, though other solutions are likely possible:

  • By default, don't use Rust enums to represent C enums. Introduce a new type (e.g. a simple struct) that can be used instead.
  • Allow the bindgen user to specify a flag that will make bindgen generate Rust enums to represent C enums only for C enums that are used as input parameters. For example, if the C function is enum foo do_something(enum bar) then the bar enum could be represented safely as a Rust enum, since it would only be constructed in the Rust code. If all the C functions that bindgen finds only take an enum by value (or a pointer to const enum), then this should be a safe optimization to make.
  • Allow the bindgen user to specify a different flag that will make bindgen generate Rust enums for all C enums. This would be an unsafe option, but would allow the user to take advantage of potential compiler optimizations, and in a controlled environment (e.g. statically linking to a known library version) can be done safely.

This would be a nontrivial amount of work, but I think at least the first point is important for ensuring the safety of Rust programs that have to interact with C libraries.

See rust-lang/rust#36927 for the relevant issue filed against Rust itself.

@fitzgen
Copy link
Member

fitzgen commented Apr 26, 2017

Thanks for the bug report!

@fitzgen
Copy link
Member

fitzgen commented Apr 26, 2017

@jeandudey
Copy link

By default, don't use Rust enums to represent C enums. Introduce a new type (e.g. a simple struct) that can be used instead.

Just as a reminder don't do this unless #[repr(transparent)] gets implemented or bugs like #439 could happen.

@lilianmoraru
Copy link

lilianmoraru commented Feb 14, 2018

Related issue #758

"Fixed" through constified enums that you then need to use correctly for "bounds checking"(you can manually check if the "value" overflows the expected API and then probably abort or something).

@emilio
Copy link
Contributor

emilio commented Jul 31, 2019

Yeah, this is fixed now.

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

No branches or pull requests

5 participants