Repository navigation
repr(Rust) unions (including MaybeUninit) do not preserve padding bytes #99604
Description
The following program isn't detected to have any UB in miri, and doesn't hit the panic, but does panic when ran (with or without optimizations) normally (using LLVM)
use std::mem::MaybeUninit;
unsafe fn print(x: MaybeUninit::<(u16, u8)>) {
let value: u32 = std::mem::transmute(x);
if value != 0xaabbccdd {
panic!("got {value:x}");
}
}
fn main() {
let mut x = MaybeUninit::<(u16, u8)>::uninit();
unsafe {
x.as_mut_ptr().cast::<u32>().write_unaligned(0xaabbccdd);
print(x);
}
}Panics with "got bbccdd", so the padding byte, presumably at aa, got zeroed.
The LLVM IR for print says it passes it as 2 arguments, so it makes sense that the padding is getting zeroed.
define void @_ZN10playground5print17hfd0afe434a63e3b7E(i16 %x.0, i8 %x.1) unnamed_addr #0 [...]
We (might?) already promise that MaybeUninit is just a bag of bytes that doesn't zero any padding, but we also promise that it has the ABI of T, and those two seem to be in conflict.
Reacted by nora, Andrei Damian, Lv Yitian, Stepan Koltsov and Alisa Sireneva
Activity
workingjubilee commentedon Jul 22, 2022
My understanding was that at some point it had been decided that we didn't have to promise anything regarding the padding byte in exactly these cases.
workingjubilee commentedon Jul 22, 2022
In the models where memory accesses are typed, this too does have a type! It's just suggesting the type of "a blob of bytes". But this would not be the first time we wrote a check that (our lowering to) LLVM can't cash.
RalfJung commentedon Jul 22, 2022
Ah, yeah, good point. :(
I agree, the implementation of
swap_nonoverlappingshould be fixed, or the compilation ofrepr(transparent)unions needs to be adjusted.The docs have recently been adjusted to say "The operation is untyped in the sense that data may be uninitialized or otherwise violate the requirements of T. The initialization state is preserved exactly". So what you say is not the intention of the implementation. See #97712.
Some other functions (
ptr::copy,ptr::copy_nonoverlapping) have documented for a long while that they act likememcpy/memmove, which also preserve all bytes disregarding the type -- IMO it would be strange forptr::swap_nonoverlappingto behave differently here.workingjubilee commentedon Jul 22, 2022
We also claim that
mem::transmutewill simply be "like a memcpy", which leads to #96140 where LLVM disagrees with us on our interpretation of our own claims. While I am all for telling LLVM to take a hike, I must stress that committing to preserving some bits what we claim we really care about is au16and au8is asking lowering and the optimizer to reason about a very different version of the code than what is written.RalfJung commentedon Jul 22, 2022
Well, we say it's like memcpy "under the hood", which is not wrong, but then the inputs and outputs are fundamentally "clamped" by this being a by-value operation. memcpy does not act on values. The type-changing memcpy is not what makes the padding disappear here, it's the passing-args-to-transmute and passing-ret-from-transmute (and it'd be the same with any safe function as well).
But probably we should just remove that mention of memcpy if it confuses people more than it helped...
I have no idea how we could possibly teach this. :(
rdrpenguin04 commentedon Jul 22, 2022
That breaks down if it's passed to multiple functions though. Also, @chorman0773 would probably have some choice words about making a fundamentally "normal" type be magic.
RalfJung commentedon Jul 12, 2024
A relevant recent Zulip discussion.
We guarantee that
MaybeUninit<T>is ABI-compatible withT, which pretty much inevitably leads to these problems. This was maybe premature as we didn't realize at the time what that would imply. But whether it is possible and a good idea to change this now -- I don't know.