From 192ad29a6fc93c89a828941c1b743d6f908abcc5 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 22 Jan 2026 03:02:02 +0100 Subject: [PATCH] capi: fix #[cfg(...)] soup for .symver Commit ba50ccdacae5 ("capi: use rustversion for symver") expanded the usage of .symver on architectures where global_asm! was stabilised after our MSRV to try to maximise our use of symbol versions. Unfortunately, the way that this was written actually caused us to disable symbol versions on all Rust versions post-1.72 (the first Rust version we enabled this support for). The solution is to swap the order so that #[rustversion::since] is the attribute being applied rather than #[cfg(target_arch = "...")] (which cannot be true more than once). While we're at it, add a CI test for this so we don't break it again in the future by accident. Fixes: ba50ccdacae5 ("capi: use rustversion for symver") Signed-off-by: Aleksa Sarai --- .github/workflows/rust.yml | 24 ++++++++- Makefile | 4 ++ hack/check-elf-symbols.sh | 99 ++++++++++++++++++++++++++++++++++++++ src/capi/utils.rs | 84 +++++++++++++++----------------- 4 files changed, 165 insertions(+), 46 deletions(-) create mode 100755 hack/check-elf-symbols.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 161b543..89a1a6b 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -23,7 +23,7 @@ on: name: rust-ci env: - RUST_MSRV: "1.63" + RUST_MSRV: &RUST_MSRV "1.63" jobs: codespell: @@ -52,7 +52,7 @@ jobs: - uses: actions/checkout@v6 - uses: dtolnay/rust-toolchain@master with: - toolchain: ${{ env.RUST_MSRV }} + toolchain: *RUST_MSRV - uses: taiki-e/install-action@cargo-hack - name: cargo check run: >- @@ -145,6 +145,25 @@ jobs: run: cargo install --force cbindgen - run: make validate-cbindgen + validate-elf-symbols: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + rust-version: + - *RUST_MSRV + - 1.72 # loongarch64 global_asm! stabilised + - 1.84 # arm64ex / s390x global_asm! stabilised + - 1.91 # loongarch32 global_asm! stabilised + - stable + - nightly + steps: + - uses: actions/checkout@v6 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ matrix.rust-version }} + - run: make validate-elf-symbols + rustdoc: name: cargo doc runs-on: ubuntu-latest @@ -478,6 +497,7 @@ jobs: - clippy - check-lint-nohack - validate-cbindgen + - validate-elf-symbols - rustdoc - doctest - nextest diff --git a/Makefile b/Makefile index 84fe948..9be9d6b 100644 --- a/Makefile +++ b/Makefile @@ -108,6 +108,10 @@ validate-cbindgen: exit 1 ; \ fi +.PHONY: validate-elf-symbols +validate-elf-symbols: release + ./hack/check-elf-symbols.sh ./target/release/libpathrs.so + .PHONY: test-rust-doctest test-rust-doctest: $(CARGO_LLVM_COV) --no-report --branch --doc diff --git a/hack/check-elf-symbols.sh b/hack/check-elf-symbols.sh new file mode 100755 index 0000000..9667a75 --- /dev/null +++ b/hack/check-elf-symbols.sh @@ -0,0 +1,99 @@ +#!/bin/bash +# SPDX-License-Identifier: MPL-2.0 OR LGPL-3.0-or-later +# +# libpathrs: safe path resolution on Linux +# Copyright (C) 2019-2025 SUSE LLC +# Copyright (C) 2026 Aleksa Sarai +# +# == MPL-2.0 == +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at https://mozilla.org/MPL/2.0/. +# +# Alternatively, this Source Code Form may also (at your option) be used +# under the terms of the GNU Lesser General Public License Version 3, as +# described below: +# +# == LGPL-3.0-or-later == +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or (at +# your option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY +# or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . + +# check-elf-symbols.sh -- make sure that libpathrs.so only contains pathrs_* +# symbols with versions included, to ensure we do not regress our symbol +# versioning again. + +set -Eeuo pipefail + +SRC_ROOT="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")" +ELF_FILE="$1" + +function bail() { + echo "ERROR:" "$@" >&2 + exit 1 +} + +# TODO: Do we actually want to also check the nodes referenced in src/capi/*? +# TODO: Also, once we make the old symbols optional this script will break. +readarray -t EXPECTED_VERSION_NODES < <( + grep -Eo "LIBPATHRS_[0-9]+(\.[0-9]+)+" "$SRC_ROOT/build.rs" | sort -u +) + +# Make sure that we at least have LIBPATHRS_0.[12]. +for node in LIBPATHRS_0.{1,2}; do + printf "%s\n" "${EXPECTED_VERSION_NODES[@]}" | grep -Fxq "$node" \ + || bail "$node node not found in build.rs" +done + +echo "== ELF SYMBOL VERSION NODES ==" + +for node in "${EXPECTED_VERSION_NODES[@]}"; do + readelf -WV "$ELF_FILE" | grep -Fwq "$node" || \ + bail "$node node not found in $ELF_FILE" + echo "$node node present in $ELF_FILE" +done + +echo "== SYMBOL VERSIONS SUMMARY ==" + +SYMBOL_OBJDUMP="$(objdump -T "$ELF_FILE" | grep -F "pathrs_")" + +[ -n "$SYMBOL_OBJDUMP" ] || bail "$ELF_FILE does not contain any pathrs_* symbols" +echo "$(wc -l <<<"$SYMBOL_OBJDUMP") pathrs symbols present in $ELF_FILE" + +for node in "${EXPECTED_VERSION_NODES[@]}"; do + awk -v NODE="$node" ' + BEGIN { num_symbols = 0 } + $(NF-1) == NODE { + symbols[$NF]++ + num_symbols++ + } + END { + print NODE, "symbols (" num_symbols, "total):" + if (num_symbols) { + for (sym in symbols) { + print " " sym + } + } + } + ' <<<"$SYMBOL_OBJDUMP" +done + +unversioned="$(awk '!($(NF-1) ~ /^LIBPATHRS_/)' <<<"$SYMBOL_OBJDUMP")" +[ -z "$unversioned" ] || { + echo "UNVERSIONED SYMBOLS ($(wc -l <<<"$unversioned") total):" + echo "$unversioned" + bail "$ELF_FILE contains unversioned symbols" +} + +echo "++ ALL SYMBOLS ARE VERSIONED! ++" diff --git a/src/capi/utils.rs b/src/capi/utils.rs index c007ebf..ab6b9a6 100644 --- a/src/capi/utils.rs +++ b/src/capi/utils.rs @@ -50,7 +50,8 @@ use libc::{c_char, c_int, size_t}; /// Generate `.symver` entries for a given function. /// -/// On platforms without stabilised +/// On platforms without stabilised `global_asm!` implementations, this macro is +/// a no-op but will eventually work once they are supported. /// /// ```ignore /// # use pathrs::capi::utils::symver; @@ -71,7 +72,7 @@ use libc::{c_char, c_int, size_t}; /// ``` macro_rules! symver { () => {}; - ($(#[$meta:meta])* fn $implsym:ident <- ($symname:ident, version = $version:literal); $($tail:tt)*) => { + (@with-meta $(#[$meta:meta])* $($block:tt)+) => { // Some architectures still have unstable ASM, which stops us from // injecting the ".symver" section. You can see the list in // LoweringContext::lower_inline_asm (compiler/rustc_asm_lowering). @@ -82,57 +83,52 @@ macro_rules! symver { target_arch = "x86_64", target_arch = "riscv32", target_arch = "riscv64", - //target_arch = "loongarch32", // MSRV(1.91?) + // These are only supported after our MSRV and have corresponding + // #[rustversion::since(1.XY)] tags below. + target_arch = "loongarch64", + target_arch = "arm64ec", + target_arch = "s390x", + target_arch = "loongarch32", // TODO: Once stabilised, add these arches: //target_arch = "powerpc", //target_arch = "powerpc64", //target_arch = "sparc64", ))] - #[::rustversion::attr(since(1.72), cfg(target_arch = "loongarch64"))] - #[::rustversion::attr(since(1.84), cfg(target_arch = "arm64ec"))] - #[::rustversion::attr(since(1.84), cfg(target_arch = "s390x"))] - // .symver $implsym, $symname@$version + #[cfg_attr(target_arch = "loongarch64", ::rustversion::since(1.72))] + #[cfg_attr(target_arch = "arm64ec", ::rustversion::since(1.84))] + #[cfg_attr(target_arch = "s390x", ::rustversion::since(1.84))] + #[cfg_attr(target_arch = "loongarch32", ::rustversion::since(1.91))] $(#[$meta])* - ::std::arch::global_asm! {concat!( - ".symver ", - stringify!($implsym), - ", ", - stringify!($symname), - "@", - $version, - )} + $($block)* + }; + ($(#[$meta:meta])* fn $implsym:ident <- ($symname:ident, version = $version:literal); $($tail:tt)*) => { + $crate::capi::utils::symver! { + @with-meta $(#[$meta])* + // .symver $implsym, $symname@$version + ::std::arch::global_asm! {concat!( + ".symver ", + stringify!($implsym), + ", ", + stringify!($symname), + "@", + $version, + )} + } $crate::capi::utils::symver! { $($tail)* } }; ($(#[$meta:meta])* fn $implsym:ident <- ($symname:ident, version = $version:literal, default); $($tail:tt)*) => { - // Some architectures still have unstable ASM, which stops us from - // injecting the ".symver" section. You can see the list in - // LoweringContext::lower_inline_asm (compiler/rustc_asm_lowering). - #[cfg(any( - target_arch = "arm", - target_arch = "aarch64", - target_arch = "x86", - target_arch = "x86_64", - target_arch = "riscv32", - target_arch = "riscv64", - // TODO: Once stabilised, add these arches: - //target_arch = "loongarch32", // MSRV(1.91?) - //target_arch = "powerpc", - //target_arch = "powerpc64", - //target_arch = "sparc64", - ))] - #[::rustversion::attr(since(1.72), cfg(target_arch = "loongarch64"))] - #[::rustversion::attr(since(1.84), cfg(target_arch = "arm64ec"))] - #[::rustversion::attr(since(1.84), cfg(target_arch = "s390x"))] - // .symver $implsym, $symname@@$version - $(#[$meta])* - ::std::arch::global_asm! {concat!( - ".symver ", - stringify!($implsym), - ", ", - stringify!($symname), - "@@", - $version, - )} + $crate::capi::utils::symver! { + @with-meta $(#[$meta])* + // .symver $implsym, $symname@@$version + ::std::arch::global_asm! {concat!( + ".symver ", + stringify!($implsym), + ", ", + stringify!($symname), + "@@", + $version, + )} + } $crate::capi::utils::symver! { $($tail)* } }; }