Skip to content

Commit be8cc36

Browse files
committed
Replace the nm symbol check with a Rust implementation
1 parent 741d4c8 commit be8cc36

File tree

5 files changed

+297
-104
lines changed

5 files changed

+297
-104
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
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

Lines changed: 13 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -47,87 +47,25 @@ else
4747
fi
4848
fi
4949

50+
export TARGET="$target"
5051

51-
declare -a rlib_paths
52+
symcheck=(cargo run -p symbol-check --release)
53+
[[ "$target" = "wasm"* ]] && symcheck+=(--features wasm)
54+
symcheck+=(-- build-and-check)
5255

53-
# Set the `rlib_paths` global array to a list of all compiler-builtins rlibs
54-
update_rlib_paths() {
55-
if [ -d /builtins-target ]; then
56-
rlib_paths=( /builtins-target/"${target}"/debug/deps/libcompiler_builtins-*.rlib )
57-
else
58-
rlib_paths=( target/"${target}"/debug/deps/libcompiler_builtins-*.rlib )
59-
fi
60-
}
61-
62-
# Remove any existing artifacts from previous tests that don't set #![compiler_builtins]
63-
update_rlib_paths
64-
rm -f "${rlib_paths[@]}"
65-
66-
cargo build -p compiler_builtins --target "$target"
67-
cargo build -p compiler_builtins --target "$target" --release
68-
cargo build -p compiler_builtins --target "$target" --features c
69-
cargo build -p compiler_builtins --target "$target" --features c --release
70-
cargo build -p compiler_builtins --target "$target" --features no-asm
71-
cargo build -p compiler_builtins --target "$target" --features no-asm --release
72-
cargo build -p compiler_builtins --target "$target" --features no-f16-f128
73-
cargo build -p compiler_builtins --target "$target" --features no-f16-f128 --release
74-
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
56+
"${symcheck[@]}" -p compiler_builtins --target "$target"
57+
"${symcheck[@]}" -p compiler_builtins --target "$target" --release
58+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features c
59+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features c --release
60+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm
61+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm --release
62+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128
63+
"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 --release
9964

10065
# 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
108-
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
126-
127-
rm -f "${rlib_paths[@]}"
12866

12967
build_intrinsics_test() {
130-
cargo build \
68+
"${symcheck[@]}" \
13169
--target "$target" --verbose \
13270
--manifest-path builtins-test-intrinsics/Cargo.toml "$@"
13371
}
@@ -143,35 +81,6 @@ build_intrinsics_test --features c --release
14381
CARGO_PROFILE_DEV_LTO=true build_intrinsics_test
14482
CARGO_PROFILE_RELEASE_LTO=true build_intrinsics_test --release
14583

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
174-
17584
# Test libm
17685

17786
# Make sure a simple build works

crates/symbol-check/Cargo.toml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
[package]
2+
name = "symbol-check"
3+
version = "0.1.0"
4+
edition = "2024"
5+
publish = false
6+
7+
[dependencies]
8+
# Used as a git dependency since the latest release does not support wasm
9+
object = { git = "https://github.com/gimli-rs/object.git", rev = "013fac75da56a684377af4151b8164b78c1790e0" }
10+
serde_json = "1.0.140"
11+
12+
[features]
13+
wasm = ["object/wasm"]

