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

Safe offset_of for packed structs using addr_of? #212

Open
phil-opp opened this issue Nov 7, 2023 · 9 comments
Open

Safe offset_of for packed structs using addr_of? #212

phil-opp opened this issue Nov 7, 2023 · 9 comments

Comments

@phil-opp
Copy link

phil-opp commented Nov 7, 2023

Hi, I just stumbled over:

/// # Usage with `#[repr(packed)]` structs
///
/// Attempting to compute the offset of a `#[repr(packed)]` struct with
/// `bytemuck::offset_of!` requires an `unsafe` block. We hope to relax this in
/// the future, but currently it is required to work around a soundness hole in
/// Rust (See [rust-lang/rust#27060]).

Wouldn't it be possible to do this safely using the core::ptr::addr_of macro to calculate the offset?

@notgull
Copy link
Contributor

notgull commented Nov 7, 2023

offset_of support predates addr_of in the standard library.

@Lokathor
Copy link
Owner

Lokathor commented Nov 8, 2023

We could probably adjust things but it would have to be opt-in, because the offset_of module isn't feature gated and so it needs to work on 1.34 by default.

@phil-opp
Copy link
Author

phil-opp commented Nov 8, 2023

Ah sorry, I missed the README note about the 1.34 MSRV. In that case, the easiest solution is probably to add a new safe_offset_of macro, gated behind a cargo feature. Would that be ok with you?

@Lokathor
Copy link
Owner

Lokathor commented Nov 8, 2023

Ah, well we could but I thought that the standard library offset_of was developing at a reasonable enough pace for people to just be able to use that sooner than later.

Also, double checking the file itself I actually don't see the unsafe block that is being referred to, so is that note old?

@phil-opp
Copy link
Author

phil-opp commented Nov 9, 2023

I wasn't aware that the standard library offset_of is so close to stabilization. I'm fine with waiting a bit and using a manual workaround based on addr_of for packed structs until then.

Also, double checking the file itself I actually don't see the unsafe block that is being referred to, so is that note old?

I think the unsafe block is required by Rust when you try to create a reference to a field of a packed struct, as it might be unaligned.

@Lokathor
Copy link
Owner

Lokathor commented Nov 9, 2023

phil why are you using packed structs to begin with? They're very bad for your health, you shouldn't ;3

@phil-opp
Copy link
Author

phil-opp commented Nov 9, 2023

Haha, yeah :D. I'm dealing with a file format that contains some 64-bit offsets that are only aligned to 4 bytes. It's just some prototype code to figure out the details of the format, so I'm ignoring endianness for now do a direct cast to Rust structs.

@Lokathor
Copy link
Owner

Lokathor commented Nov 9, 2023

https://docs.rs/pack1/latest/pack1/ may help you

@phil-opp
Copy link
Author

phil-opp commented Nov 9, 2023

Thanks, this helps indeed!

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

3 participants