Skip to content

Commit 267ae1f

Browse files
committed
symcheck: Add a wrapper around an archive
Rather than re-opening the archive file for each check, add a wrapper that keeps the data in memory. Additionally, collect the `--target` argument so it can be used within this crate.
1 parent 41b5e34 commit 267ae1f

File tree

1 file changed

+80
-24
lines changed

1 file changed

+80
-24
lines changed

crates/symbol-check/src/main.rs

Lines changed: 80 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use std::path::{Path, PathBuf};
88
use std::process::{Command, Stdio};
99

1010
use object::read::archive::{ArchiveFile, ArchiveMember};
11-
use object::{Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection};
11+
use object::{
12+
File as ObjFile, Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection,
13+
};
1214
use serde_json::Value;
1315

1416
const CHECK_LIBRARIES: &[&str] = &["compiler_builtins", "builtins_test_intrinsics"];
@@ -28,13 +30,11 @@ fn main() {
2830
let args_ref = args.iter().map(String::as_str).collect::<Vec<_>>();
2931

3032
match &args_ref[1..] {
31-
["build-and-check", rest @ ..] if !rest.is_empty() => {
32-
let paths = exec_cargo_with_args(rest);
33-
for path in paths {
34-
println!("Checking {}", path.display());
35-
verify_no_duplicates(&path);
36-
verify_core_symbols(&path);
37-
}
33+
["build-and-check", "--target", target, args @ ..] if !args.is_empty() => {
34+
run_build_and_check(Some(target), args);
35+
}
36+
["build-and-check", args @ ..] if !args.is_empty() => {
37+
run_build_and_check(None, args);
3838
}
3939
_ => {
4040
println!("{USAGE}");
@@ -43,12 +43,42 @@ fn main() {
4343
}
4444
}
4545

46+
fn run_build_and_check(target: Option<&str>, args: &[&str]) {
47+
let paths = exec_cargo_with_args(target, args);
48+
for path in paths {
49+
println!("Checking {}", path.display());
50+
let archive = Archive::from_path(&path);
51+
52+
verify_no_duplicates(&archive);
53+
verify_core_symbols(&archive);
54+
}
55+
}
56+
57+
fn host_target() -> String {
58+
let out = Command::new("rustc")
59+
.arg("--version")
60+
.arg("--verbose")
61+
.output()
62+
.unwrap();
63+
assert!(out.status.success());
64+
let out = String::from_utf8(out.stdout).unwrap();
65+
out.lines()
66+
.find_map(|s| s.strip_prefix("host: "))
67+
.unwrap()
68+
.to_owned()
69+
}
70+
4671
/// Run `cargo build` with the provided additional arguments, collecting the list of created
4772
/// libraries.
48-
fn exec_cargo_with_args(args: &[&str]) -> Vec<PathBuf> {
73+
fn exec_cargo_with_args(target: Option<&str>, args: &[&str]) -> Vec<PathBuf> {
74+
let mut host = String::new();
75+
let target = target.unwrap_or_else(|| {
76+
host = host_target();
77+
host.as_str()
78+
});
79+
4980
let mut cmd = Command::new("cargo");
50-
cmd.arg("build")
51-
.arg("--message-format=json")
81+
cmd.args(["build", "--target", target, "--message-format=json"])
5282
.args(args)
5383
.stdout(Stdio::piped());
5484

@@ -133,12 +163,12 @@ impl SymInfo {
133163
/// Note that this will also locate cases where a symbol is weakly defined in more than one place.
134164
/// Technically there are no linker errors that will come from this, but it keeps our binary more
135165
/// straightforward and saves some distribution size.
136-
fn verify_no_duplicates(path: &Path) {
166+
fn verify_no_duplicates(archive: &Archive) {
137167
let mut syms = BTreeMap::<String, SymInfo>::new();
138168
let mut dups = Vec::new();
139169
let mut found_any = false;
140170

141-
for_each_symbol(path, |symbol, member| {
171+
archive.for_each_symbol(|symbol, member| {
142172
// Only check defined globals
143173
if !symbol.is_global() || symbol.is_undefined() {
144174
return;
@@ -185,12 +215,12 @@ fn verify_no_duplicates(path: &Path) {
185215
}
186216

187217
/// Ensure that there are no references to symbols from `core` that aren't also (somehow) defined.
188-
fn verify_core_symbols(path: &Path) {
218+
fn verify_core_symbols(archive: &Archive) {
189219
let mut defined = BTreeSet::new();
190220
let mut undefined = Vec::new();
191221
let mut has_symbols = false;
192222

193-
for_each_symbol(path, |symbol, member| {
223+
archive.for_each_symbol(|symbol, member| {
194224
has_symbols = true;
195225

196226
// Find only symbols from `core`
@@ -219,14 +249,40 @@ fn verify_core_symbols(path: &Path) {
219249
println!(" success: no undefined references to core found");
220250
}
221251

222-
/// For a given archive path, do something with each symbol.
223-
fn for_each_symbol(path: &Path, mut f: impl FnMut(Symbol, &ArchiveMember)) {
224-
let data = fs::read(path).expect("reading file failed");
225-
let archive = ArchiveFile::parse(data.as_slice()).expect("archive parse failed");
226-
for member in archive.members() {
227-
let member = member.expect("failed to access member");
228-
let obj_data = member.data(&*data).expect("failed to access object");
229-
let obj = object::File::parse(obj_data).expect("failed to parse object");
230-
obj.symbols().for_each(|sym| f(sym, &member));
252+
/// Thin wrapper for owning data used by `object`.
253+
struct Archive {
254+
data: Vec<u8>,
255+
}
256+
257+
impl Archive {
258+
fn from_path(path: &Path) -> Self {
259+
Self {
260+
data: fs::read(path).expect("reading file failed"),
261+
}
262+
}
263+
264+
fn file(&self) -> ArchiveFile<'_> {
265+
ArchiveFile::parse(self.data.as_slice()).expect("archive parse failed")
266+
}
267+
268+
/// For a given archive, do something with each object file.
269+
fn for_each_object(&self, mut f: impl FnMut(ObjFile, &ArchiveMember)) {
270+
let archive = self.file();
271+
272+
for member in archive.members() {
273+
let member = member.expect("failed to access member");
274+
let obj_data = member
275+
.data(self.data.as_slice())
276+
.expect("failed to access object");
277+
let obj = ObjFile::parse(obj_data).expect("failed to parse object");
278+
f(obj, &member);
279+
}
280+
}
281+
282+
/// For a given archive, do something with each symbol.
283+
fn for_each_symbol(&self, mut f: impl FnMut(Symbol, &ArchiveMember)) {
284+
self.for_each_object(|obj, member| {
285+
obj.symbols().for_each(|sym| f(sym, member));
286+
});
231287
}
232288
}

0 commit comments

Comments
 (0)