I am getting an error like Error: package '/Users/alpharush/tob/../Cargo.toml' is a member of the wrong workspace. Can we improve support for cargo workspaces?

Probably relevant to the fix:

Maybe related to #31

Running setup.sh gives me these warnings:

warning: trait objects without an explicit `dyn` are deprecated
   --> src/deps.rs:268:23
    |
268 |         let exec: Arc<Executor> = Arc::new(cust_exec);
    |                       ^^^^^^^^ help: use `dyn`: `dyn Executor`
    |
    = note: #[warn(bare_trait_objects)] on by default

warning: trait objects without an explicit `dyn` are deprecated
   --> src/deps.rs:446:30
    |
446 |         _handle_stdout: &mut FnMut(&str) -> CargoResult<()>,
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `dyn`: `dyn FnMut(&str) -> CargoResult<()>`

warning: trait objects without an explicit `dyn` are deprecated
   --> src/deps.rs:447:30
    |
447 |         _handle_stderr: &mut FnMut(&str) -> CargoResult<()>,
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `dyn`: `dyn FnMut(&str) -> CargoResult<()>`

    Finished release [optimized] target(s)

Newer versions of rustc use LLVM 15, which is why the CI is currently broken.

Needs support in llvm-ir first: cdisselkoen/llvm-ir#24

Do benchmark against this, and see if we can improve it! https://github.com/ktrianta/rust-callgraph-benchmark

Currently, you run a bash script in a certain directory and it tests. These should be unit tests instead.

LLVM 12 has been out for a while. We should try to support it, either by moving from LLVM 11 or in tandem with LLVM 11.

Hi! First of all, siderophile is a great tool. We've tried it as part of identifying MIRI/fuzz targets and I wanted to inform you on our experiences with the tool and what we had wished to be possible.

No false positives as far as I can see!

It would be good if certain crates or paths can be excluded from the badness computation. There are a number of crates that extend the ability of the standard library carefully, execute their own MIRI test, run fuzzers or even mathematically proof check their implemenation (efforts ongoing on that front). Consider std itself is not part of the badness considerations. (This would require the std-src component to be feasible in the first place.) It makes a limited amount of sense to run those tools from the downstream consumer crates if only safe interface are accessed. The benefit is small because sound interfaces—like the whole of std—mustn't have any UB and the high level causes escalating computational effort to be expended on reaching the unsafe leafs in the first place. As an example, a large fraction of the badness in image is caused by bytemuck and byteorder. But contrary to other ad-hoc unsafety, these are carefully reviewed. This has the effect of hiding the ad-hoc cases that are more likely to be error prone.

Secondly, it would be incredible if siderophile could determine which functions with badness are not executed in a set of tests or fuzzers, and how many tests cover them! In terms of the call graph: Given the the test methods as roots find which bad methods are reachable from them. And conversely the program could answer which tests are purely functional tests and do not execute any unsafe blocks. This would allow a CI approach where MIRI only runs on tests determined relevant by the badness metric and guide the process of creating new test suites—which again would conserve computational efforts.

For crates that only build on certain platforms it is convenient to use a cross compilation target. Setting up the default target almost works but a target specific dir would need to be pushed here:

Probably the easiest way to do this would be to allow passing down a target flag to cargo. If the target flag gets passed down, then the bitcode gets looked for in a non-default dir.

See https://crates.io/crates/llvm-ir, https://github.com/TheDan64/inkwell.

We should change this to have it be configurable to use other LLVM versions, using the Inkwell library, which supports other versions.

Users would have to figure out their LLVM version using rustc --version --verbose, then configure inkwell to do the right thing in the Siderophile Cargo.toml, then build.

siderophile will currently output something like

<http::uri::scheme::Scheme as HttpTryFrom<&'a[u8]>>::try_from

or

<futures_io_preview::if_std::Pin<&'amutT> as AsyncRead>::initializer

The lifetime 'a should not be present, and there should be a space between mut and T.

It would be helpful to be able to specify feature flags to compile the crate with when creating the callgraph

Blocked on cdisselkoen/llvm-ir#38.

siderophile insists on using the default compiler. siderophile should check that the default compiler uses the LLVM version it expects.

Originally posted by @smoelius in #146 (comment)

Thanks for this cool tool!

I've tried this on good ol' authenticator-rs, which Firefox uses for security keys for Web Authentication. I failed out somewhere, it appears, in LLVM, running on MacOS 10.14.5:

