Skip to content

Commit 886176c

Browse files
tgross35tgross35
committed
Replace the nm symbol check with a Rust implementation
1 parent 313e3ee commit 886176c

File tree

4 files changed

+263
-81
lines changed

4 files changed

+263
-81
lines changed

‎Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ members = [
66
"crates/libm-macros",
77
"crates/musl-math-sys",
88
"crates/panic-handler",
9+
"crates/symbol-check",
910
"crates/util",
1011
"libm",
1112
"libm-test",

‎ci/run.sh

+98-81
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,26 @@ else
4848
fi
4949

5050

51-
declare -a rlib_paths
52-
53-
# Set the `rlib_paths` global array to a list of all compiler-builtins rlibs
54-
update_rlib_paths() {
51+
# Run a command for each `compiler_builtins` rlib file
52+
for_each_rlib() {
5553
if [ -d /builtins-target ]; then
5654
rlib_paths=( /builtins-target/"${target}"/debug/deps/libcompiler_builtins-*.rlib )
5755
else
5856
rlib_paths=( target/"${target}"/debug/deps/libcompiler_builtins-*.rlib )
5957
fi
58+
59+
if [ "${#rlib_paths[@]}" -lt 1 ]; then
60+
echo "rlibs expected but not found"
61+
exit 1
62+
fi
63+
64+
"$@" "${rlib_paths[@]}"
6065
}
6166

67+
6268
# Remove any existing artifacts from previous tests that don't set #![compiler_builtins]
6369
update_rlib_paths
64-
rm -f "${rlib_paths[@]}"
70+
for_each_rlib rm -f
6571

6672
cargo build -p compiler_builtins --target "$target"
6773
cargo build -p compiler_builtins --target "$target" --release
@@ -72,57 +78,64 @@ cargo build -p compiler_builtins --target "$target" --features no-asm --release
7278
cargo build -p compiler_builtins --target "$target" --features no-f16-f128
7379
cargo build -p compiler_builtins --target "$target" --features no-f16-f128 --release
7480

75-
PREFIX=${target//unknown-/}-
76-
case "$target" in
77-
armv7-*)
78-
PREFIX=arm-linux-gnueabihf-
79-
;;
80-
thumb*)
81-
PREFIX=arm-none-eabi-
82-
;;
83-
*86*-*)
84-
PREFIX=
85-
;;
86-
esac
87-
88-
NM=$(find "$(rustc --print sysroot)" \( -name llvm-nm -o -name llvm-nm.exe \) )
89-
if [ "$NM" = "" ]; then
90-
NM="${PREFIX}nm"
91-
fi
92-
93-
# i686-pc-windows-gnu tools have a dependency on some DLLs, so run it with
94-
# rustup run to ensure that those are in PATH.
95-
TOOLCHAIN="$(rustup show active-toolchain | sed 's/ (default)//')"
96-
if [[ "$TOOLCHAIN" == *i686-pc-windows-gnu ]]; then
97-
NM="rustup run $TOOLCHAIN $NM"
98-
fi
81+
symcheck=(cargo run -p symbol-check)
82+
[[ "$target" = "wasm"* ]] && symcheck+=(--features wasm)
9983

