(Users of) untrusted is miscompiled in Rust 1.53 nightly since 2021-03-23 when LTO is enabled.

[briansmith]

These all fail (cargo 1.53.0-nightly (0ed318d18 2021-04-23):

cargo +nightly test --target=x86_64-unknown-linux-gnu --release ecdsa_from_pkcs8_test
cargo +nightly test --target=x86_64-pc-windows-msvc --release ecdsa_from_pkcs8_test
cargo +nightly test --target=i686-pc-windows-msvc --release ecdsa_from_pkcs8_test

These all succeed using the same nightly version, but not --release:

cargo +nightly test --target=i686-pc-windows-msvc ecdsa_from_pkcs8_test
cargo +nightly test --target=x86_64-pc-windows-msvc
cargo +1.51.0 test --target=i686-pc-windows-msvc --release ecdsa_from_pkcs8_test
cargo +beta test --target=i686-pc-windows-msvc --release ecdsa_from_pkcs8_test

ring's tests also all pass with the latest 0.7.1 release of untrusted before 0.8.0.

So the combination of untrusted 0.8.0 + ring main branch + nightly Rust seems to be broken.

[briansmith]

I yanked the 0.8.0 release of untrusted while I investigate this. You can use untrusted = { path = "../untrusted" } in ring's Cargo.toml to reproduce it.

[briansmith]

Rust nightly-2021-03-23 seems to have introduced a regression.

nightly-2021-03-23 fails:

$ export N=2021-03-23; rustup toolchain add nightly-$N --profile minimal && cargo +nightly-$N test --target=x86_64-unknown-linux-gnu --release ecdsa_from_pkcs8_test
$ export N=2021-03-23; rustup toolchain add nightly-$N --profile minimal && cargo +nightly-$N test --target=x86_64-pc-windows-msvc --release ecdsa_from_pkcs8_test

nightly-2021-03-22 succeeds:

export N=2021-03-22; rustup toolchain add nightly-$N --profile minimal && cargo +nightly-$N test --target=x86_64-unknown-linux-gnu --release ecdsa_from_pkcs8_test
export N=2021-03-22; rustup toolchain add nightly-$N --profile minimal && cargo +nightly-$N test --target=x86_64-pc-windows-msvc --release ecdsa_from_pkcs8_test

[briansmith]

With this minimized test case, cargo +nightly test --release fails and cargo +nightly test passes:

Cargo.toml:

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

[dependencies]
untrusted = { path = "../untrusted" }

[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>,
) -> Result<untrusted::Input<'a>, ()> {
    let _tag = input.read_byte();
    let _len = input.read_byte().unwrap();
    let value = input.read_bytes(1).unwrap();
    Ok(value)
}

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

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

        assert!(input.peek(0x30));
        input.skip_to_end();
        Ok(())
    });
}

Removing #[profile.bench] makes the test pass. Will continue minimizing.

[briansmith]

Here's the new minimized 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(())
    });
}

[briansmith]

I filed rust-lang/rust#84958 to track this bug in rustc.

[briansmith]

I filed rust-lang/rust#84958 to track this bug in rustc.

This ended up being a bug in rustc (LLVM) which was fixed for the 1.53 stable release. The rustc bug wasn't triggered by the untrusted tests, but it was triggered by at least one ring tests.

Ideally, we'd have a regression test in this repo based on the minimal repro I submitted in my rustc bug report. That would require this repo, AFAICT, to grow from one crate to two crates, which is inconvenient for me to do at this time.

[briansmith]

Closing this. Again, it was a bug in the Rust compiler, and again it's hard to write the regression test (AFAICT), so I'm not bothering.