~/git/siderophile/analyze.sh authenticator
trawling source code of dependencies for unsafety
   Compiling autocfg v0.1.2
   Compiling libc v0.2.50
   Compiling core-foundation-sys v0.6.2
    Checking rand_core v0.4.0
    Checking cfg-if v0.1.7
    Checking runloop v0.1.0
    Checking boxfnonce v0.0.3
    Checking bitflags v1.0.4
    Checking log v0.4.6
    Checking rand_core v0.3.1
    Checking rand_hc v0.1.0
    Checking rand_isaac v0.1.1
    Checking rand_xorshift v0.1.1
   Compiling rand_pcg v0.1.2
   Compiling rand_chacha v0.1.1
   Compiling rand v0.6.5
    Checking rand_jitter v0.1.3
    Checking rand_os v0.1.3
    Checking core-foundation v0.6.3
    Checking authenticator v0.2.7 (/Users/jcjones/git/authenticator-rs)
    Finished dev [unoptimized + debuginfo] target(s) in 4.88s
generating LLVM bitcode for the callgraph
   Compiling libc v0.2.50
   Compiling autocfg v0.1.2
   Compiling core-foundation-sys v0.6.2
   Compiling rand_core v0.4.0
   Compiling cfg-if v0.1.7
   Compiling bitflags v1.0.4
   Compiling boxfnonce v0.0.3
   Compiling runloop v0.1.0
   Compiling log v0.4.6
   Compiling rand_core v0.3.1
   Compiling rand_xorshift v0.1.1
   Compiling rand_isaac v0.1.1
   Compiling rand_hc v0.1.0
   Compiling rand_os v0.1.3
   Compiling rand_jitter v0.1.3
   Compiling core-foundation v0.6.3
   Compiling rand_pcg v0.1.2
   Compiling rand_chacha v0.1.1
   Compiling rand v0.6.5
   Compiling authenticator v0.2.7 (/Users/jcjones/git/authenticator-rs)
    Finished dev [unoptimized + debuginfo] target(s) in 21.17s
generating callgraph
opt: ./target/debug/deps/authenticator-6782ed23156be235.bc: error: Invalid record

Amending the analyze.sh to be verbose, the actual command that fails is

rustup shows:

active toolchain
----------------

stable-x86_64-apple-darwin (default)
rustc 1.35.0 (3c235d560 2019-05-20)

llvm-config --version shows: 7.0.1

Thanks!

Siderophile should have a mode of operation where instead of building everything via cargo it will just output its build plan, so it can be analysed/debugged.

This should help introducing Siderophile to projects with complex build systems.

This is possible as Cargo API's BuildConfig has a build_plan field, which, if set to true outputs that information to stdout.

Just to be clear, this issue is a matter of adding e.g. a:

     opt.build_config.build_plan = true;

to the build_complex_options function though it needs to be enabled only when a certain flag or environment variable is present, when launching Siderophile.

My project has a rust-toolchain file in its root for a nightly toolchain, but ./setup.sh simply calls "cargo" so it built with a different version than my project. This lead to the following error when running analyze.sh.

$ ~/src/siderophile/analyze.sh myapp
trawling source code of dependencies for unsafety
   Compiling libc v0.2.60

...


error[E0514]: found crate `cached` compiled by an incompatible version of rustc myapp                                   
  --> src/lib.rs:15:1
   |
15 | extern crate cached;
   | ^^^^^^^^^^^^^^^^^^^^
   |
   = help: please recompile that crate using this compiler (rustc 1.37.0-nightly (8aa42ed7c 2019-06-24))
   = note: the following crate versions were found:
           crate `cached` compiled by rustc 1.36.0 (a53f9df32 2019-07-03): ~/src/myapp/target/debug/deps/libcached-b5e2b19b56d13d52.rmeta

error: aborting due to previous error

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Cargo("Could not compile `myapp`.")', src/libcore/result.rs:1051:5
stack backtrace:
   0: std::panicking::default_hook::{{closure}}
   1: std::panicking::default_hook
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::continue_panic_fmt
   4: rust_begin_unwind
   5: core::panicking::panic_fmt
   6: core::result::unwrap_failed
   7: siderophile::main
   8: std::rt::lang_start::{{closure}}
   9: std::panicking::try::do_call
  10: __rust_maybe_catch_panic
  11: std::rt::lang_start_internal
  12: main

