Rollup merge of #128935 - lqd:needs-zstd, r=Kobzol

More work on `zstd` compression

r? ``@Kobzol`` as we've discussed this.

This is a draft to show the current approach of supporting zstd in compiletest, and making the tests using it unconditional.

Knowing whether llvm/lld was built with `LLVM_ENABLE_ZSTD` is quite hard, so there are two strategies. There are details in the code, and we can discuss this approach. Until we know the config used to build CI artifacts, it seems our options are somewhat limited in any case.

zlib compression seems always enabled, so we only check this in its dedicated test, allowing the test to ignore errors due to zstd not being supported.

The zstd test is made unconditional in what it tests, by relying on `needs-llvm-zstd` to be ignored when `llvm.libzstd` isn't enabled in `config.toml`.

try-job: x86_64-gnu
try-job: x86_64-msvc
try-job: x86_64-gnu-distcheck

10 files changed, 175 insertions, 46 deletions

diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs
index ce23b7735f8..bdfee55d8d1 100644
--- a/src/bootstrap/src/core/config/config.rs
+++ b/src/bootstrap/src/core/config/config.rs
@@ -1885,6 +1885,22 @@ impl Config {
                     warn("link-shared");
                 }
 
+                // FIXME(#129153): instead of all the ad-hoc `download-ci-llvm` checks that follow,
+                // use the `builder-config` present in tarballs since #128822 to compare the local
+                // config to the ones used to build the LLVM artifacts on CI, and only notify users
+                // if they've chosen a different value.
+
+                if libzstd.is_some() {
+                    println!(
+                        "WARNING: when using `download-ci-llvm`, the local `llvm.libzstd` option, \
+                        like almost all `llvm.*` options, will be ignored and set by the LLVM CI \
+                        artifacts builder config."
+                    );
+                    println!(
+                        "HELP: To use `llvm.libzstd` for LLVM/LLD builds, set `download-ci-llvm` option to false."
+                    );
+                }
+
                 // None of the LLVM options, except assertions, are supported
                 // when using downloaded LLVM. We could just ignore these but
                 // that's potentially confusing, so force them to not be
@@ -1894,7 +1910,6 @@ impl Config {
                 check_ci_llvm!(optimize_toml);
                 check_ci_llvm!(thin_lto);
                 check_ci_llvm!(release_debuginfo);
-                check_ci_llvm!(libzstd);
                 check_ci_llvm!(targets);
                 check_ci_llvm!(experimental_targets);
                 check_ci_llvm!(clang_cl);
diff --git a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile
index 61e9694f1e2..e857f38e68a 100644
--- a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile
+++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile
@@ -62,9 +62,9 @@ COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/
 RUN ./build-clang.sh
 ENV CC=clang CXX=clang++
 
-# rustc's LLVM needs zstd.
-COPY scripts/zstd.sh /tmp/
-RUN ./zstd.sh
+# Build zstd to enable `llvm.libzstd`.
+COPY host-x86_64/dist-x86_64-linux/build-zstd.sh /tmp/
+RUN ./build-zstd.sh
 
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh
diff --git a/src/ci/docker/scripts/zstd.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh
index a3d37ccc311..a3d37ccc311 100755
--- a/src/ci/docker/scripts/zstd.sh
+++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh
diff --git a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile
index 19683317126..83c2aa8cfb3 100644
--- a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile
+++ b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile
@@ -28,5 +28,6 @@ ENV RUST_CONFIGURE_ARGS \
  --build=x86_64-unknown-linux-gnu \
  --enable-sanitizers \
  --enable-profiler \
- --enable-compiler-docs
+ --enable-compiler-docs \
+ --set llvm.libzstd=true
 ENV SCRIPT python3 ../x.py --stage 2 test
diff --git a/src/tools/compiletest/src/command-list.rs b/src/tools/compiletest/src/command-list.rs
index 7f8080235c8..a559d6f81a2 100644
--- a/src/tools/compiletest/src/command-list.rs
+++ b/src/tools/compiletest/src/command-list.rs
@@ -141,6 +141,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "needs-force-clang-based-tests",
     "needs-git-hash",
     "needs-llvm-components",
+    "needs-llvm-zstd",
     "needs-profiler-support",
     "needs-relocation-model-pic",
     "needs-run-enabled",
diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs
index 1fc24301c85..933913eb47c 100644
--- a/src/tools/compiletest/src/header.rs
+++ b/src/tools/compiletest/src/header.rs
@@ -1203,6 +1203,107 @@ pub fn extract_llvm_version_from_binary(binary_path: &str) -> Option<u32> {
     None
 }
 
+/// For tests using the `needs-llvm-zstd` directive:
+/// - for local LLVM builds, try to find the static zstd library in the llvm-config system libs.
+/// - for `download-ci-llvm`, see if `lld` was built with zstd support.
+pub fn llvm_has_libzstd(config: &Config) -> bool {
+    // Strategy 1: works for local builds but not with `download-ci-llvm`.
+    //
+    // We check whether `llvm-config` returns the zstd library. Bootstrap's `llvm.libzstd` will only
+    // ask to statically link it when building LLVM, so we only check if the list of system libs
+    // contains a path to that static lib, and that it exists.
+    //
+    // See compiler/rustc_llvm/build.rs for more details and similar expectations.
+    fn is_zstd_in_config(llvm_bin_dir: &Path) -> Option<()> {
+        let llvm_config_path = llvm_bin_dir.join("llvm-config");
+        let output = Command::new(llvm_config_path).arg("--system-libs").output().ok()?;
+        assert!(output.status.success(), "running llvm-config --system-libs failed");
+
+        let libs = String::from_utf8(output.stdout).ok()?;
+        for lib in libs.split_whitespace() {
+            if lib.ends_with("libzstd.a") && Path::new(lib).exists() {
+                return Some(());
+            }
+        }
+
+        None
+    }
+
+    // Strategy 2: `download-ci-llvm`'s `llvm-config --system-libs` will not return any libs to
+    // use.
+    //
+    // The CI artifacts also don't contain the bootstrap config used to build them: otherwise we
+    // could have looked at the `llvm.libzstd` config.
+    //
+    // We infer whether `LLVM_ENABLE_ZSTD` was used to build LLVM as a byproduct of testing whether
+    // `lld` supports it. If not, an error will be emitted: "LLVM was not built with
+    // LLVM_ENABLE_ZSTD or did not find zstd at build time".
+    #[cfg(unix)]
+    fn is_lld_built_with_zstd(llvm_bin_dir: &Path) -> Option<()> {
+        let lld_path = llvm_bin_dir.join("lld");
+        if lld_path.exists() {
+            // We can't call `lld` as-is, it expects to be invoked by a compiler driver using a
+            // different name. Prepare a temporary symlink to do that.
+            let lld_symlink_path = llvm_bin_dir.join("ld.lld");
+            if !lld_symlink_path.exists() {
+                std::os::unix::fs::symlink(lld_path, &lld_symlink_path).ok()?;
+            }
+
+            // Run `lld` with a zstd flag. We expect this command to always error here, we don't
+            // want to link actual files and don't pass any.
+            let output = Command::new(&lld_symlink_path)
+                .arg("--compress-debug-sections=zstd")
+                .output()
+                .ok()?;
+            assert!(!output.status.success());
+
+            // Look for a specific error caused by LLVM not being built with zstd support. We could
+            // also look for the "no input files" message, indicating the zstd flag was accepted.
+            let stderr = String::from_utf8(output.stderr).ok()?;
+            let zstd_available = !stderr.contains("LLVM was not built with LLVM_ENABLE_ZSTD");
+
+            // We don't particularly need to clean the link up (so the previous commands could fail
+            // in theory but won't in practice), but we can try.
+            std::fs::remove_file(lld_symlink_path).ok()?;
+
+            if zstd_available {
+                return Some(());
+            }
+        }
+
+        None
+    }
+
+    #[cfg(not(unix))]
+    fn is_lld_built_with_zstd(_llvm_bin_dir: &Path) -> Option<()> {
+        None
+    }
+
+    if let Some(llvm_bin_dir) = &config.llvm_bin_dir {
+        // Strategy 1: for local LLVM builds.
+        if is_zstd_in_config(llvm_bin_dir).is_some() {
+            return true;
+        }
+
+        // Strategy 2: for LLVM artifacts built on CI via `download-ci-llvm`.
+        //
+        // It doesn't work for cases where the artifacts don't contain the linker, but it's
+        // best-effort: CI has `llvm.libzstd` and `lld` enabled on the x64 linux artifacts, so it
+        // will at least work there.
+        //
+        // If this can be improved and expanded to less common cases in the future, it should.
+        if config.target == "x86_64-unknown-linux-gnu"
+            && config.host == config.target
+            && is_lld_built_with_zstd(llvm_bin_dir).is_some()
+        {
+            return true;
+        }
+    }
+
+    // Otherwise, all hope is lost.
+    false
+}
+
 /// Takes a directive of the form "<version1> [- <version2>]",
 /// returns the numeric representation of <version1> and <version2> as
 /// tuple: (<version1> as u32, <version2> as u32)
diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs
index 8f935d5b744..72b1b9c6d48 100644
--- a/src/tools/compiletest/src/header/needs.rs
+++ b/src/tools/compiletest/src/header/needs.rs
@@ -1,5 +1,5 @@
 use crate::common::{Config, Sanitizer};
-use crate::header::IgnoreDecision;
+use crate::header::{llvm_has_libzstd, IgnoreDecision};
 
 pub(super) fn handle_needs(
     cache: &CachedNeedsConditions,
@@ -144,6 +144,11 @@ pub(super) fn handle_needs(
             condition: cache.symlinks,
             ignore_reason: "ignored if symlinks are unavailable",
         },
+        Need {
+            name: "needs-llvm-zstd",
+            condition: cache.llvm_zstd,
+            ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression",
+        },
     ];
 
     let (name, comment) = match ln.split_once([':', ' ']) {
@@ -169,7 +174,7 @@ pub(super) fn handle_needs(
             } else {
                 return IgnoreDecision::Ignore {
                     reason: if let Some(comment) = comment {
-                        format!("{} ({comment})", need.ignore_reason)
+                        format!("{} ({})", need.ignore_reason, comment.trim())
                     } else {
                         need.ignore_reason.into()
                     },
@@ -210,6 +215,8 @@ pub(super) struct CachedNeedsConditions {
     rust_lld: bool,
     dlltool: bool,
     symlinks: bool,
+    /// Whether LLVM built with zstd, for the `needs-llvm-zstd` directive.
+    llvm_zstd: bool,
 }
 
 impl CachedNeedsConditions {
@@ -253,6 +260,7 @@ impl CachedNeedsConditions {
                 .join(if config.host.contains("windows") { "rust-lld.exe" } else { "rust-lld" })
                 .exists(),
 
+            llvm_zstd: llvm_has_libzstd(&config),
             dlltool: find_dlltool(&config),
             symlinks: has_symlinks(),
         }
diff --git a/tests/run-make/rust-lld-compress-debug-sections/main.rs b/tests/run-make/compressed-debuginfo-zstd/main.rs
index f328e4d9d04..f328e4d9d04 100644
--- a/tests/run-make/rust-lld-compress-debug-sections/main.rs
+++ b/tests/run-make/compressed-debuginfo-zstd/main.rs
diff --git a/tests/run-make/compressed-debuginfo-zstd/rmake.rs b/tests/run-make/compressed-debuginfo-zstd/rmake.rs
new file mode 100644
index 00000000000..8356373e949
--- /dev/null
+++ b/tests/run-make/compressed-debuginfo-zstd/rmake.rs
@@ -0,0 +1,42 @@
+// Checks debuginfo compression both for the always-enabled zlib, and when the optional zstd is
+// enabled:
+// - via rustc's `debuginfo-compression`,
+// - and via rust-lld's `compress-debug-sections`
+
+//@ needs-llvm-zstd: we want LLVM/LLD to be built with zstd support
+//@ needs-rust-lld: the system linker will most likely not support zstd
+//@ only-linux
+//@ ignore-cross-compile
+
+use run_make_support::{llvm_readobj, run_in_tmpdir, Rustc};
+
+fn check_compression(compression: &str, to_find: &str) {
+    // check compressed debug sections via rustc flag
+    prepare_and_check(to_find, |rustc| {
+        rustc.arg(&format!("-Zdebuginfo-compression={compression}"))
+    });
+
+    // check compressed debug sections via rust-lld flag
+    prepare_and_check(to_find, |rustc| {
+        rustc.link_arg(&format!("-Wl,--compress-debug-sections={compression}"))
+    });
+}
+
+fn prepare_and_check<F: FnOnce(&mut Rustc) -> &mut Rustc>(to_find: &str, prepare_rustc: F) {
+    run_in_tmpdir(|| {
+        let mut rustc = Rustc::new();
+        rustc
+            .arg("-Zlinker-features=+lld")
+            .arg("-Clink-self-contained=+linker")
+            .arg("-Zunstable-options")
+            .arg("-Cdebuginfo=full")
+            .input("main.rs");
+        prepare_rustc(&mut rustc).run();
+        llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find);
+    });
+}
+
+fn main() {
+    check_compression("zlib", "ZLIB");
+    check_compression("zstd", "ZSTD");
+}
diff --git a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs b/tests/run-make/rust-lld-compress-debug-sections/rmake.rs
deleted file mode 100644
index ea4997fab80..00000000000
--- a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs
+++ /dev/null
@@ -1,39 +0,0 @@
-// Checks the `compress-debug-sections` option on rust-lld.
-
-//@ needs-rust-lld
-//@ only-linux
-//@ ignore-cross-compile
-
-// FIXME: This test isn't comprehensive and isn't covering all possible combinations.
-
-use run_make_support::{assert_contains, llvm_readobj, run_in_tmpdir, rustc};
-
-fn check_compression(compression: &str, to_find: &str) {
-    run_in_tmpdir(|| {
-        let out = rustc()
-            .arg("-Zlinker-features=+lld")
-            .arg("-Clink-self-contained=+linker")
-            .arg("-Zunstable-options")
-            .arg("-Cdebuginfo=full")
-            .link_arg(&format!("-Wl,--compress-debug-sections={compression}"))
-            .input("main.rs")
-            .run_unchecked();
-        let stderr = out.stderr_utf8();
-        if stderr.is_empty() {
-            llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find);
-        } else {
-            assert_contains(
-                stderr,
-                format!(
-                    "LLVM was not built with LLVM_ENABLE_{to_find} \
-                     or did not find {compression} at build time"
-                ),
-            );
-        }
-    });
-}
-
-fn main() {
-    check_compression("zlib", "ZLIB");
-    check_compression("zstd", "ZSTD");
-}