-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
port tests/run-make/extern-fn-reachable to rmake #128314
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
use std::path::Path; | ||
|
||
use object::{self, Object, ObjectSymbol, SymbolIterator}; | ||
use object::{self, Object, ObjectSymbol, Symbol, SymbolIterator}; | ||
|
||
/// Iterate through the symbols in an object file. | ||
/// | ||
|
@@ -42,3 +42,46 @@ pub fn any_symbol_contains(path: impl AsRef<Path>, substrings: &[&str]) -> bool | |
false | ||
}) | ||
} | ||
|
||
/// Get a list of symbols that are in `symbol_names` but not the final binary. | ||
/// | ||
/// The symbols must also match `pred`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: match |
||
/// | ||
/// The symbol names must match exactly. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: apparently dyld expects dylib symbol names to be prefixed with |
||
/// | ||
/// Panics if `path` is not a valid object file readable by the current user. | ||
pub fn missing_exact_symbols<'a>( | ||
path: impl AsRef<Path>, | ||
symbol_names: &[&'a str], | ||
pred: impl Fn(&Symbol<'_, '_>) -> bool, | ||
) -> Vec<&'a str> { | ||
let mut found = vec![false; symbol_names.len()]; | ||
with_symbol_iter(path, |syms| { | ||
for sym in syms.filter(&pred) { | ||
for (i, symbol_name) in symbol_names.iter().enumerate() { | ||
found[i] |= sym.name_bytes().unwrap() == symbol_name.as_bytes(); | ||
} | ||
} | ||
}); | ||
return found | ||
.iter() | ||
.enumerate() | ||
.filter_map(|(i, found)| if !*found { Some(symbol_names[i]) } else { None }) | ||
.collect(); | ||
} | ||
|
||
/// Assert that the symbol file contains all of the listed symbols and they all match the given predicate | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: ditto above, we should describe on macOS we expect |
||
pub fn assert_contains_exact_symbols( | ||
path: impl AsRef<Path>, | ||
symbol_names: &[&str], | ||
pred: impl Fn(&Symbol<'_, '_>) -> bool, | ||
) { | ||
let missing = missing_exact_symbols(path.as_ref(), symbol_names, pred); | ||
if missing.len() > 0 { | ||
eprintln!("{} does not contain symbol(s): ", path.as_ref().display()); | ||
for sn in missing { | ||
eprintln!("* {}", sn); | ||
} | ||
panic!("missing symbols"); | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: could we add a brief comment on what this test is trying to check? I think it's trying to check that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, can you please squash the commits into one after adding a comment? There's some formatting commits. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ping |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
//@ ignore-cross-compile | ||
use run_make_support::object::ObjectSymbol; | ||
use run_make_support::rustc; | ||
use run_make_support::symbols::assert_contains_exact_symbols; | ||
fn main() { | ||
rustc().input("dylib.rs").output("dylib.so").prefer_dynamic().run(); | ||
assert_contains_exact_symbols("dylib.so", &["fun1", "fun2", "fun3", "fun4", "fun5"], |sym| { | ||
dbg!(dbg!(sym).is_global()) && !dbg!(sym.is_undefined()) | ||
}); | ||
Comment on lines
+7
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remark: original Makefile only tested for contains not exact match, I think one of the reasons being the exact naming is different. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm....that still feels pretty fragile to me, maybe we should just add an underscore on macOS any idea what's going on with the symbol types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you clarify what you mean by "symbol types"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: see above, macOS expects dylib symbols to be prefixed with |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping