Tracking issue for panics in mem::uninitialized/zeroed

[RalfJung]

With #66059, mem::uninitialized and mem::zeroed dynamically detect some misuses (such as mem::zeroed::<&T>() or mem::uninitialized::<bool>()) and panic instead of causing UB. Also see this summary for the original FCP. But the check is conservative for now to reduce breakage. This is to track strengthening the check.

• We should recursively check the fields of structs and tuples. This is happening in might_permit_raw_init: also check aggregate fields #71274.
• We should check inside arrays, too. We currently do not because smallvec < 0.6.13 didn't use MaybeUninit and thus triggers this check (if the type inside the array has invalid bit patterns). Other widely-used crates that trigger the panic as of Dec 2021:
  • hyper (fixed since 0.14.12)
  • crossbeam-queue 0.2.1, crossbeam-channel 0.3.9, crossbeam 0.2.12 (these are ancient; current versions are fine AFAIK)
• We should check Variant::Multiple enums, which currently we do not just to stay conservative.

[RalfJung]

git2-rs needed some adjustment to work even with the minimal checks that have already landed, which is good enough for now but even those adjustments will nut suffice once we properly check enums. The reason for this is that ctest uses mem::uninitialized() to test that the bindings are declared correctly, and an uninitialized enum (with more than one variant in its layout) is UB. It has to be UB because the point of the validity invariant of an enum is to make sure the discriminant is always valid, and for uninitialized memory the discriminant test would raise UB.

Cc @gnzlbg @alexcrichton

[valarauca]

Will the unstable methods associated with MaybeUninit be stabilized along with or as part of this change?

I agree this is a positive change for the language, but with a large amount of the MaybeUninit API unstable/nightly only this change feels premature. There's a lot of patterns which simply are not allowed in non-nightly MaybeUninit, and having it suggested as an upgrade path from mem::uninitialized/zeroed feels a bit like a bad joke.

[RalfJung]

@valarauca MaybeUninit has been stable for quite a while. The remaining unstable methods are just convenience, they do not limit functionality. For example, instead of get_mut (just renamed to assume_init_mut) you can do &mut *x.as_mut_ptr(). The safety rules are the same as for assume_init_mut: the data must be already fully initialized before you create a mutable reference.

Which patterns specifically are you talking about?

[Aaron1011]

In #77970 (comment), the panic in mem::uninitialized ended up causing a segfault, since unwinding caused some unsafe drop code to run in an invalid state.

I think it might make sense to force an abort (with a message), rather than use whatever panic strategy is configured. There's some precedent for this - both Rc and Arc will abort in the face of a pathological number of clone() calls (which can actually be triggered from safe code). Since this panic occurs when unsafe code has already triggered undefined behavior, catching the panic or unwinding will probably just make things worse.

One downside is that this test would become more complicated, since we can longer repeatedly trigger the panic in a single test.

[scottmcm]

From #66059 (comment):

It is not entirely clear whether, when a violation of validity is detected, we should panic or abort. For now, we panic, which has the advantage of printing a backtrace where possible. However, unsafe could could be surprised by the panic and not expect unwinding, and that has lead to segfaults on crater. The issue with aborting, however, is that we have no good way to print a backtrace.

[Aaron1011]

There's ongoing work to make Backtrace available in core: #72981 (comment)

Once that becomes available on nightly, we could use it internally to attempt to print a backtrace, and then abort.