Regression: Miscompilation due to bug in "mutable noalias" logic

[briansmith]

The ring test suite started failing when I tried to upgrade to untrusted 0.8.0. The regression first shipped in nightly-2021-03-23:

We were further able to narrow down the regression by bisecting down to: 97663b6 "Auto merge of #82834 - nikic:mutable-noalias, r=nagisa Enable mutable noalias for LLVM >= 12."

The symptoms are 100% consistent with lifetimes being an input into the problem. The regression occurs in --release mode.

Code

I tried this code:

Cargo.toml:

[package]
name = "untrusted-user"
version = "0.1.0"
edition = "2018"

[dependencies]
untrusted = { git = "https://github.com/briansmith/untrusted", commit = "bb8d942ee2af7c37b64e6dbc06b2aaa78b3059aa" }

[profile.bench]
lto = true
codegen-units = 1

src/lib.rs:

#[inline(never)] // Required otherwise the test passes.
fn read_single_byte_value<'a>(input: &mut untrusted::Reader<'a>) -> untrusted::Input<'a> {
    input.read_bytes(1).unwrap()
}

#[test]
fn test_regression() {
    let input = &[0x00];
    let input = untrusted::Input::from(input);
    let _ = input.read_all((), |input| {
        let value = read_single_byte_value(input);

        let mut r2 = untrusted::Reader::new(value);
        r2.read_byte().unwrap();

        assert!(input.at_end());

        Ok(())
    });
}

I expected to see this happen: explanation

cargo +nightly test --release should pass.

Instead, this happened: explanation

cargo +nightly test --release fails with:

thread 'test_regression' panicked at 'assertion failed: input.at_end()', src\lib.rs:16:9

Version it worked on

Stable
Beta
nightly-2021-03-22

Version with regression

cargo +nightly-2021-03-23 version --verbose

cargo 1.52.0-nightly (90691f2bf 2021-03-16)
release: 1.52.0
commit-hash: 90691f2bfe9a50291a98983b1ed2feab51d5ca55
commit-date: 2021-03-16

rustc 1.53.0-nightly (5d04957a4 2021-03-22)
binary: rustc
commit-hash: 5d04957a4b4714f71d38326fc96a0b0ef6dc5800
commit-date: 2021-03-22
host: x86_64-pc-windows-msvc
release: 1.53.0-nightly
LLVM version: 12.0.0

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

[jyn514]

Enable mutable noalias by default on LLVM 12, as previously known miscompiles have been resolved. Now it's time to find the next one ;)

@nikic: found it :P

[jyn514]

Marking this as a beta regression since 1.53 is about to be promoted to beta: #84909

[briansmith]

cargo +nightly test --release fails, but cargo +nightly test (without --release) passes. Full investigation in briansmith/untrusted#57.

The code in untrusted 0.8.0 (which I yanked) is not substantially different from the untrusted 0.7.1 so I'm afraid that ring (and all ring-based crates) relying on untrusted 0.7.1 to avoid this will not be a viable solution.

I think there is sufficient evidence here to warrant disabling the broken optimization for the beta. I am happy to continue minimizing the test case too.

[jyn514]

Marking this as needs-MCVE since we'll need it to fix the upstream bug, but I agree this alone is probably enough to get it reverted.