Rust: Regression from 1.44 to 1.45

[mcarton]

The following MVE prints 42 with rustc 1.45.0 but 13 with rustc 1.44.0

struct Foo {
    x: i32,
}

fn main() {
    let mut foo = Foo { x: 42 };
    let x = &mut foo.x;
    *x = 13;
    let y = foo;
    println!("{}", y.x); // -> 42; expected result: 13
}

Initially reported on Stack Overflow.

[mcarton]

With beta or nightly this again prints 13 as expected.

[omrerdn]

struct Foo {
    x: i32,
}

fn main() {
    let mut foo = Foo { x: 42 };

    let x = &mut foo.x;
    *x = 13;
    let y = foo;
    println!("{:?}", (&y).x);  //only added this line
    println!("{:?}", y.x); //13
}

If there is any benefit, it works as expected with this code.

[lcnr]

This is a duplicate of the already fixed #73609

It seems like this bug somehow slipped into beta :/

[SimonSapin]

@rust-lang/release Should there be a 1.45.1 with https://github.com/rust-lang/rust/issues/73609?

[peterjoel]

@SimonSapin Can 1.45.0 be yanked?

[jonas-schievink]

Yanking a release is not a thing. Presumably it wouldn't do anything once the stable channel is updated anyways.

[peterjoel]

@jonas-schievink It would do something for anyone who typed rustup install 1.45.0.

[benschulz]

I'm confused. Based on the examples this bug appears to be very impactful, i.e. it should break a lot of code. Yet it made it to stable which, I assume, involves a crater run and independent testing on beta. Is there a subtle condition to this bug that isn't met in most productive code?

[oli-obk]

The bug is almost impossible to trigger on real world code. You need all values that are going into the bug to be constant values and there can't be any control flow or function calls in between.

[wesleywiser]

As far as I know, rustup has no support for yanking releases. Doing so would probably have to be done manually by the infra team and it could cause more harm than good. For example, are current or old versions of rustup going to gracefully handle the case where a user has an installed release that doesn't exist anymore? We've had other point releases for major issues in the past and users are generally quick to upgrade.

There is already a 1.45.1 release scheduled for later this week with other backports and this fix will almost certainly be included.

[jieyouxu]

Could rustup try to print a massive warning (anytime it is invoked) telling the user that the patch is critical and is strongly recommended and that the previous release is deprecated?

[LeSeulArtichaut]

Assigning P-critical as discussed as part of the Prioritization WG Procedure.

[Mark-Simulacrum]

I am happy to include 5fa8b0880825d83eb01151e43e7de1e94e05cd2d in 1.45.1. @oli-obk -- there's some merge conflicts when cherry-picking onto stable, could you provide a commit/PR against stable with the merge conflicts fixed?

FWIW, I find the notion of putting warnings or saying "don't use this release" to be something we probably don't want to do right now. We'd basically have to do so for all non-latest Rust versions; they're all "unsupported" and may contain this (or other) problems. We ourselves do not patch more than the latest stable release if CVEs are detected, generally speaking. I would also ask that further discussion about yanking releases and such be directed to an internals thread and not this issue to avoid creating noise while we resolve this bug.

[oli-obk]

on it Nevermind, @wesleywiser is already on it :heart:

[zhuxiujia]

Why this silly mistake，The production environment is going to break

[llacroix]

What @jieyouxu said.
It could be done with rustup check that will check and warn the user that new release introduce critical bug fixes or even security fixes.

[oli-obk]

@llacroix I don't think this is the place to discuss policy changes. This issue is specifically for fixing the regression, not how regression fixes are communicated in general.

[jonas-schievink]

This thread has served its purpose, so locking it.

[pnkfelix]

@lcnr commented:

This is a duplicate of the already fixed #73609

by the way, this issue was not, technically, a duplicate of #73609.

Or at least, the issue described in #73609 only surfaces under conditions witnessed after commit 38bd83df88288f2f8d1fc2dd317189cac3825920 (PR #71911, merged on June 21st)

bisected with cargo-bisect-rustc v0.5.2

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 2020-04-01 --end 2020-06-22 -- run

while this issue surfaces under conditions witnessed after commit 3fe4dd2dda2826643c4ce4ee7307707a90e08d25 (PR #71953, merged on May 11th)

bisected with cargo-bisect-rustc v0.5.2

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 2020-05-01 --end 2020-06-01 -- run

It does seem like the same patch (PR #73613) fixes both issues; i.e., it seems likely that there is indeed one bug underlying both distinct issues.

But the fact that they are distinct issues helps explain why we did not backport the fix to beta way back when.