Rust: Uninhabited types are ZSTs despite partial initialization being allowed in safe code.

Created on 23 Mar 2018  路  14Comments  路  Source: rust-lang/rust

When we optimized product types with uninhabited fields to be ZST, partial initialization was overlooked, i.e. writes to the inhabited sibling fields are allowed despite missing in the layout.
Thankfully, it's impossible to read or borrow such fields, because the whole value must be initialized (although this may be relaxed in the future). However , the initialized fields are still dropped.

One way we could work around this is track uninhabitedness but only take advantage of it in enums.
(I am not aware of a solution that works with variant types, which I think we should keep in mind.)
EDIT: we can disallow creating the enum from an incomplete variant. This is also required by niche optimizations because the niche slot being uninitialized would result in UB.
EDIT2: incomplete variants might be possible today already (https://github.com/rust-lang/rust/issues/49298#issuecomment-380615281).

As an example, in debug mode, this snippet prints 0, because x.1 and y overlap:

enum Void {}

fn main() {
    let mut x: (Void, usize);
    let y = 1;
    x.1 = 0;
    println!("{}", y)
}

cc @nikomatsakis @nagisa

A-codegen C-bug I-unsound 馃挜 P-medium T-compiler regression-from-stable-to-stable

Most helpful comment

To pile on the scary labels, that also makes this a regression from stable to stable!

All 14 comments

Seems I-unsound.

enum Void {}

fn main() {
    let mut x: (Void, [u8; 20]);
    x.1 = [0x12; 20];
}

Also, this regression was introduced in Rust 1.24.0 from what I see, this correctly works in 1.23.0.

To pile on the scary labels, that also makes this a regression from stable to stable!

48493 looks like an instance of this - cc @alexcrichton

@eddyb Weak creates a value of the uninhabited type via mem::uninitialized, it does not perform partial initalization. It's UB anyway. This issue (edit: resolving this issue) would at best make the code working-but-still-UB.

@rkruppe is there a separate bug for that?

@eddyb Uh, I guess #48493 is that bug? Although if we land a workaround before the proper fix (MaybeUninitialized with unsizing support) becomes available, we should make sure we open a new bug for tracking removal of the workaround and introduction of the proper fix.

There's potentially an even more problematic implication here: initializing an uninhabited (because of its fields) variant and panicking while doing so could also result in writing over data following the whole enum, assuming the data is larger than the enum itself, e.g.:

let x: Option<(String, !)>;
let y = vec!["foo".to_string()];
x = Some((String::new("bar"), panic!("{:?}", y)));

This might be hard to demonstrate today, because of some extra temporaries, but optimizations would likely prefer to write to x directly, which would spell disaster.
I'm not sure what a proper solution would be here, we certainly need to discuss it.

We discussed in the meeting. It seems like the rules want to be:

  • Ignore uninhabitedness when laying out product types (! is treated like any ZST)
  • Ignore variants for enums if they are both uninhabited and ZST

This ensures there is space for non-ZST data... we need to do this if we want to be able to optimize temporaries away pre-monomorphization.

Ignore variants for enums if they are both uninhabited and ZST

Specifically, the data inside the variant (not including any tag or anything else enum-specific).

Shall there be some #[attribute] to opt out partial initialisation, but gain "multiply by zero" layout optimisation?

Related idea: #[repr(blackhole)] forcing any enum, struct or union to be ZST, making all writes drops and statically forbidding reads or referencings.

@vi Note that, IIRC, Option<(T, !)> should be zero-sized even if (T, !) isn't (unless I'm misremembering).
Anyway, the tricky cases are tricky because they make "in-place optimizations" much harder, and we'd prefer to cater to usecases without making (T, !) a ZST.

Note that https://github.com/rust-lang/rust/issues/49298#issuecomment-380615281 is more general than partial initialization, and it concerns writes to return destination, followed by a panic later, in the context of wanting to optimize away copies.

IIRC, Option<(T, !)> should be zero-sized even if (T, !) isn't (unless I'm misremembering).

println!("{}", std::mem::size_of::<Option<(!,u32)>>());
8.

Ah, right, I was thinking of a subtler distinction, nevermind.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dnsl48 picture dnsl48  路  3Comments

Robbepop picture Robbepop  路  3Comments

wthrowe picture wthrowe  路  3Comments

drewcrawford picture drewcrawford  路  3Comments

pedrohjordao picture pedrohjordao  路  3Comments