avoid letting compiler-builtins' weak math definitions shadow system libm

Author: cezarbbb

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1,3 +1,4 @@
+// ignore-tidy-file-filelength
 mod raw_dylib;
 
 use std::collections::BTreeSet;
@@ -751,6 +752,72 @@ fn link_rlib<'a>(
 ///
 /// There's no need to include metadata in a static archive, so ensure to not link in the metadata
 /// object file (and also don't prepare the archive with a metadata file).
+// Weak f32/f64 math symbols from c-b's `full_availability` module; keep in sync with
+// `compiler-builtins/src/math/mod.rs`.
+const COMPILER_BUILTINS_LIBM_SYMBOLS: &[&str] = &[
+    "cbrtf",
+    "ceilf",
+    "copysignf",
+    "fabsf",
+    "fdimf",
+    "floorf",
+    "fmaf",
+    "fmaxf",
+    "fminf",
+    "fmodf",
+    "rintf",
+    "roundf",
+    "sqrtf",
+    "truncf",
+    "cbrt",
+    "ceil",
+    "copysign",
+    "fabs",
+    "fdim",
+    "floor",
+    "fma",
+    "fmax",
+    "fmin",
+    "fmod",
+    "rint",
+    "round",
+    "sqrt",
+    "trunc",
+];
+
+fn compiler_builtins_libm_members(rlib_path: &Path) -> FxHashSet<String> {
+    let Ok(file) = File::open(rlib_path) else { return FxHashSet::default() };
+    let Ok(mmap) = (unsafe { Mmap::map(file) }) else { return FxHashSet::default() };
+    let Ok(archive) = object::read::archive::ArchiveFile::parse(&*mmap) else {
+        return FxHashSet::default();
+    };
+    let Some(symbols) = archive.symbols().ok().flatten() else { return FxHashSet::default() };
+
+    let mut members = FxHashSet::default();
+    for symbol in symbols {
+        let Ok(symbol) = symbol else { continue };
+        if !COMPILER_BUILTINS_LIBM_SYMBOLS.iter().any(|&name| name.as_bytes() == symbol.name()) {
+            continue;
+        }
+        if let Ok(member) = archive.member(symbol.offset())
+            && let Ok(name) = str::from_utf8(member.name())
+        {
+            members.insert(name.to_string());
+        }
+    }
+    members
+}
+
+// Whether a system libm (`-lm`) is among the native libraries recorded for the final link.
+fn links_libm(crate_info: &CrateInfo, sess: &Session) -> bool {
+    crate_info
+        .native_libraries
+        .values()
+        .chain(std::iter::once(&crate_info.used_libraries))
+        .flatten()
+        .any(|lib| lib.name.as_str() == "m" && lib.kind.is_dllimport() && relevant_lib(sess, lib))
+}
+
 fn link_staticlib(
     sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
@@ -773,7 +840,15 @@ fn link_staticlib(
     );
     let mut all_native_libs = vec![];
 
+    // With `-lm` recorded, drop c-b's weak math definitions so libm's strong ones win (#142119).
+    let links_libm = links_libm(crate_info, sess);
     let res = each_linked_rlib(crate_info, Some(CrateType::StaticLib), &mut |cnum, path| {
+        debug_assert!(
+            !links_libm
+                || crate_info.compiler_builtins != Some(cnum)
+                || ignored_for_lto(sess, crate_info, cnum),
+            "compiler-builtins must not participate in LTO for the omit-libm skip to hold"
+        );
         let lto = are_upstream_rust_objects_already_included(sess)
             && !ignored_for_lto(sess, crate_info, cnum);
 
@@ -792,6 +867,11 @@ fn link_staticlib(
             .enumerate()
             .filter_map(|(i, _)| bundled_filenames.get(i).copied().flatten())
             .collect();
+        let cb_libm_members = if links_libm && crate_info.compiler_builtins == Some(cnum) {
+            compiler_builtins_libm_members(path)
+        } else {
+            FxHashSet::default()
+        };
         ab.add_archive(
             path,
             AddArchiveKind::Rlib(rmeta_link_cache, &|fname: &str, entry_kind| {
@@ -810,6 +890,9 @@ fn link_staticlib(
                     return true;
                 }
 
+                if cb_libm_members.contains(fname) {
+                    return true;
+                }
                 false
             }),
         )
@@ -3036,6 +3119,12 @@ fn linker_with_args(
         cmd.link_arg(std::path::absolute(&*sess.target_tlib_path.dir).unwrap());
     }
 
+    // Emit `-lm` ahead of the rlibs on glibc so its strong definitions win over c-b's weak ones;
+    // every other native dylib stays after the rlibs below.
+    if sess.target.env == Env::Gnu && links_libm(crate_info, sess) {
+        cmd.link_dylib_by_name("m", false, true);
+    }
+
     // Upstream rust crates and their non-dynamic native libraries.
     add_upstream_rust_crates(
         cmd,

tests/run-make/staticlib-omit-builtins-libm/lib.rs

@@ -0,0 +1,24 @@
+#![no_std]
+#![crate_type = "staticlib"]
+
+use core::panic::PanicInfo;
+
+#[panic_handler]
+fn panic(_: &PanicInfo) -> ! {
+    loop {}
+}
+
+// Declaring `ceilf`/`sqrtf` pulls compiler-builtins' weak definitions into the archive, while
+// `#[link(name = "m")]` makes the staticlib record system libm (`-lm`) as a dependency. When
+// libm is recorded, rustc must skip those weak definitions so that a later link against `-lm`
+// resolves the strong libm versions (rust-lang/rust#142119).
+#[link(name = "m")]
+extern "C" {
+    fn ceilf(x: f32) -> f32;
+    fn sqrtf(x: f32) -> f32;
+}
+
+#[no_mangle]
+pub extern "C" fn use_mathf(x: f32) -> f32 {
+    unsafe { ceilf(x) + sqrtf(x) }
+}

tests/run-make/staticlib-omit-builtins-libm/no_libm.rs

@@ -0,0 +1,21 @@
+#![no_std]
+#![crate_type = "staticlib"]
+
+use core::panic::PanicInfo;
+
+#[panic_handler]
+fn panic(_: &PanicInfo) -> ! {
+    loop {}
+}
+
+// No `#[link(name = "m")]`: without a system libm recorded, compiler-builtins' weak f32/f64 math
+// definitions are the only provider of `ceilf`/`sqrtf`, so they must be kept.
+extern "C" {
+    fn ceilf(x: f32) -> f32;
+    fn sqrtf(x: f32) -> f32;
+}
+
+#[no_mangle]
+pub extern "C" fn use_mathf(x: f32) -> f32 {
+    unsafe { ceilf(x) + sqrtf(x) }
+}

tests/run-make/staticlib-omit-builtins-libm/rmake.rs

@@ -0,0 +1,139 @@
+// compiler-builtins weakly defines the f32/f64 math symbols (its `full_availability` list) so
+// `no_std` targets without a system libm still work. A staticlib that records `-lm` must omit
+// them so libm's strong definitions win at the final link (#142119); the no-`-lm` control keeps
+// its weak fallbacks. `only-gnu` because only gnu libm (glibc) provides these symbols.
+
+//@ only-gnu
+//@ ignore-cross-compile
+
+use std::collections::HashSet;
+
+use run_make_support::object::read::archive::ArchiveFile;
+use run_make_support::object::read::elf::{FileHeader as _, SectionHeader as _, Sym as _};
+use run_make_support::object::{Endianness, elf};
+use run_make_support::{rfs, rustc, static_lib_name};
+
+// Keep in sync with `COMPILER_BUILTINS_LIBM_SYMBOLS` in rustc_codegen_ssa/src/back/link.rs.
+const LIBM_SYMBOLS: &[&str] = &[
+    "cbrtf",
+    "ceilf",
+    "copysignf",
+    "fabsf",
+    "fdimf",
+    "floorf",
+    "fmaf",
+    "fmaxf",
+    "fminf",
+    "fmodf",
+    "rintf",
+    "roundf",
+    "sqrtf",
+    "truncf",
+    "cbrt",
+    "ceil",
+    "copysign",
+    "fabs",
+    "fdim",
+    "floor",
+    "fma",
+    "fmax",
+    "fmin",
+    "fmod",
+    "rint",
+    "round",
+    "sqrt",
+    "trunc",
+];
+
+fn main() {
+    rustc().input("no_libm.rs").crate_type("staticlib").panic("abort").run();
+    let no_libm = static_lib_name("no_libm");
+    let no_libm_defined = defined_global_symbols(&no_libm);
+    for name in LIBM_SYMBOLS {
+        assert!(
+            no_libm_defined.contains(*name),
+            "expected weak fallback `{name}` to be kept without `-lm` in `{}`",
+            no_libm
+        );
+    }
+
+    rustc().input("lib.rs").crate_type("staticlib").panic("abort").run();
+    let with_libm = static_lib_name("lib");
+    let with_libm_defined = defined_global_symbols(&with_libm);
+    for name in LIBM_SYMBOLS {
+        assert!(
+            !with_libm_defined.contains(*name),
+            "weak definition `{name}` should be omitted when `-lm` is recorded in `{with_libm}`"
+        );
+    }
+
+    // Not a blanket strip: the f16/f128 and integer fallbacks survive. `ceilf16` also guards
+    // against matching `ceilf` as a prefix of `ceilf16`.
+    assert!(
+        with_libm_defined.contains("ceilf16"),
+        "compiler-builtins' f16 fallback must survive the omit in `{with_libm}`"
+    );
+    assert!(
+        with_libm_defined.contains("__floatdidf"),
+        "compiler-builtins' integer fallback must survive the omit in `{with_libm}`"
+    );
+}
+
+/// Every defined global/weak symbol in the archive. The omitted symbols can still appear as
+/// undefined references (e.g. `use_mathf` calling `ceilf`), so filter `SHN_UNDEF` rather than
+/// checking mere presence.
+fn defined_global_symbols(archive_path: &str) -> HashSet<String> {
+    let archive_data = rfs::read(archive_path);
+    let archive = ArchiveFile::parse(archive_data.as_slice()).unwrap();
+    let mut defined = HashSet::new();
+
+    for member in archive.members() {
+        let member = member.unwrap();
+        let data = member.data(archive_data.as_slice()).unwrap();
+
+        if let Ok(header) = elf::FileHeader64::<Endianness>::parse(data) {
+            collect_elf_defined(header, data, &mut defined);
+        } else if let Ok(header) = elf::FileHeader32::<Endianness>::parse(data) {
+            collect_elf_defined(header, data, &mut defined);
+        }
+    }
+
+    defined
+}
+
+fn collect_elf_defined<
+    Elf: run_make_support::object::read::elf::FileHeader<Endian = Endianness>,
+>(
+    header: &Elf,
+    data: &[u8],
+    defined: &mut HashSet<String>,
+) {
+    let Ok(endian) = header.endian() else { return };
+    let Ok(sections) = header.sections(endian, data) else { return };
+
+    for (si, section) in sections.enumerate() {
+        if section.sh_type(endian) != elf::SHT_SYMTAB {
+            continue;
+        }
+        let Ok(symbols) = run_make_support::object::read::elf::SymbolTable::parse(
+            endian, data, &sections, si, section,
+        ) else {
+            continue;
+        };
+        let strtab = symbols.strings();
+
+        for symbol in symbols.symbols() {
+            let bind = symbol.st_bind();
+            if bind != elf::STB_GLOBAL && bind != elf::STB_WEAK {
+                continue;
+            }
+            if symbol.st_shndx(endian) == elf::SHN_UNDEF {
+                continue;
+            }
+            let Ok(name_bytes) = symbol.name(endian, strtab) else { continue };
+            if let Ok(name) = str::from_utf8(name_bytes) {
+                defined.insert(name.to_string());
+            }
+        }
+    }
+}