Support zstd-compressed ELF sections.

[khuey]

zstd has been introduced as an alternative to zlib for the compression of debug sections.[0] Toolchain support is widely present at this time but lack of support in backtrace is a severe limitation on using this feature in Rust programs.

Unlike zlib compression, the most widely used Rust library for zstd is a set of bindings to the zstd C library. There does exist a Rust reimplementation (ruzstd) however the author claims it is significantly slower than the C library and it sees substantially less usage, so I have not used it here.

Using the C library poses challenges because it requires compiling native code. I made a few adjustments to the CI configuration to make that possible on all platforms, and pinned the zstd crate to a slightly older version due to gyscos/zstd-rs#271 breaking the crate on wasm targets on the very latest version. Additionally, I've disabled this entirely on illumos rather than try to get native code to compile successfully there.

[0] https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections

[khuey]

Perhaps @jclulow would be interested in figuring out how to get the zstd crate to build on illumos.

[pfmooney]

Perhaps @jclulow would be interested in figuring out how to get the zstd crate to build on illumos.

Out of curiosity, what failure(s) are you seeing in the build?

[khuey]

Perhaps @jclulow would be interested in figuring out how to get the zstd crate to build on illumos.

Out of curiosity, what failure(s) are you seeing in the build?

It dies complaining that gar is not available.

[pfmooney]

It dies complaining that gar is not available.

Which distribution are you attempting this on? On OmniOSCE, gar appears to be provided by pkg:/developer/gnu-binutils

[khuey]

It dies complaining that gar is not available.

Which distribution are you attempting this on? On OmniOSCE, gar appears to be provided by pkg:/developer/gnu-binutils

Based on https://github.com/rust-lang/backtrace-rs/blob/master/.github/workflows/main.yml#L220, I suppose it's attempting a cross-compile from standard Ubuntu to illumos.

[bjorn3]

Unlike zlib compression, the most widely used Rust library for zstd is a set of bindings to the zstd C library. There does exist a Rust reimplementation (ruzstd) however the author claims it is significantly slower than the C library and it sees substantially less usage, so I have not used it here.

If the performance is still somewhat reasonable, I don't see it as too big of a deal to use it anyway. Backtrace symbolication is not on the performance critical path anyway (panics are supposed to be uncommon and only symbolize when RUST_BACKTRACE is set and errors with backtraces captured only symbolize when actually printed) and decompression should happen only once and cached for further symbolication queries.

[bjorn3]

I just saw that the libzstd C library is licensed as BSD-3-Clause OR GPL-2.0. GPL-2.0 is definitively out for a standard library dependency. As for BSD-3-Clause, the rust compiler has some dependencies that use it, but it isn't in tidy's list of permitted licenses for the standard library. https://github.com/rust-lang/rust/blob/9e297bf54d31eb3b30067208ff9af4416945a2ed/src/tools/tidy/src/deps.rs#L12-L33 I will ask on zulip if it is fine to add to the list of allowed licenses.

Edit: https://rust-lang.zulipchat.com/#narrow/stream/231349-t-core.2Flicensing/topic/zstd.20dependency.20for.20the.20standard.20library

[Mark-Simulacrum]

The README for ruzstd suggests a roughly 1.4x to 3.5x slowdown, and it seems significantly less complex to me -- it's really nice to have a pure-Rust standard library dependency tree, even on targets that usually have C compilers, for things like cross-compilation x.py check. I would be in favor of avoiding taking a dependency on real zstd in std, even if perhaps backtrace-rs does offer that (feature gated).

[luqmana]

It dies complaining that gar is not available.

Which distribution are you attempting this on? On OmniOSCE, gar appears to be provided by pkg:/developer/gnu-binutils

Based on https://github.com/rust-lang/backtrace-rs/blob/master/.github/workflows/main.yml#L220, I suppose it's attempting a cross-compile from standard Ubuntu to illumos.

Yea, compiling on illumos itself works fine but cross-compiling would require a suitable illumos sysroot. One way is the approach used in the Rust repo to build it in docker:

0001-ci-build-x86_64-unknown-illumos-in-docker.patch

```patch From 9d606a39eb9f342030d66a411262cb97a023da87 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Fri, 24 May 2024 15:37:07 -0700 Subject: [PATCH] ci: build x86_64-unknown-illumos in docker The zstd crate builds & links against the zstd c library. Crib the docker image from the Rust repo for the illumos target. --- .github/workflows/main.yml | 2 +- Cargo.toml | 2 - ci/docker/x86_64-unknown-illumos/Dockerfile | 34 ++++ ci/illumos-toolchain.sh | 177 ++++++++++++++++++++ crates/as-if-std/Cargo.toml | 2 - src/symbolize/gimli/elf.rs | 8 +- 6 files changed, 215 insertions(+), 10 deletions(-) create mode 100644 ci/docker/x86_64-unknown-illumos/Dockerfile create mode 100644 ci/illumos-toolchain.sh diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ca8a4b8..5309e76 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -210,6 +210,7 @@ jobs: - aarch64-linux-android - i686-linux-android - x86_64-linux-android + - x86_64-unknown-illumos steps: - uses: actions/checkout@v3 with: @@ -243,7 +244,6 @@ jobs: - wasm32-wasi - x86_64-unknown-fuchsia - x86_64-fortanix-unknown-sgx - - x86_64-unknown-illumos steps: - uses: actions/checkout@v3 with: diff --git a/Cargo.toml b/Cargo.toml index 72d062c..c9af1e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,8 +43,6 @@ cpp_demangle = { default-features = false, version = "0.4.0", optional = true, f miniz_oxide = { version = "0.7.0", default-features = false } addr2line = { version = "0.22.0", default-features = false } libc = { version = "0.2.146", default-features = false } - -[target.'cfg(not(any(all(windows, target_env = "msvc", not(target_vendor = "uwp")), target_os = "illumos")))'.dependencies] zstd = { version = "= 0.13.0", default-features = false } [target.'cfg(not(all(windows, target_env = "msvc", not(target_vendor = "uwp"))))'.dependencies.object] diff --git a/ci/docker/x86_64-unknown-illumos/Dockerfile b/ci/docker/x86_64-unknown-illumos/Dockerfile new file mode 100644 index 0000000..31381f3 --- /dev/null +++ b/ci/docker/x86_64-unknown-illumos/Dockerfile @@ -0,0 +1,34 @@ +FROM ubuntu:20.04 + +# Enable source repositories, which are disabled by default on Ubuntu >= 18.04 +RUN sed -i 's/^# deb-src/deb-src/' /etc/apt/sources.list + +RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ + automake \ + bison \ + bzip2 \ + ca-certificates \ + curl \ + flex \ + gawk \ + gcc \ + g++ \ + libc6-dev \ + libgmp-dev \ + libmpfr-dev \ + libmpc-dev \ + make \ + texinfo \ + xz-utils + +COPY illumos-toolchain.sh /tmp/ + +RUN bash /tmp/illumos-toolchain.sh x86_64 sysroot +RUN bash /tmp/illumos-toolchain.sh x86_64 binutils +RUN bash /tmp/illumos-toolchain.sh x86_64 gcc + +ENV CARGO_TARGET_X86_64_UNKNOWN_ILLUMOS_LINKER=x86_64-illumos-gcc \ + # TODO: should actually run these tests + CARGO_TARGET_X86_64_UNKNOWN_ILLUMOS_RUNNER=echo \ + CC=x86_64-illumos-gcc \ + AR=x86_64-illumos-ar \ No newline at end of file diff --git a/ci/illumos-toolchain.sh b/ci/illumos-toolchain.sh new file mode 100644 index 0000000..0b2c09b --- /dev/null +++ b/ci/illumos-toolchain.sh @@ -0,0 +1,177 @@ +#!/bin/bash + +set -o errexit +set -o pipefail +set -o xtrace + +ARCH="$1" +PHASE="$2" + +JOBS="$(getconf _NPROCESSORS_ONLN)" + +case "$ARCH" in +x86_64) + SYSROOT_MACH='i386' + ;; +*) + printf 'ERROR: unknown architecture: %s\n' "$ARCH" + exit 1 +esac + +BUILD_TARGET="$ARCH-pc-solaris2.10" + +# +# The illumos and the Solaris build both use the same GCC-level host triple, +# though different versions of GCC are used and with different configure +# options. To ensure as little accidental cross-pollination as possible, we +# build the illumos toolchain in a specific directory tree and just symlink the +# expected tools into /usr/local/bin at the end. We omit /usr/local/bin from +# PATH here for similar reasons. +# +PREFIX="/opt/illumos/$ARCH" +export PATH="$PREFIX/bin:/usr/bin:/bin:/usr/sbin:/sbin" + +# +# NOTE: The compiler version selected here is more specific than might appear. +# GCC 7.X releases do not appear to cross-compile correctly for Solaris +# targets, at least insofar as they refuse to enable TLS in libstdc++. When +# changing the GCC version in future, one must carefully verify that TLS is +# enabled in all of the static libraries we intend to include in output +# binaries. +# +GCC_VERSION='8.4.0' +GCC_SUM='e30a6e52d10e1f27ed55104ad233c30bd1e99cfb5ff98ab022dc941edd1b2dd4' +GCC_BASE="gcc-$GCC_VERSION" +GCC_TAR="gcc-$GCC_VERSION.tar.xz" +GCC_URL="https://ftp.gnu.org/gnu/gcc/$GCC_BASE/$GCC_TAR" + +SYSROOT_VER='20181213-de6af22ae73b-v1' +SYSROOT_SUM='ee792d956dfa6967453cebe9286a149143290d296a8ce4b8a91d36bea89f8112' +SYSROOT_TAR="illumos-sysroot-$SYSROOT_MACH-$SYSROOT_VER.tar.gz" +SYSROOT_URL='https://github.com/illumos/sysroot/releases/download/' +SYSROOT_URL+="$SYSROOT_VER/$SYSROOT_TAR" +SYSROOT_DIR="$PREFIX/sysroot" + +BINUTILS_VERSION='2.40' +BINUTILS_SUM='f8298eb153a4b37d112e945aa5cb2850040bcf26a3ea65b5a715c83afe05e48a' +BINUTILS_BASE="binutils-$BINUTILS_VERSION" +BINUTILS_TAR="$BINUTILS_BASE.tar.bz2" +BINUTILS_URL="https://ftp.gnu.org/gnu/binutils/$BINUTILS_TAR" + + +download_file() { + local file="$1" + local url="$2" + local sum="$3" + + while :; do + if [[ -f "$file" ]]; then + if ! h="$(sha256sum "$file" | awk '{ print $1 }')"; then + printf 'ERROR: reading hash\n' >&2 + exit 1 + fi + + if [[ "$h" == "$sum" ]]; then + return 0 + fi + + printf 'WARNING: hash mismatch: %s != expected %s\n' \ + "$h" "$sum" >&2 + rm -f "$file" + fi + + printf 'Downloading: %s\n' "$url" + if ! curl -f -L -o "$file" "$url"; then + rm -f "$file" + sleep 1 + fi + done +} + + +case "$PHASE" in +sysroot) + download_file "/tmp/$SYSROOT_TAR" "$SYSROOT_URL" "$SYSROOT_SUM" + mkdir -p "$SYSROOT_DIR" + cd "$SYSROOT_DIR" + tar -xzf "/tmp/$SYSROOT_TAR" + rm -f "/tmp/$SYSROOT_TAR" + ;; + +binutils) + download_file "/tmp/$BINUTILS_TAR" "$BINUTILS_URL" "$BINUTILS_SUM" + mkdir -p /ws/src/binutils + cd /ws/src/binutils + tar -xjf "/tmp/$BINUTILS_TAR" + rm -f "/tmp/$BINUTILS_TAR" + + mkdir -p /ws/build/binutils + cd /ws/build/binutils + "/ws/src/binutils/$BINUTILS_BASE/configure" \ + --prefix="$PREFIX" \ + --target="$BUILD_TARGET" \ + --program-prefix="$ARCH-illumos-" \ + --with-sysroot="$SYSROOT_DIR" + + make -j "$JOBS" + + mkdir -p "$PREFIX" + make install + + cd / + rm -rf /ws/src/binutils /ws/build/binutils + ;; + +gcc) + download_file "/tmp/$GCC_TAR" "$GCC_URL" "$GCC_SUM" + mkdir -p /ws/src/gcc + cd /ws/src/gcc + tar -xJf "/tmp/$GCC_TAR" + rm -f "/tmp/$GCC_TAR" + + mkdir -p /ws/build/gcc + cd /ws/build/gcc + export CFLAGS='-fPIC' + export CXXFLAGS='-fPIC' + export CXXFLAGS_FOR_TARGET='-fPIC' + export CFLAGS_FOR_TARGET='-fPIC' + "/ws/src/gcc/$GCC_BASE/configure" \ + --prefix="$PREFIX" \ + --target="$BUILD_TARGET" \ + --program-prefix="$ARCH-illumos-" \ + --with-sysroot="$SYSROOT_DIR" \ + --with-gnu-as \ + --with-gnu-ld \ + --disable-nls \ + --disable-libgomp \ + --disable-libquadmath \ + --disable-libssp \ + --disable-libvtv \ + --disable-libcilkrts \ + --disable-libada \ + --disable-libsanitizer \ + --disable-libquadmath-support \ + --disable-shared \ + --enable-tls + + make -j "$JOBS" + + mkdir -p "$PREFIX" + make install + + # + # Link toolchain commands into /usr/local/bin so that cmake and others + # can find them: + # + (cd "$PREFIX/bin" && ls -U) | grep "^$ARCH-illumos-" | + xargs -t -I% ln -s "$PREFIX/bin/%" '/usr/local/bin/' + + cd / + rm -rf /ws/src/gcc /ws/build/gcc + ;; + +*) + printf 'ERROR: unknown phase "%s"\n' "$PHASE" >&2 + exit 100 + ;; +esac diff --git a/crates/as-if-std/Cargo.toml b/crates/as-if-std/Cargo.toml index 488fffd..14d60f6 100644 --- a/crates/as-if-std/Cargo.toml +++ b/crates/as-if-std/Cargo.toml @@ -19,8 +19,6 @@ libc = { version = "0.2.146", default-features = false } [target.'cfg(not(all(windows, target_env = "msvc", not(target_vendor = "uwp"))))'.dependencies] miniz_oxide = { version = "0.7.0", optional = true, default-features = false } addr2line = { version = "0.22.0", optional = true, default-features = false } - -[target.'cfg(not(any(all(windows, target_env = "msvc", not(target_vendor = "uwp")), target_os = "illumos")))'.dependencies] zstd = { version = "= 0.13.0", optional = true, default-features = false } [target.'cfg(not(all(windows, target_env = "msvc", not(target_vendor = "uwp"))))'.dependencies.object] diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index 20f2d3f..8c64a90 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -7,9 +7,9 @@ use super::{gimli, Context, Endian, EndianSlice, Mapping, Stash, Vec}; use alloc::sync::Arc; use core::convert::{TryFrom, TryInto}; use core::str; -#[cfg(not(target_os = "illumos"))] -use object::elf::ELFCOMPRESS_ZSTD; -use object::elf::{ELFCOMPRESS_ZLIB, ELF_NOTE_GNU, NT_GNU_BUILD_ID, SHF_COMPRESSED}; +use object::elf::{ + ELFCOMPRESS_ZLIB, ELFCOMPRESS_ZSTD, ELF_NOTE_GNU, NT_GNU_BUILD_ID, SHF_COMPRESSED, +}; use object::read::elf::{CompressionHeader, FileHeader, SectionHeader, SectionTable, Sym}; use object::read::StringTable; use object::{BigEndian, Bytes, NativeEndian}; @@ -188,7 +188,6 @@ impl<'a> Object<'a> { decompress_zlib(data.0, buf)?; return Some(buf); } - #[cfg(not(target_os = "illumos"))] ELFCOMPRESS_ZSTD => { let size = usize::try_from(header.ch_size(self.endian)).ok()?; let buf = stash.allocate(size); @@ -315,7 +314,6 @@ fn decompress_zlib(input: &[u8], output: &mut [u8]) -> Option<()> { } } -#[cfg(not(target_os = "illumos"))] fn decompress_zstd(input: &[u8], output: &mut [u8]) -> Option<()> { zstd::stream::copy_decode(input, output).ok() } -- 2.25.1 ```

But that does increase overall ci time having to build gcc & friends. A pure rust solution would simplify things.

[khuey]

There's an alternative PR using a Rust reimplementation of zstd (the ruzstd crate) in #626. Pick your poison 😄

[workingjubilee]

I have been very vocal about not significantly raising the maintenance burden of this crate, so it would be out-of-character for me to accept this. I will review #626.