10084
# Look out for duplicated symbols when we include the compiler-rt (C) implementation
101-
update_rlib_paths
102-
for rlib in "${rlib_paths[@]}"; do
103-
set +x
104-
echo "================================================================"
105-
echo "checking $rlib for duplicate symbols"
106-
echo "================================================================"
107-
set -x
85+
for_each_rlib "${symcheck[@]}" -- check-duplicates
86+
for_each_rlib rm -f
87+
88+
# PREFIX=${target//unknown-/}-
89+
# case "$target" in
90+
# armv7-*)
91+
# PREFIX=arm-linux-gnueabihf-
92+
# ;;
93+
# thumb*)
94+
# PREFIX=arm-none-eabi-
95+
# ;;
96+
# *86*-*)
97+
# PREFIX=
98+
# ;;
99+
# esac
100+
101+
# NM=$(find "$(rustc --print sysroot)" \( -name llvm-nm -o -name llvm-nm.exe \) )
102+
# if [ "$NM" = "" ]; then
103+
# NM="${PREFIX}nm"
104+
# fi
105+
106+
# # i686-pc-windows-gnu tools have a dependency on some DLLs, so run it with
107+
# # rustup run to ensure that those are in PATH.
108+
# TOOLCHAIN="$(rustup show active-toolchain | sed 's/ (default)//')"
109+
# if [[ "$TOOLCHAIN" == *i686-pc-windows-gnu ]]; then
110+
# NM="rustup run $TOOLCHAIN $NM"
111+
# fi
112+
113+
# # Look out for duplicated symbols when we include the compiler-rt (C) implementation
114+
# update_rlib_paths
115+
# for rlib in "${rlib_paths[@]}"; do
116+
# set +x
117+
# echo "================================================================"
118+
# echo "checking $rlib for duplicate symbols"
119+
# echo "================================================================"
120+
# set -x
108121

109-
duplicates_found=0
110-
111-
# NOTE On i586, It's normal that the get_pc_thunk symbol appears several
112-
# times so ignore it
113-
$NM -g --defined-only "$rlib" 2>&1 |
114-
sort |
115-
uniq -d |
116-
grep -v __x86.get_pc_thunk --quiet |
117-
grep 'T __' && duplicates_found=1
118-
119-
if [ "$duplicates_found" != 0 ]; then
120-
echo "error: found duplicate symbols"
121-
exit 1
122-
else
123-
echo "success; no duplicate symbols found"
124-
fi
125-
done
122+
# duplicates_found=0
123+
124+
# # NOTE On i586, It's normal that the get_pc_thunk symbol appears several
125+
# # times so ignore it
126+
# $NM -g --defined-only "$rlib" 2>&1 |
127+
# sort |
128+
# uniq -d |
129+
# grep -v __x86.get_pc_thunk --quiet |
130+
# grep 'T __' && duplicates_found=1
131+
132+
# if [ "$duplicates_found" != 0 ]; then
133+
# echo "error: found duplicate symbols"
134+
# exit 1
135+
# else
136+
# echo "success; no duplicate symbols found"
137+
# fi
138+
# done
126139

127140
rm -f "${rlib_paths[@]}"
128141

@@ -143,34 +156,38 @@ build_intrinsics_test --features c --release
143156
CARGO_PROFILE_DEV_LTO=true build_intrinsics_test
144157
CARGO_PROFILE_RELEASE_LTO=true build_intrinsics_test --release
145158

146-
# Ensure no references to any symbols from core
147-
update_rlib_paths
148-
for rlib in "${rlib_paths[@]}"; do
149-
set +x
150-
echo "================================================================"
151-
echo "checking $rlib for references to core"
152-
echo "================================================================"
153-
set -x
154-
155-
tmpdir="${CARGO_TARGET_DIR:-target}/tmp"
156-
test -d "$tmpdir" || mkdir "$tmpdir"
157-
defined="$tmpdir/defined_symbols.txt"
158-
undefined="$tmpdir/defined_symbols.txt"
159-
160-
$NM --quiet -U "$rlib" | grep 'T _ZN4core' | awk '{print $3}' | sort | uniq > "$defined"
161-
$NM --quiet -u "$rlib" | grep 'U _ZN4core' | awk '{print $2}' | sort | uniq > "$undefined"
162-
grep_has_results=0
163-
grep -v -F -x -f "$defined" "$undefined" && grep_has_results=1
164-
165-
if [ "$target" = "powerpc64-unknown-linux-gnu" ]; then
166-
echo "FIXME: powerpc64 fails these tests"
167-
elif [ "$grep_has_results" != 0 ]; then
168-
echo "error: found unexpected references to core"
169-
exit 1
170-
else
171-
echo "success; no references to core found"
172-
fi
173-
done
159+
for_each_rlib "${symcheck[@]}" -- check-core-syms
160+
161+
# # Ensure no references to any symbols from core
162+
# update_rlib_paths
163+
# for rlib in "${rlib_paths[@]}"; do
164+
# set +x
165+
# echo "================================================================"
166+
# echo "checking $rlib for references to core"
167+
# echo "================================================================"
168+
# set -x
169+
170+
# tmpdir="${CARGO_TARGET_DIR:-target}/tmp"
171+
# test -d "$tmpdir" || mkdir "$tmpdir"
172+
# defined="$tmpdir/defined_symbols.txt"
173+
# undefined="$tmpdir/defined_symbols.txt"
174+
175+
# $NM --quiet -U "$rlib" | grep 'T _ZN4core' | awk '{print $3}' | sort | uniq > "$defined"
176+
# $NM --quiet -u "$rlib" | grep 'U _ZN4core' | awk '{print $2}' | sort | uniq > "$undefined"
177+
# grep_has_results=0
178+
# grep -v -F -x -f "$defined" "$undefined" && grep_has_results=1
179+
180+
# if [ "$target" = "powerpc64-unknown-linux-gnu" ]; then
181+
# echo "FIXME: powerpc64 fails these tests"
182+
# elif [ "$grep_has_results" != 0 ]; then
183+
# echo "error: found unexpected references to core"
184+
# exit 1
185+
# else
186+
# echo "success; no references to core found"
187+
# fi
188+
# done
189+
190+
174191

175192
# Test libm
176193

‎crates/symbol-check/Cargo.toml

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[package]
2+
name = "symbol-check"
3+
version = "0.1.0"
4+
edition = "2024"
5+
6+
[dependencies]
7+
object = "0.36.7"
8+
9+
[features]
10+
wasm = ["object/wasm"]

‎crates/symbol-check/src/main.rs