crates/symbol-check/src/main.rs

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
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 std::collections::{BTreeMap, BTreeSet};
5+
use std::fs;
6+
use std::io::{BufRead, BufReader};
7+
use std::path::{Path, PathBuf};
8+
use std::process::{Command, Stdio};
9+
10+
use object::read::archive::{ArchiveFile, ArchiveMember};
11+
use object::{Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection};
12+
use serde_json::Value;
13+
14+
const USAGE: &str = "Usage:
15+
16+
symbol-check build-and-check CARGO_ARGS ...
17+
18+
Cargo will get invoked with `CARGO_ARGS` and all output
19+
`compiler_builtins*.rlib` files will be checked.
20+
";
21+
22+
fn main() {
23+
// Create a `&str` vec so we can match on it.
24+
let args = std::env::args().collect::<Vec<_>>();
25+
let args_ref = args.iter().map(String::as_str).collect::<Vec<_>>();
26+
27+
match &args_ref[1..] {
28+
["build-and-check", rest @ ..] if !rest.is_empty() => {
29+
let paths = exec_cargo_with_args(rest);
30+
for path in paths {
31+
println!("Checking {}", path.display());
32+
verify_no_duplicates(&path);
33+
verify_core_symbols(&path);
34+
}
35+
}
36+
_ => {
37+
println!("{USAGE}");
38+
std::process::exit(1);
39+
}
40+
}
41+
}
42+
43+
/// Run `cargo build` with the provided additional arguments.
44+
fn exec_cargo_with_args(args: &[&str]) -> Vec<PathBuf> {
45+
let mut cmd = Command::new("cargo")
46+
.arg("build")
47+
.arg("--message-format=json")
48+
.args(args)
49+
.stdout(Stdio::piped())
50+
.spawn()
51+
.expect("failed to launch Cargo");
52+
53+
let stdout = cmd.stdout.take().unwrap();
54+
let reader = BufReader::new(stdout);
55+
56+
let mut x = Vec::new();
57+
58+
for m in reader.lines() {
59+
let m = m.expect("failed to read line");
60+
println!("{m}");
61+
62+
let j: Value = serde_json::from_str(&m).expect("failed to deserialize");
63+
if j["reason"] != "compiler-artifact" {
64+
continue;
65+
}
66+
67+
for fname in j["filenames"].as_array().expect("filenames not an array") {
68+
let path = fname.as_str().expect("file name not a string");
69+
let p = PathBuf::from(path);
70+
71+
if let Some(ex) = p.extension()
72+
&& ex == "rlib"
73+
&& p.file_name()
74+
.unwrap()
75+
.to_str()
76+
.unwrap()
77+
.contains("compiler_builtins")
78+
{
79+
x.push(p);
80+
}
81+
}
82+
}
83+
84+
cmd.wait().expect("failed to wait on Cargo");
85+
86+
assert!(!x.is_empty(), "no compiler_builtins rlibs found");
87+
println!("Collected the following rlibs to check: {x:#?}");
88+
89+
x
90+
}
91+
92+
#[expect(unused)] // only for printing
93+
#[derive(Clone, Debug)]
94+
struct SymInfo {
95+
name: String,
96+
kind: SymbolKind,
97+
scope: SymbolScope,
98+
section: SymbolSection,
99+
is_undefined: bool,
100+
is_global: bool,
101+
is_local: bool,
102+
is_weak: bool,
103+
is_common: bool,
104+
address: u64,
105+
object: String,
106+
}
107+
108+
impl SymInfo {
109+
fn new(sym: &Symbol, member: &ArchiveMember) -> Self {
110+
Self {
111+
name: sym.name().expect("missing name").to_owned(),
112+
kind: sym.kind(),
113+
scope: sym.scope(),
114+
section: sym.section(),
115+
is_undefined: sym.is_undefined(),
116+
is_global: sym.is_global(),
117+
is_local: sym.is_local(),
118+
is_weak: sym.is_weak(),
119+
is_common: sym.is_common(),
120+
address: sym.address(),
121+
object: String::from_utf8_lossy(member.name()).into_owned(),
122+
}
123+
}
124+
}
125+
126+
/// Ensure that the same global symbol isn't defined in multiple object files within an archive.
127+
fn verify_no_duplicates(path: impl AsRef<Path>) {
128+
let mut syms = BTreeMap::<String, SymInfo>::new();
129+
let mut dups = Vec::new();
130+
let mut found_any = false;
131+
132+
for_each_symbol(&path, |sym, member| {
133+
// Only check defined globals
134+
if !sym.is_global() || sym.is_undefined() {
135+
return;
136+
}
137+
138+
let info = SymInfo::new(&sym, member);
139+
140+
// x86-32 includes multiple copies of thunk symbols
141+
if info.name.starts_with("__x86.get_pc_thunk") {
142+
return;
143+
}
144+
145+
// Windows has symbols for literal numeric constants, string literals, and MinGW pseudo-
146+
// relocations. These are allowed to have repeated definitions.
147+
let win_allowed_dup_pfx = ["__real@", "__xmm@", "??_C@_", ".refptr"];
148+
if win_allowed_dup_pfx
149+
.iter()
150+
.any(|pfx| info.name.starts_with(pfx))
151+
{
152+
return;
153+
}
154+
155+
match syms.get(&info.name) {
156+
Some(existing) => {
157+
dups.push(info);
158+
dups.push(existing.clone());
159+
}
160+
None => {
161+
syms.insert(info.name.clone(), info);
162+
}
163+
}
164+
165+
found_any = true;
166+
});
167+
168+
assert!(found_any, "no symbols found");
169+
170+
if !dups.is_empty() {
171+
dups.sort_unstable_by(|a, b| a.name.cmp(&b.name));
172+
173+
let bash = if cfg!(windows) {
174+
r#"C:\Program Files\Git\bin\bash.EXE"#
175+
} else {
176+
"bash"
177+
};
178+
179+
Command::new(bash)
180+
.arg("run-rust-nm.sh")
181+
.arg(std::env::var("TARGET").unwrap())
182+
.arg(path.as_ref())
183+
.status()
184+
.unwrap();
185+
186+
panic!("found duplicate symbols: {dups:#?}");
187+
}
188+
189+
println!(" success: no duplicate symbols found");
190+
}
191+
192+
/// Ensure that there are no references to symbols from `core` that aren't also (somehow) defined.
193+
fn verify_core_symbols(path: impl AsRef<Path>) {
194+
let mut defined = BTreeSet::new();
195+
let mut undefined = Vec::new();
196+
let mut has_symbols = false;
197+
198+
for_each_symbol(path, |sym, member| {
199+
has_symbols = true;
200+
201+
// Find only symbols from `core`
202+
if !sym.name().unwrap().contains("_ZN4core") {
203+
return;
204+
}
205+
206+
let info = SymInfo::new(&sym, member);
207+
if info.is_undefined {
208+
undefined.push(info);
209+
} else {
210+
defined.insert(info.name);
211+
}
212+
});
213+
214+
assert!(has_symbols, "no symbols found");
215+
216+
// Discard any symbols that are defined somewhere in the archive
217+
undefined.retain(|sym| !defined.contains(&sym.name));
218+
219+
if !undefined.is_empty() {
220+
undefined.sort_unstable_by(|a, b| a.name.cmp(&b.name));
221+
panic!("found undefined symbols from core: {undefined:#?}");
222+
}
223+
224+
println!(" success: no undefined references to core found");
225+
}
226+
227+
/// For a given archive path, do something with each symbol.
228+
fn for_each_symbol(path: impl AsRef<Path>, mut f: impl FnMut(Symbol, &ArchiveMember)) {
229+
let data = fs::read(path).expect("reading file failed");
230+
let archive = ArchiveFile::parse(data.as_slice()).expect("archive parse failed");
231+
for member in archive.members() {
232+
let member = member.expect("failed to access member");
233+
let obj_data = member.data(&*data).expect("failed to access object");
234+
let obj = object::File::parse(obj_data).expect("failed to parse object");
235+
obj.symbols().for_each(|sym| f(sym, &member));
236+
}
237+
}

0 commit comments

Comments
 (0)