Rust: re-land #62150 (Implement mem::{zeroed,uninitialized} in terms of MaybeUninit)

[ishitatsuyuki]

62150 got reverted to give users some time to migrate; we should re-land it.

Original issue:

https://github.com/rust-lang/rust/pull/62150 has changed the behavior of mem::uninitialized() (and zeroed()), causing a few popular crates to be killed by SIGILL. A writeup can be found here.

There are cases where outdated (EOL) versions of downstream dependencies have this issue. In that case, the upstream crate will have to migrate to a newer major version of the dependency, which means it will have to deal with those breaking changes.

Therefore, I suggest we back out the change for at least one stable release, providing the community with the time to upgrade.

cc @RalfJung

[RalfJung]

Note that (to my knowledge) this change is in nightly only currently, and won't become stable until 1.38 (released in late September). But it causes trouble already now e.g. by making CI fail, I suppose.

Nominating for lang team discussion. I hope I am doing this right.^^

[RalfJung]

Some more relevant discussion:

• https://github.com/rust-lang/rust/issues/52898#issuecomment-505782110 is an old issue about mem::zeroed causing SIGILLs when used incorrectly.
• I proposed to eventually make zeroed (and uninitialized) panic deterministically for some more cases that we can detect statically, likely including the ones causing issues here. But we should probably wait a bit with this.

[joshtriplett]

Rather than panicking at runtime, could we detect this at compile-time, and issue a compile-time warning about calling mem::uninitialized or mem::zeroed with a type of &T or a function type or similar?

I think we need some kind of compile-time warning that people can start fixing, so that people know they need to fix this. Otherwise, we'll be in the same boat when we un-revert this.

[joshtriplett]

Based on discussion in the lang team: we'd be fine with backing this out, but we do need a lint or similar mechanism to make sure people notice and fix code.

[RalfJung]

@joshtriplett

Rather than panicking at runtime, could we detect this at compile-time, and issue a compile-time warning about calling mem::uninitialized or mem::zeroed with a type of &T or a function type or similar?

I assume you are responding to this proposal?

For some cases yes, but not reliably in generic contexts.

[joshtriplett]

On July 25, 2019 12:38:13 PM PDT, Ralf Jung [e-mail] wrote:

@joshtriplett

Rather than panicking at runtime, could we detect this at
compile-time, and issue a compile-time warning about calling
mem::uninitialized or mem::zeroed with a type of &T or a function type
or similar?

I assume you are responding to this
proposal?

For some cases yes, but not reliably in generic contexts.

Doesn't have to be perfect. If we can detect common cases like &T and function types, that would help people know they need to migrate.

[RalfJung]

Agreed. I think we can do something that traverses the type as far as we see it, and complains when it finds a reference, fn, NonNull or NonZero*.

We should probably stop looking when encountering an enum, or else we'd have to figure out which variant would be picked by the all-0 bit pattern and then look at the fields of that.

[RalfJung]

@ishitatsuyuki beta branches in 12 days. If you want this to be backed out, someone has to do it before that.

[ishitatsuyuki]

Is the lint a requirement for backing out? If so, we may want to reach a consensus on how to implement the check.

[RalfJung]

Is the lint a requirement for backing out?

No, I don't think so. @joshtriplett could you clarify?

[joshtriplett]

Definitely not a requirement for backing out. More a requirement to happen well in advance of re-adding.

[RalfJung]

I'm working on a lint, should have a PR ready tomorrow.

[RalfJung]

Lint PR up at https://github.com/rust-lang/rust/pull/63346

[RalfJung]

@joshtriplett what is the timeline you had in mind for putting the change back in place? uninit and init are two rather questionable intrinsics, so I'd like to get rid of them for good as soon as we can.

To my knowledge, the only crates that we know got broken by this are x11-rs (the broken part of which is "one giant hack" in the words of the maintainer) and old versions of crossbeam::queue (version 0.4 is reported to be broken; 0.5 has been released 9 months ago but I do not know if that one is fixed but crossbeam-queue 0.1 released 7 months ago and it is fixed).

[Centril]

@joshtriplett what is the timeline you had in mind for putting the change back in place? uninit and init are two rather questionable intrinsics, so I'd like to get rid of them for good as soon as we can.

We discussed a one version grace period in this issue (see OP) and in the language team meeting. The lint has also been implemented (and then some...) so I think we should go ahead and revert the revert.

[RalfJung]

Okay. I personally would prefer to have a dynamic check in place as well on top of the static check that we have, just to be sure.

But either way I won't have time to work on this for this release cycle.

[RalfJung]

Status update: I filed https://github.com/rust-lang/rust/pull/66059 to add some run-time protection on top of the static lint that we already have (a first version of that lint is stable for 5 weeks already; a stronger version will become stable in a week).

Once the run-time protection lands, my plan is to wait a week to see if there's a big outcry like there was when #62150 landed. If not, I'll submit a PR to re-implement #62150 and kill the init and uninit intrinsics.

[RalfJung]

Re-landing PR is up at https://github.com/rust-lang/rust/pull/69922.