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

Question about packed structs #2

Closed
jgarvin opened this issue Dec 31, 2020 · 5 comments
Closed

Question about packed structs #2

jgarvin opened this issue Dec 31, 2020 · 5 comments

Comments

@jgarvin
Copy link

jgarvin commented Dec 31, 2020

The documentation has a very interesting warning about fields of packed structs getting moved. This isn't really an issue with this library but I found it very surprising, and was wondering if you had any links to discussion about why this happens. It seems very surprising and I haven't found anything googling (packed is all about getting the compiler to not do something that it normally does so it's a little weird for it to trigger new behavior). Could also be useful to just link in the documentation for a future curious people :)

Closest thing I found was this which suggested not allowing drop for packed structs, don't know if it was accepted or if it applies recursively to fields contained inside but I guess not otherwise this wouldn't be a problem.

@RustyYato
Copy link
Owner

You can see it in the ptr::drop_in_place docs

Unaligned values cannot be dropped in place, they must be copied to an aligned location first using ptr::read_unaligned. For packed structs, this move is done automatically by the compiler. This means the fields of packed structs are not dropped in-place.

You can figure this out for yourself too! Take the following type:

#[repr(packed)]
struct Foo {
    offset: u8,
    value: String,
}

How should the compiler drop the String? It can't just take a pointer to the string and cast it to a &mut String to pass to Drop::drop, because that would create an unaligned reference,

reference for alignment

The alignment of a value specifies what addresses values are preferred to start at. Always a power of two. References to a value must be aligned. More.

So the only option is to copy out the String to some temporary well-aligned location and then call drop_in_place.

@jgarvin
Copy link
Author

jgarvin commented Dec 31, 2020

Wow, that's an interesting design choice, doesn't seem consistent with the rust philosophy of not having implicit copies. Definitely answers my question though thanks!

@jgarvin jgarvin closed this as completed Dec 31, 2020
@RustyYato
Copy link
Owner

This isn't a deep copy, it's just copying the bytes of the String (which is just 3 usizes), so it's not expensive, and you are opting in to this by using repr(packed), so it's not that hidden. It's exactly the same as moving the fields to a new location before dropping them, and I wouldn't be worried about a few extra moves. 😄

@jgarvin
Copy link
Author

jgarvin commented Jan 1, 2021 via email

@RustyYato
Copy link
Owner

That was a really interesting short history of packed structs in C++. I wasn't aware of how C++ handled packed structs, having not used it that much.

I wouldn't expect my object's address to change right before it destruct

There are many ways that your object's address can change right before it destruct. For example, someone else could move your type before destruction. In fact this pattern isn't uncommon as a safe replacement for ManuallyDroping fields in Drop:

sturct Foo {
    value: Option<T>,
}

impl Drop {
    fn drop(&mut self) {
        let value = self.value.take().unwrap();
        // do work
    }
}

I don't see how patterns like this are any different from moving a packed sturct's fields right before it destructs. However I can see how this can be surprising coming from C++

in general you can't safely do this in C++ because there is no guarantee that
there aren't still pointers left pointing to the object when its destructor
triggers, and the code inside the destructor may even use those pointers.

That's exactly how I found out about this! Someone pointed out that people may want to use RelPtr to point into a parent's fields like so:

#[repr(packed)]
struct Foo {
    w: u8,
    x: String,
    y: Vec<i32>,
    z: Bar,
}

struct Bar {
    x: RelPtr<u8>, // points to Foo.w
}

But if the parent is packed and Bar uses RelPtr on drop, you will likely run into UB because the offset has changed! I think this is similar to what you're describing from C++, where you could have pointers pointing into some object, and you don't want it to move before you destruct. (But in the opposite direction this time).

But, in neither case does the compiler move your object for you. The rust
design is definitely safer, just surprising. [..]
On the other hand I probably wouldn't have fields
with Drop either in that case, so maybe Rust's move on drop behavior just
helps make packed more usable for people who want to sprinkle it for
memory/cache savings in critical places.

Yeah, Rust consistency prioritizes safety over convenience if there is tension. So this decision makes sense.

A) packed telling the compiler to not put padding between fields

This is true, but also it forces the alignment of the entire struct to be 1

B) packed telling the compiler fields must be copied to an aligned location
before accessing them, then copied back if modifying

From this thread I'm learning B happens, at least for Drop. Does it happen
whenever I directly invoke a method on any field of a packed struct?

Rust only implicitly copied the elements of a packed struct on implicit drop or when you call drop_in_place on a packed struct. That's why it's UB to access the elements of a packed struct by reference.

C) accessing a packed struct field by pointer directly to the field

This is only possible with the currently nightly #![feature(raw_ref_op)] or doing the pointer offsets with ptr::offset and friends, but yes this is safe as long as you check the alignment before using anything that requires the value to be aligned, or copy the data to an aligned location.

D) accessing a packed struct field by reference directly to the field

This is UB in general, unless the alignment of the field is 1, because the alignment of the struct is 1 so we can't infer the alignments of any of the fields.

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

No branches or pull requests

2 participants