Do #[repr(Rust)] unions have internal padding?

[alercah]

Forked from #156. The question is specifically whether a union type has non-tail padding when every variant has padding at a particular byte, meaning that the padding could be clobbered. @RalfJung says no, because he would like unions to simply be [byte; N]. Nobody has made a case for any other answer, as far as I'm aware. But making a separate issue since it's a separate question about the #[repr(Rust)] ABI and what we want to guarantee.

[RalfJung]

It's also an LLVM lowering question, we might have to do some work to ensure that LLVM does not consider any repr(Rust) union to have padding.

[Lokathor]

Is there a scenario where it's an advantage to have internal padding within a union?

[comex]

It can provide niches.

[alercah]

Padding can't provide niches because it's subject to clobbering.

[alercah]

Technically one advantage is that padding allows calling convention optimizations.

I also think it would be a bit weird and possibly uninituitive if overaligned unions with tail padding didn't treat it as proper padding?

[CAD97]

The extra work to tell LLVM there is no padding is "simple enough"; lower the union as if it had a variant which contains no padding.

If we do allow interior padding in #[repr(Rust)] unions, then I'd actually prefer the stricter definition of "byte must be potentially valid for some variant" to also benefit from niches, and introduce #[repr(trivial)]¹ for the full bag-o-bytes, additionally without trailing padding.

I'm torn on whether #[repr(Rust)] unions having trailing padding is enough for me to prefer strengthening their byte-validity in this way.

Footnotes

1. bikeshed ahoy; raw, simple, etc; could maybe tie into putting the repr on struct as well for non-C-FFI stable ABI where we can guarantee the trivial layout, and free up #[repr(C)] to change to match the various "platform C" layout peculiarities rather than being target independent and used for manual allocation and pointer offsetting. ↩

[scottmcm]

The extra work to tell LLVM there is no padding is "simple enough"; lower the union as if it had a variant which contains no padding.

That's what we already do, based on the discussion in rust-lang/rust#97712 (comment)

[CAD97]

That implementation is interesting, as IIUC it's relying on MaybeUninit not having padding in order to copy padding in the untyped copy, at least in the case where it's not broken down into zero-padding-int-type chunks.

The actual behavior of the pointer copy APIs is an orthogonal decision issue from whether unions have padding, though.

Silly libs patch to avoid relying on non-padding of union in MaybeUninit

union MaybeUninit<T> {
    value: T,
    undef: (),
    nopad: [u8; const{size_of::<T>()}],
}

[RalfJung]

@scottmcm yeah I don't see how that commit relates to how we lower unions.

Currently we don't seem to be doing this right, this

pub union U {
    f: (u8, u16),
    g: (),
}

pub fn u(u: U) {}

becomes define void @_ZN10playground1u17h71b96be7befbcafcE(i8 %u.0, i16 %u.1). :(

[RalfJung]

Ironically, making it #[repr(C)] "fixes" it to define void @_ZN10playground1u17hdaf5dbffaf364510E(i32 %0).^^ I think both of these are wrong, they are exactly swapped...

[bjorn3]

No, it is actually correct for #[repr(C)]. The abi specifies that it is to be passed as 32bit integer in a single register: https://godbolt.org/z/8YzKqdEfv If you use a larger type, it will split it: https://godbolt.org/z/sxqqKf46z Just like rustc does: https://godbolt.org/z/fMYEETnjx

[RalfJung]

Oh I see.

But anyway, for repr(Rust) we do not currently have preserve-all-bits semantics.

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);
    }
}

Funky, I didn't know that failed :) I'm assuming this isn't UB, and that it is indeed illegal for rust to zero the padding here, seeing as miri runs it perfectly fine.

Worth me making an issue on rust-lang/rust about to track this? I can't seem to find an existing one.

[CAD97]

Just a note to make sure it's noted in the discussion: MaybeUninit is (currently) #[repr(transparent)] and not #[repr(Rust)].

And #[repr(transparent)] unions are guaranteed to have the same ABI as the transparently wrapped type.

(Tuples are #[repr(Rust)], though, so the previous should follow #[repr(Rust)] padding rules.)

However, Miri IIRC doesn't do any padding deinit at the moment, so Miri accepting it isn't necessarily an indicator that it's intended to be supported.