search

rust-lang / nomicon

The Dark Arts of Advanced and Unsafe Rust Programming
https://doc.rust-lang.org/nomicon/
Apache License 2.0
1.85k stars 269 forks source link

FFI section suggests dangerous practice of using empty opaque type #29

Closed RalfJung closed 6 years ago

RalfJung commented 7 years ago

The FFI section of the Nomicon currently suggests to use an empty type for "opaque pointer types". That seems pretty dangerous. This came up in the discussion at https://github.com/rust-lang/rfcs/pull/1861, and also in https://internals.rust-lang.org/t/recent-change-to-make-exhaustiveness-and-uninhabited-types-play-nicer-together/4602 many were not happy with using the empty type here.

Until we have proper opaque types, the safer suggestion seems to be do use a ZST with a private field, rather than an empty type. Then at least the type is not actively lying.

sfackler commented 7 years ago

How is this dangerous?

RalfJung commented 7 years ago

with x: &Empty, match *x {} could well be (no final decision has been bade yet) insta-UB if ever executed.

Actually, just passing a &Empty to another function is not really different from passing aliasing &mut u32 -- the promise made by the type is violated. It is not unlikely for the unsafe code guidelines process to decide that doing either of these is UB.

sfackler commented 7 years ago

&Empty is not okay, but you're working with *mut Empty and *const Empty in FFI.

RalfJung commented 7 years ago

&Empty is not okay,

Glad we agree on that. :)

you're working with mut Empty and const Empty in FFI.

While these types are fine, I am worried that people will quickly move to variables or function arguments of type &Empty in the wrappers they build around their FFI code.

This happened e.g. in https://github.com/briansmith/ring/, where BIGNUM used to be an empty enum, and you can indeed find &BIGNUM in the code.

SimonSapin commented 6 years ago

I’ve opened https://github.com/rust-lang-nursery/nomicon/pull/44 to recommend zero-size structs with a private field, instead.

https://github.com/rust-lang/rust/pull/45225#issuecomment-346493804 is another case of Bad Things happening with empty enums used in types that are not "impossible".