Miscompilation with LLVM 10 and 11 with codegen-units=1

[ehuss]

As part of the bootstrap bump in #90042, we are running into an issue where the x86_64-gnu-llvm-10 CI test is failing due to some miscompilation of some code.

The bump in the bootstrap brings in a change in cargo where tests now inherit profile settings from the dev/release profiles. The standard library tests are now being built with codegen-units=1 due to this. This has unearthed a miscompilation with llvm-10. In my tests, llvm-11 also seems to have the same problem, but llvm-12 seems to work correctly.

The test that seems to be failing is test_copy_within. Somehow the optimizations are causing it to compile down to an unconditional call to the first assert_eq being a failure.

This can be reproduced on latest master with llvm-10-dev installed on Ubuntu 20:

# llvm-10 is installed with `apt install llvm-10-dev`
# Also fails with llvm-11, but not llvm-12
./configure --llvm-root=/usr/lib/llvm-10 --enable-llvm-link-shared
./x.py build library/std

cat <<EOF >> foo.rs
fn main() {
    let mut bytes = *b"Hello, World!";
    bytes.copy_within(..3, 10);
    assert_eq!(&bytes, b"Hello, WorHel");
}
EOF

./build/x86_64-unknown-linux-gnu/stage1/bin/rustc foo.rs -Copt-level=3 -Ccodegen-units=1
./foo

[ehuss]

One option is to disable codegen-units=1 for that builder only, would could be roughly something like this:

diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-10/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-10/Dockerfile
index c34198708c4..e5143930270 100644
--- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-10/Dockerfile
+++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-10/Dockerfile
@@ -32,6 +32,10 @@ ENV RUST_CONFIGURE_ARGS \
       --enable-llvm-link-shared \
       --set rust.thin-lto-import-instr-limit=10

+# Due to a miscompilation with codegen-units=1 with llvm-10 and 11, switch to
+# the default here. This can be removed when updating to llvm-12.
+ENV DEFAULT_STD_CGU=1
+
 ENV SCRIPT python2.7 ../x.py --stage 2 test --exclude src/tools/tidy && \
            # Run the `mir-opt` tests again but this time for a 32-bit target.
            # This enforces that tests using `// EMIT_MIR_FOR_EACH_BIT_WIDTH` have
diff --git a/src/ci/run.sh b/src/ci/run.sh
index b5019d83af4..e5256e32b71 100755
--- a/src/ci/run.sh
+++ b/src/ci/run.sh
@@ -51,7 +51,9 @@ RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-sccache"
 RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --disable-manage-submodules"
 RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-locked-deps"
 RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-cargo-native-static"
-RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-units-std=1"
+if [ "$DEFAULT_STD_CGU" = ""]; then
+  RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.codegen-units-std=1"
+fi

 # Only produce xz tarballs on CI. gz tarballs will be generated by the release
 # process by recompressing the existing xz ones. This decreases the storage

[nagisa]

LLVM 12 is the minimum supported version now, so this is resolved?

[Mark-Simulacrum]

Yeah, I think so -- closing.