+154
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
//! Tool used by CI to inspect compiler-builtins archives and help ensure we won't run into any
2+
//! linking errors.
3+
4+
use object::read::archive::{ArchiveFile, ArchiveMember};
5+
use object::{Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection};
6+
use std::collections::{BTreeMap, BTreeSet};
7+
use std::{fs, path::Path};
8+
9+
const USAGE: &str = "Usage:
10+
11+
symbol-check check-duplicates ARCHIVE ...
12+
symbol-check check-core-syms ARCHIVE ...
13+
14+
Note that multiple archives may be specified but they are checked independently
15+
rather than as a group.";
16+
17+
fn main() {
18+
// Create a `&str` vec so we can match on it.
19+
let args = std::env::args().collect::<Vec<_>>();
20+
let args_ref = args.iter().map(String::as_str).collect::<Vec<_>>();
21+
22+
match &args_ref[1..] {
23+
["check-duplicates", rest @ ..] if !rest.is_empty() => {
24+
rest.iter().for_each(verify_no_duplicates);
25+
}
26+
["check-core-syms", rest @ ..] if !rest.is_empty() => {
27+
rest.iter().for_each(verify_core_symbols);
28+
}
29+
_ => {
30+
println!("{USAGE}");
31+
std::process::exit(1);
32+
}
33+
}
34+
}
35+
36+
#[expect(unused)] // only for printing
37+
#[derive(Clone, Debug)]
38+
struct SymInfo {
39+
name: String,
40+
kind: SymbolKind,
41+
scope: SymbolScope,
42+
section: SymbolSection,
43+
is_undefined: bool,
44+
is_global: bool,
45+
is_local: bool,
46+
is_weak: bool,
47+
is_common: bool,
48+
address: u64,
49+
object: String,
50+
}
51+
52+
impl SymInfo {
53+
fn new(sym: &Symbol, member: &ArchiveMember) -> Self {
54+
Self {
55+
name: sym.name().expect("missing name").to_owned(),
56+
kind: sym.kind(),
57+
scope: sym.scope(),
58+
section: sym.section(),
59+
is_undefined: sym.is_undefined(),
60+
is_global: sym.is_global(),
61+
is_local: sym.is_local(),
62+
is_weak: sym.is_weak(),
63+
is_common: sym.is_common(),
64+
address: sym.address(),
65+
object: String::from_utf8_lossy(member.name()).into_owned(),
66+
}
67+
}
68+
}
69+
70+
/// Ensure that the same global symbol isn't defined in multiple object files within an archive.
71+
fn verify_no_duplicates(path: impl AsRef<Path>) {
72+
println!("Checking `{}` for duplicates", path.as_ref().display());
73+
74+
let mut syms = BTreeMap::<String, SymInfo>::new();
75+
let mut dups = Vec::new();
76+
77+
for_each_symbol(path, |sym, member| {
78+
// Only check defined globals, exclude wasm file symbols
79+
if !sym.is_global() || sym.is_undefined() || sym.kind() == SymbolKind::File {
80+
return;
81+
}
82+
83+
let info = SymInfo::new(&sym, member);
84+
match syms.get(&info.name) {
85+
Some(existing) => {
86+
dups.push(info);
87+
dups.push(existing.clone());
88+
}
89+
None => {
90+
syms.insert(info.name.clone(), info);
91+
}
92+
}
93+
});
94+
95+
if cfg!(windows) {
96+
// Ignore literal constants
97+
let allowed_dup_pfx = ["__real@", "__xmm@"];
98+
dups.retain(|sym| !allowed_dup_pfx.iter().any(|pfx| sym.name.starts_with(pfx)));
99+
}
100+
101+
if !dups.is_empty() {
102+
dups.sort_unstable_by(|a, b| a.name.cmp(&b.name));
103+
panic!("found duplicate symbols: {dups:#?}");
104+
}
105+
106+
println!("success: no duplicate symbols found");
107+
}
108+
109+
/// Ensure that there are no references to symbols from `core` that aren't also (somehow) defined.
110+
fn verify_core_symbols(path: impl AsRef<Path>) {
111+
println!(
112+
"Checking `{}` for references to core",
113+
path.as_ref().display()
114+
);
115+
116+
let mut defined = BTreeSet::new();
117+
let mut undefined = Vec::new();
118+
119+
for_each_symbol(path, |sym, member| {
120+
// Find only symbols from `core`
121+
if !sym.name().unwrap().contains("_ZN4core") {
122+
return;
123+
}
124+
125+
let info = SymInfo::new(&sym, member);
126+
if info.is_undefined {
127+
undefined.push(info);
128+
} else {
129+
defined.insert(info.name);
130+
}
131+
});
132+
133+
// Discard any symbols that are defined somewhere in the archive
134+
undefined.retain(|sym| !defined.contains(&sym.name));
135+
136+
if !undefined.is_empty() {
137+
undefined.sort_unstable_by(|a, b| a.name.cmp(&b.name));
138+
panic!("found undefined symbols from core: {undefined:#?}");
139+
}
140+
141+
println!("success: no undefined references to core found");
142+
}
143+
144+
/// For a given archive path, do something with each symbol.
145+
fn for_each_symbol(path: impl AsRef<Path>, mut f: impl FnMut(Symbol, &ArchiveMember)) {
146+
let data = fs::read(path).expect("reading file failed");
147+
let archive = ArchiveFile::parse(data.as_slice()).expect("archive parse failed");
148+
for member in archive.members() {
149+
let member = member.expect("failed to access member");
150+
let obj_data = member.data(&*data).expect("failed to access object");
151+
let obj = object::File::parse(obj_data).expect("failed to parse object");
152+
obj.symbols().for_each(|sym| f(sym, &member));
153+
}
154+
}

0 commit comments

Comments
 (0)