MTLPixelFormat and related enumeration types are unsound?

[drewcrawford]

Trying to figure out if I'm missing some neat trick. At first glance it appears that MTLPixelFormat and related are unsound.

For example, MTLPixelFormat is returned by objc here. If I have followed the controlflow correctly back to the source, I believe we transmute it here, e.g. we are transmuting the value from C.

The trouble with this is that on the C side, MTLPixelFormat is a glorified typedef for NSUInteger

typedef NS_ENUM(NSUInteger, MTLPixelFormat)

which has a legal range usually of 4 million values, any of which may be returned from a ObjC API. If the value returned is unknown to the Rust enum, this is UB for the Rust memory model.

I should mention to be clear that the "values unknown to Rust" problem is something that typically happens when new pixel formats are added in OS releases, such as depth16Unorm fairly recently. So we might expect older programs compiled with older metal-rs to UB when run on newer OS.

I believe this needs to be represented with constants and an integer type internally. Externally, it may be feasible to add an "unknown" case to the enum with a integer payload type. I am a little unclear on what the situation is with breakages (e.g., adding a payload to #[repr(u64)] pub enum will be a different memory layout) or if it would make more sense to just expose constant values as the binding.

Further reading

• similar example from Unsafe code guidelines
• Reddit thread
• Bindgen bug

[kvark]

Thank you for raising this! Agree this is unsafe and needs to be fixed.
I'm thinking that we should fix it fully internally. All the functions returning enums should read integers first, check their value, and only then transmute. They'd panic otherwise, which is not expected but still safe (also, controversial).