To fix this problem, I had to build and run siderophile with the correct toolchain selected.

cd ~/src/siderophile
RUSTUP_TOOLCHAIN=$(cat ~/src/myapp/rust-toolchain) cargo build --release

$ cd ~/src/myapp
$ RUSTUP_TOOLCHAIN=$(cat rust-toolchain) ~/src/siderophile/analyze.sh myapp

trawling source code of dependencies for unsafety
   Compiling libc v0.2.60
...
generating LLVM bitcode for the callgraph
...

A note about this should probably be added to the readme.

The "how to use" section in the README is an 11 chapter tome of incantations. Ideally, all functionality can be rolled into a single Rust binary

siderophile currently outputs

<lock_api::mutex::MutexGuard<'a,R,T> as DerefMut>::deref_mut

whereas the callgraph entry is

<lock_api::mutex::MutexGuard<R,T> as core::ops::deref::DerefMut>::deref_mut

To fix, all impl paths should be formatted without lifetime parameters.

cargo-audit is a utility which compares dependencies in Cargo.lock agains the RustSec Advisory Database. I've opened an issue proposing a potential integration with Siderophile here:

rustsec/rustsec#89

Recently we published an advisory high severity but low exploitability vulnerability to the database which resulted in false positive alerts for many users. The issue linked above goes into details about why this is an interesting case of where a call graph analysis would've helped.

We've done some work on collecting paths to affected vulnerabilities already to support this kind of analysis, and have the ability to collect this sort of information in advisories. Here's an example:

https://github.com/RustSec/advisory-db/blob/a8e2ec8/crates/safe-transmute/RUSTSEC-2018-0013.toml#L21

[affected_paths]
">= 0.4.0, <= 0.10.0"  = ["safe_transmute::guarded_transmute_vec_permissive"]
"= 0.10.0"             = ["safe_transmute::guarded_transmute_to_bytes_vec"]

(sidebar: looking that again, it feels like we should swap these so the path is on the left and the impacted versions are on the right)

What we need out of a call graph analysis tool is something that can both compute the call graph for --all-features, and then a way of testing if particular paths exist in the call graph. Compared to what Siderophile already does, this seems fairly simple.

If this sounds like a good idea, I'm curious what you think the best way to integrate cargo-audit and Siderophile would be. Should we invoke it as a subprocess, or is there a way to use it as a library/crate dependency? Is there a particular crate we can use that provides the call graph analysis functionality in isolation?

Siderophile currently only works for libraries. Under the hood, it runs a cargo rustc --lib. In theory, this tool could work just as well for binaries.

The Cargo library is not guaranteed to have any stable API at all- you're supposed to shell out to the cargo binary instead.

Things will continue to break periodically until we fix this- if we don't update the Cargo library version, some Cargo.toml files will break siderophile, and if we do update, things will just break. So at some point I need to change this to call the binary instead.

The PR #22 added a workaround for issue #14.

However, this solution is not ideal as:

• it does not detect/respect rust-toolchain in a project itself (it will use whatever toolchain is the default one, which is good enough for most cases)
• it is implemented as a workaround in a bash script: we simply pass an environment variable.

I tried to track this issue more deeply, but I could not find the proper solution. Anyway here is some info that can be useful for further investigation into it:

• Siderophile runs cargo clean and cargo check through Cargo API:

  siderophile/src/deps.rs

  Lines 244 to 270 in c3e6c76

  	/// Trigger a `cargo clean` + `cargo check` and listen to the cargo/rustc
  	/// communication to figure out which source files were used by the build.
  	pub(crate) fn resolve_rs_file_deps(
  	copt: &CompileOptions,
  	ws: &Workspace,
  	) -> Result<HashMap<PathBuf, u32>, RsResolveError> {
  	let config = ws.config();
  	// Need to run a cargo clean to identify all new .d deps files.
  	// TODO: Figure out how this can be avoided to improve performance, clean
  	// Rust builds are __slow__.
  	let clean_opt = CleanOptions {
  	config: &config,
  	spec: vec![],
  	target: None,
  	release: false,
  	doc: false,
  	};
  	cargo::ops::clean(ws, &clean_opt).map_err(|e| RsResolveError::Cargo(e.to_string()))?;
  	let inner_arc = Arc::new(Mutex::new(CustomExecutorInnerContext::default()));
  	{
  	let cust_exec = CustomExecutor {
  	cwd: config.cwd().to_path_buf(),
  	inner_ctx: inner_arc.clone(),
  	};
  	let exec: Arc<Executor> = Arc::new(cust_exec);
  	cargo::ops::compile_with_exec(ws, &copt, &exec)
  	.map_err(|e| RsResolveError::Cargo(e.to_string()))?;

• It uses a custom Cargo executor for that where it calls the underlying rustc compilation by itself - this is the point where it fails to compile a dependency (on the .exec(..) call) as it tries to do it with the toolchain specified in the dependency's rust-toolchain file:

  siderophile/src/deps.rs

  Lines 389 to 434 in c3e6c76

  	fn exec(
  	&self,
  	cmd: ProcessBuilder,
  	_id: PackageId,
  	_target: &Target,
  	_mode: CompileMode,
  	) -> CargoResult<()> {
  	let args = cmd.get_args();
  	let out_dir_key = OsString::from("--out-dir");
  	let out_dir_key_idx = args
  	.iter()
  	.position(|s| *s == out_dir_key)
  	.ok_or_else(|| CustomExecutorError::OutDirKeyMissing(cmd.to_string()))?;
  	let out_dir = args
  	.get(out_dir_key_idx + 1)
  	.ok_or_else(|| CustomExecutorError::OutDirValueMissing(cmd.to_string()))
  	.map(PathBuf::from)?;
  	// This can be different from the cwd used to launch the wrapping cargo
  	// plugin. Discovered while fixing
  	// https://github.com/anderejd/cargo-geiger/issues/19
  	let cwd = cmd
  	.get_cwd()
  	.map(PathBuf::from)
  	.unwrap_or_else(|| self.cwd.to_owned());
  	{
  	// Scope to drop and release the mutex before calling rustc.
  	let mut ctx = self
  	.inner_ctx
  	.lock()
  	.map_err(|e| CustomExecutorError::InnerContextMutex(e.to_string()))?;
  	for tuple in args
  	.iter()
  	.map(|s| (s, s.to_string_lossy().to_lowercase()))
  	.filter(|t| t.1.ends_with(".rs"))
  	{
  	let raw_path = cwd.join(tuple.0);
  	let p = raw_path
  	.canonicalize()
  	.map_err(|e| CustomExecutorError::Io(e, raw_path))?;
  	ctx.rs_file_args.insert(p);
  	}
  	ctx.out_dir_args.insert(out_dir);
  	}
  	cmd.exec()?;

(Imported from geiger-rs/cargo-geiger#50 for want of a call-stack graph).

Panics are very overused in libraries, and can unexpectedly crash unwary users and poison their mutexes. We need something to warn unsuspecting consumers which of their functions is most "explosive" / invokes the most panics, and which libraries are the cause. Without this, it's impossible for large codebases to be certain their threads won't panic and explode, nor even for small codebases that use complex libraries with many dependencies to be certain how explosive their calls truly are.

Ideally, panics provably unreachable by the compiler should not be reported. And a flag to suppress slicing panics is worthy of consideration.

siderophile naively looks for "src" occurring in the current Rust file path, and will sometimes overshoot the actual crate root. This sometimes gives output like

unsafe expr in function log::github::log_0::tests::filters::set_boxed_logger

since filters.rs presumably occurs in the tests/ directory of the log crate.

A neat fix would be to give a relative path to ast_walker::find_unsafe_in_file instead of an absolute path.

Example: siderophile would output

<rand_pcg::pcg128::Mcg128Xsl64 as RngCore>::next_u64

whereas the label in the crate's callgraph would be

<rand_pcg::pcg128::Mcg128Xsl64 as rand_core::RngCore>::next_u64

This might end up being quite hard to implement, since the tools only looks at source code, and trait path resolution happens at compile time.

Some useful one-liners for this in the comments of this: https://www.reddit.com/r/rust/comments/g9mw57/oneliner_to_correctly_list_all_uses_ofunsafe_in/

We should configure GitHub Actions to build and test Siderophile on every commit and PR, as well as on a periodic job.

TLDR: A given cargo rustc -- --emit=llvm-bc compilation might fail due to lack of target features specified in RUSTFLAGS e.g. RUSTFLAGS=-Ctarget-feature=+aes,+ssse3.

The analyze.sh should probably allow to specify this via CLI or even detect needed features of a given package if it is possible.

An easy workaround is to just prepend the flag into cargo rustc ... invocation.