bytemuck icon indicating copy to clipboard operation
bytemuck copied to clipboard

Safe `offset_of` for packed structs using `addr_of`?

Open phil-opp opened this issue 2 years ago • 9 comments

Hi, I just stumbled over: https://github.com/Lokathor/bytemuck/blob/c705218630d0a11af39b39a1c7639de6dce04db3/src/offset_of.rs#L60-L65

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

phil-opp avatar Nov 07 '23 15:11 phil-opp

offset_of support predates addr_of in the standard library.

notgull avatar Nov 07 '23 19:11 notgull

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.

Lokathor avatar Nov 08 '23 05:11 Lokathor

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?

phil-opp avatar Nov 08 '23 08:11 phil-opp

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?

Lokathor avatar Nov 08 '23 13:11 Lokathor

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.

phil-opp avatar Nov 09 '23 11:11 phil-opp

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

Lokathor avatar Nov 09 '23 15:11 Lokathor

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.

phil-opp avatar Nov 09 '23 16:11 phil-opp

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

Lokathor avatar Nov 09 '23 18:11 Lokathor

Thanks, this helps indeed!

phil-opp avatar Nov 09 '23 18:11 phil-opp