search

Amanieu / intrusive-rs

Intrusive collections for Rust
Apache License 2.0
412 stars 48 forks source link

offset_of causes UB by calling mem::uninitialized on any type #25

Closed RalfJung closed 5 years ago

RalfJung commented 5 years ago

The offset_of macro contains the following

        // Create an instance of the container and calculate the offset to its
        // field. Although we are creating references to uninitialized data this
        // is fine since we are not dereferencing them.
        #[allow(unused_unsafe)]
        let val: $container = unsafe { $crate::__core::mem::uninitialized() };

Unfortunately, the comment is wrong. It's not just creating a reference to uninitialized data, it is creating a local variable with uninitialized data. For instance, the following code is UB:

let x: bool = mem::uninitialized();

That said, there currently is no correct way to do what you want. This bug can only really be fixed once MaybeUninit gets stabilized.

Also see this discussion on internals.

Amanieu commented 5 years ago

One possibility would be to create a fake, but properly aligned pointer out of thin air and then getting the offset of the field from that. Although this technically involves dereferencing the pointer to get the address of its field, no memory is actually accessed.

RalfJung commented 5 years ago

Field offset computations use getelementptr inbounds, I don't think LLVM lets us do that on fake pointers (not backed by an actual allocation). We can argue our way around that if the offset is 0, but for non-0 offsets I think this is still UB.

RalfJung commented 5 years ago

Do you think it would be a good idea to centralize the "offset_of" hacks into one crate? If intrusive-rs used the macro from https://github.com/Gilnaa/memoffset/, we'd only have to worry about getting one instance of this work despite technically being UB.

Amanieu commented 5 years ago

I'm happy to have a centralized implementation of offset_of!. In fact, I feel that container_of! should probably also be moved to that crate.