From 5e50b41b5da99336230d2950da27b7638723937b Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 04:16:09 +0000 Subject: [PATCH 1/7] Also read the --emit flag to rustc --- cargo-auditable/src/rustc_arguments.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index 8c59556..0c7447c 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -1,6 +1,9 @@ //! Parses rustc arguments to extract the info not provided via environment variables. -use std::{ffi::OsString, path::PathBuf}; +use std::{ + ffi::{OsStr, OsString}, + path::PathBuf, +}; // We use pico-args because we only need to extract a few specific arguments out of a larger set, // and other parsers (rustc's `getopts`, cargo's `clap`) make that difficult. @@ -16,6 +19,7 @@ pub struct RustcArgs { pub crate_name: String, pub crate_types: Vec, pub cfg: Vec, + pub emit: Vec, pub out_dir: PathBuf, pub target: Option, pub print: Vec, @@ -37,12 +41,27 @@ impl RustcArgs { pub fn parse_args() -> Result { let raw_args: Vec = std::env::args_os().skip(2).collect(); + parse_args_inner(raw_args) +} + +// Split into its own function for unit testing +fn parse_args_inner(raw_args: Vec) -> Result { let mut parser = pico_args::Arguments::from_vec(raw_args); + // --emit requires slightly more complex parsing + let raw_emit_args: Vec = parser.values_from_str("--emit")?; + let mut emit: Vec = Vec::new(); + for raw_arg in raw_emit_args { + for item in raw_arg.split(',') { + emit.push(item.to_owned()); + } + } + Ok(RustcArgs { crate_name: parser.value_from_str("--crate-name")?, crate_types: parser.values_from_str("--crate-type")?, cfg: parser.values_from_str("--cfg")?, + emit, out_dir: parser.value_from_os_str::<&str, PathBuf, pico_args::Error>("--out-dir", |s| { Ok(PathBuf::from(s)) })?, From cca3a5d53c0767ddcec9b4037fbb525de20e70ea Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 04:25:42 +0000 Subject: [PATCH 2/7] Refactoring in preparation for fixes --- cargo-auditable/src/rustc_arguments.rs | 24 ++++++++++++++++++++++-- cargo-auditable/src/rustc_wrapper.rs | 10 +++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index 0c7447c..76d245e 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -41,11 +41,11 @@ impl RustcArgs { pub fn parse_args() -> Result { let raw_args: Vec = std::env::args_os().skip(2).collect(); - parse_args_inner(raw_args) + parse_args_from_vec(raw_args) } // Split into its own function for unit testing -fn parse_args_inner(raw_args: Vec) -> Result { +fn parse_args_from_vec(raw_args: Vec) -> Result { let mut parser = pico_args::Arguments::from_vec(raw_args); // --emit requires slightly more complex parsing @@ -69,3 +69,23 @@ fn parse_args_inner(raw_args: Vec) -> Result bool { + // Only inject audit data into crate types 'bin' and 'cdylib', + // it doesn't make sense for static libs and weird other types. + if !(args.crate_types.contains(&"bin".to_owned()) + || args.crate_types.contains(&"cdylib".to_owned())) + { + return false; + } + + //if !args.emit.is_empty() && !args.emit.contains("link".to_owned()) + + if ! args.print.is_empty() { + // --print disables compilation, + // UNLESS --emit is also explicitly specified + return false; + } + + true +} diff --git a/cargo-auditable/src/rustc_wrapper.rs b/cargo-auditable/src/rustc_wrapper.rs index 6431c0e..e1db295 100644 --- a/cargo-auditable/src/rustc_wrapper.rs +++ b/cargo-auditable/src/rustc_wrapper.rs @@ -7,7 +7,8 @@ use std::{ use crate::{ binary_file, collect_audit_data, platform_detection::{is_apple, is_msvc, is_wasm}, - rustc_arguments, target_info, + rustc_arguments::{self, should_embed_audit_data}, + target_info, }; use std::io::BufRead; @@ -20,12 +21,7 @@ pub fn main(rustc_path: &OsStr) { if env::var_os("CARGO_PRIMARY_PACKAGE").is_some() { let arg_parsing_result = rustc_arguments::parse_args(); if let Ok(args) = rustc_arguments::parse_args() { - // Only inject audit data into crate types 'bin' and 'cdylib', - // and only if --print is not specified (which disables compilation) - if args.print.is_empty() - && (args.crate_types.contains(&"bin".to_owned()) - || args.crate_types.contains(&"cdylib".to_owned())) - { + if should_embed_audit_data(&args) { // Get the audit data to embed let target_triple = args .target From 79a1af6f17dfc8f64c958c545d81f291ca80ea5b Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 04:31:22 +0000 Subject: [PATCH 3/7] Attempt to fix cargo-capi incompatibility --- cargo-auditable/src/rustc_arguments.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index 76d245e..c25dbbb 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -1,9 +1,6 @@ //! Parses rustc arguments to extract the info not provided via environment variables. -use std::{ - ffi::{OsStr, OsString}, - path::PathBuf, -}; +use std::{ffi::OsString, path::PathBuf}; // We use pico-args because we only need to extract a few specific arguments out of a larger set, // and other parsers (rustc's `getopts`, cargo's `clap`) make that difficult. @@ -79,13 +76,18 @@ pub fn should_embed_audit_data(args: &RustcArgs) -> bool { return false; } - //if !args.emit.is_empty() && !args.emit.contains("link".to_owned()) - - if ! args.print.is_empty() { - // --print disables compilation, - // UNLESS --emit is also explicitly specified + // when --emit is specified explicitly, only inject audit data for --emit=link + // because it doesn't make sense for all other types such as llvm-ir, asm, etc. + if !args.emit.is_empty() && !args.emit.contains(&"link".to_owned()) { return false; } + // --print disables compilation UNLESS --emit is also specified + if !args.print.is_empty() { + if args.emit.is_empty() { + return false; + } + } + true } From bea2ad15635f6f72af4f7cb85ddf6baffa6289a4 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 04:59:33 +0000 Subject: [PATCH 4/7] Actually fix cargo-c compatibility --- cargo-auditable/Cargo.toml | 2 +- cargo-auditable/src/rustc_arguments.rs | 70 +++++++++++++++++--------- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/cargo-auditable/Cargo.toml b/cargo-auditable/Cargo.toml index 42e163e..783becd 100644 --- a/cargo-auditable/Cargo.toml +++ b/cargo-auditable/Cargo.toml @@ -18,7 +18,7 @@ auditable-serde = {version = "0.8.0", path = "../auditable-serde"} miniz_oxide = {version = "0.8.0"} serde_json = "1.0.57" cargo_metadata = "0.18" -pico-args = "0.5" +pico-args = { version = "0.5", features = ["eq-separator"] } serde = "1.0.147" wasm-gen = "0.1.4" diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index c25dbbb..a75f719 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -12,6 +12,7 @@ use std::{ffi::OsString, path::PathBuf}; // https://github.com/rust-lang/rust/blob/26ecd44160f54395b3bd5558cc5352f49cb0a0ba/compiler/rustc_session/src/config.rs /// Includes only the rustc arguments we care about +#[derive(Debug)] pub struct RustcArgs { pub crate_name: String, pub crate_types: Vec, @@ -36,35 +37,38 @@ impl RustcArgs { } } -pub fn parse_args() -> Result { - let raw_args: Vec = std::env::args_os().skip(2).collect(); - parse_args_from_vec(raw_args) -} - -// Split into its own function for unit testing -fn parse_args_from_vec(raw_args: Vec) -> Result { - let mut parser = pico_args::Arguments::from_vec(raw_args); +impl RustcArgs { + // Split into its own function for unit testing + fn from_vec(raw_args: Vec) -> Result { + let mut parser = pico_args::Arguments::from_vec(raw_args); - // --emit requires slightly more complex parsing - let raw_emit_args: Vec = parser.values_from_str("--emit")?; - let mut emit: Vec = Vec::new(); - for raw_arg in raw_emit_args { - for item in raw_arg.split(',') { - emit.push(item.to_owned()); + // --emit requires slightly more complex parsing + let raw_emit_args: Vec = parser.values_from_str("--emit")?; + let mut emit: Vec = Vec::new(); + for raw_arg in raw_emit_args { + for item in raw_arg.split(',') { + emit.push(item.to_owned()); + } } + + Ok(RustcArgs { + crate_name: parser.value_from_str("--crate-name")?, + crate_types: parser.values_from_str("--crate-type")?, + cfg: parser.values_from_str("--cfg")?, + emit, + out_dir: parser + .value_from_os_str::<&str, PathBuf, pico_args::Error>("--out-dir", |s| { + Ok(PathBuf::from(s)) + })?, + target: parser.opt_value_from_str("--target")?, + print: parser.values_from_str("--print")?, + }) } +} - Ok(RustcArgs { - crate_name: parser.value_from_str("--crate-name")?, - crate_types: parser.values_from_str("--crate-type")?, - cfg: parser.values_from_str("--cfg")?, - emit, - out_dir: parser.value_from_os_str::<&str, PathBuf, pico_args::Error>("--out-dir", |s| { - Ok(PathBuf::from(s)) - })?, - target: parser.opt_value_from_str("--target")?, - print: parser.values_from_str("--print")?, - }) +pub fn parse_args() -> Result { + let raw_args: Vec = std::env::args_os().skip(2).collect(); + RustcArgs::from_vec(raw_args) } pub fn should_embed_audit_data(args: &RustcArgs) -> bool { @@ -91,3 +95,19 @@ pub fn should_embed_audit_data(args: &RustcArgs) -> bool { true } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cargo_c_compatibility() { + let raw_rustc_args = vec!["--crate-name", "rustls", "--edition=2021", "src/lib.rs", "--error-format=json", "--json=diagnostic-rendered-ansi,artifacts,future-incompat", "--crate-type", "staticlib", "--crate-type", "cdylib", "--emit=dep-info,link", "-C", "embed-bitcode=no", "-C", "debuginfo=2", "-C", "link-arg=-Wl,-soname,librustls.so.0.14.0", "-Cmetadata=rustls-ffi", "--cfg", "cargo_c", "--print", "native-static-libs", "--cfg", "feature=\"aws-lc-rs\"", "--cfg", "feature=\"capi\"", "--cfg", "feature=\"default\"", "--check-cfg", "cfg(docsrs)", "--check-cfg", "cfg(feature, values(\"aws-lc-rs\", \"capi\", \"cert_compression\", \"default\", \"no_log_capture\", \"read_buf\", \"ring\"))", "-C", "metadata=b6a43041f637feb8", "--out-dir", "/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps", "--target", "x86_64-unknown-linux-gnu", "-C", "linker=clang", "-C", "incremental=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/incremental", "-L", "dependency=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps", "-L", "dependency=/home/user/Code/rustls-ffi/target/debug/deps", "--extern", "libc=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/liblibc-4fc7c9f82dda33ee.rlib", "--extern", "log=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/liblog-6f7c8f4d1d5ec422.rlib", "--extern", "rustls=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls-a93cda0ba0380929.rlib", "--extern", "pki_types=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls_pki_types-27749859644f0979.rlib", "--extern", "rustls_platform_verifier=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls_platform_verifier-bceca5cf09f3d7ba.rlib", "--extern", "webpki=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/libwebpki-bc4a16dd84e0b062.rlib", "-C", "link-arg=-fuse-ld=/home/user/mold-2.32.0-x86_64-linux/bin/mold", "-L", "native=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/build/aws-lc-sys-d52f8990d9ede41d/out"]; + let raw_rustc_args: Vec = raw_rustc_args + .into_iter() + .map(|s| OsString::from(s)) + .collect(); + let args = RustcArgs::from_vec(raw_rustc_args).unwrap(); + assert!(should_embed_audit_data(&args)); + } +} From abbb29f9157621ad21f19f7633e5162d70e4e787 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 05:01:38 +0000 Subject: [PATCH 5/7] shorter code with cargo fmt --- cargo-auditable/src/rustc_arguments.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index a75f719..83f15cd 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -103,11 +103,9 @@ mod tests { #[test] fn cargo_c_compatibility() { let raw_rustc_args = vec!["--crate-name", "rustls", "--edition=2021", "src/lib.rs", "--error-format=json", "--json=diagnostic-rendered-ansi,artifacts,future-incompat", "--crate-type", "staticlib", "--crate-type", "cdylib", "--emit=dep-info,link", "-C", "embed-bitcode=no", "-C", "debuginfo=2", "-C", "link-arg=-Wl,-soname,librustls.so.0.14.0", "-Cmetadata=rustls-ffi", "--cfg", "cargo_c", "--print", "native-static-libs", "--cfg", "feature=\"aws-lc-rs\"", "--cfg", "feature=\"capi\"", "--cfg", "feature=\"default\"", "--check-cfg", "cfg(docsrs)", "--check-cfg", "cfg(feature, values(\"aws-lc-rs\", \"capi\", \"cert_compression\", \"default\", \"no_log_capture\", \"read_buf\", \"ring\"))", "-C", "metadata=b6a43041f637feb8", "--out-dir", "/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps", "--target", "x86_64-unknown-linux-gnu", "-C", "linker=clang", "-C", "incremental=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/incremental", "-L", "dependency=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps", "-L", "dependency=/home/user/Code/rustls-ffi/target/debug/deps", "--extern", "libc=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/liblibc-4fc7c9f82dda33ee.rlib", "--extern", "log=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/liblog-6f7c8f4d1d5ec422.rlib", "--extern", "rustls=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls-a93cda0ba0380929.rlib", "--extern", "pki_types=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls_pki_types-27749859644f0979.rlib", "--extern", "rustls_platform_verifier=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/librustls_platform_verifier-bceca5cf09f3d7ba.rlib", "--extern", "webpki=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/deps/libwebpki-bc4a16dd84e0b062.rlib", "-C", "link-arg=-fuse-ld=/home/user/mold-2.32.0-x86_64-linux/bin/mold", "-L", "native=/home/user/Code/rustls-ffi/target/x86_64-unknown-linux-gnu/debug/build/aws-lc-sys-d52f8990d9ede41d/out"]; - let raw_rustc_args: Vec = raw_rustc_args - .into_iter() - .map(|s| OsString::from(s)) - .collect(); + let raw_rustc_args: Vec = raw_rustc_args.into_iter().map(|s| s.into()).collect(); let args = RustcArgs::from_vec(raw_rustc_args).unwrap(); assert!(should_embed_audit_data(&args)); } + } From 043113503743e87c73526a43698331fed4fce0a5 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 05:07:43 +0000 Subject: [PATCH 6/7] add another --emit parsing test --- cargo-auditable/src/rustc_arguments.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index 83f15cd..0eba036 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -108,4 +108,27 @@ mod tests { assert!(should_embed_audit_data(&args)); } + #[test] + fn multiple_emit_values() { + let raw_rustc_args = vec![ + "--emit=dep-info,link", + "--emit", + "llvm-bc", + // end of interesting args, start of boilerplate + "--crate-name", + "foobar", + "--out-dir", + "/foo/bar", + ]; + let raw_rustc_args: Vec = raw_rustc_args.into_iter().map(|s| s.into()).collect(); + let mut args = RustcArgs::from_vec(raw_rustc_args).unwrap(); + + let expected = vec!["dep-info", "link", "llvm-bc"]; + let mut expected: Vec = expected.into_iter().map(|s| s.into()).collect(); + + args.emit.sort(); + expected.sort(); + + assert_eq!(args.emit, expected) + } } From c9e926fc7e56913fb4765dc70774eed71e3cb591 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 23 Nov 2024 05:12:18 +0000 Subject: [PATCH 7/7] apply clippy lint --- cargo-auditable/src/rustc_arguments.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cargo-auditable/src/rustc_arguments.rs b/cargo-auditable/src/rustc_arguments.rs index 0eba036..c73ed57 100644 --- a/cargo-auditable/src/rustc_arguments.rs +++ b/cargo-auditable/src/rustc_arguments.rs @@ -87,10 +87,8 @@ pub fn should_embed_audit_data(args: &RustcArgs) -> bool { } // --print disables compilation UNLESS --emit is also specified - if !args.print.is_empty() { - if args.emit.is_empty() { - return false; - } + if !args.print.is_empty() && args.emit.is_empty() { + return false; } true