From dd5f89e37d3558f19b947a1cf710bc9762dc5535 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Thu, 28 Jul 2022 22:51:44 -0700 Subject: [PATCH 1/6] Add a `--runtime` argument, which is used by `cargo add c2rust-analysis-rt` instead of the user having to manually do it. However, we have to use `+stable` as `cargo add` was added in 1.62 (on 1.60 now, but upgrading in https://github.com/immunant/c2rust/pull/513). --- analysis/test/Cargo.toml | 2 +- dynamic_instrumentation/src/main.rs | 22 ++++++++++++++++++++++ scripts/pdg.sh | 2 ++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/analysis/test/Cargo.toml b/analysis/test/Cargo.toml index 838e8ce06e..5055716415 100644 --- a/analysis/test/Cargo.toml +++ b/analysis/test/Cargo.toml @@ -7,4 +7,4 @@ edition = "2021" [dependencies] libc = "0.2" -c2rust-analysis-rt = { path = "../../analysis/runtime", optional = true } +c2rust-analysis-rt = { path = "../runtime", optional = true, version = "0.1.0" } diff --git a/dynamic_instrumentation/src/main.rs b/dynamic_instrumentation/src/main.rs index 9667f0613b..d69316325a 100644 --- a/dynamic_instrumentation/src/main.rs +++ b/dynamic_instrumentation/src/main.rs @@ -48,6 +48,10 @@ struct Args { #[clap(long, value_parser)] metadata: PathBuf, + /// Path to the `c2rust-analysis-rt` crate. + #[clap(long, value_parser)] + runtime: Option, + /// `cargo` args. cargo_args: Vec, } @@ -286,6 +290,7 @@ fn set_rust_toolchain() -> anyhow::Result<()> { fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { let Args { metadata, + runtime, mut cargo_args, } = Args::parse(); @@ -312,6 +317,23 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { cmd.args(&["clean", "--package", root_package.name.as_str()]); })?; + // TODO(kkysen) Once we upgrade to 1.62, we can just use `cargo` and not specify `+stable`. + // The problem currently is that `+stable` doesn't work on a resolved `$CARGO`, + // which happens when this runs inside of `cargo test`, + // and our current nightly is 1.60, which is before `cargo add` got added + // (though it's been in `cargo-edit` for a while). + Cargo { + path: "cargo".into(), + } + .run(|cmd| { + cmd.args(&["+stable", "add", "--optional", "c2rust-analysis-rt"]); + if let Some(runtime) = runtime { + // Since it's a local path, we don't need the internet, + // and running it offline saves a slow index sync. + cmd.args(&["--offline", "--path"]).arg(runtime); + } + })?; + // Create and truncate the metadata file for the [`rustc_wrapper`]s to append to. OpenOptions::new() .create(true) diff --git a/scripts/pdg.sh b/scripts/pdg.sh index 55fdf90a28..9554cc45bf 100755 --- a/scripts/pdg.sh +++ b/scripts/pdg.sh @@ -41,6 +41,7 @@ main() { local c2rust="${CWD}/${profile_dir}/c2rust" local c2rust_instrument="${CWD}/${profile_dir}/${instrument}" local metadata="${CWD}/${test_dir}/metadata.bc" + local runtime="${CWD}/analysis/runtime" (cd "${test_dir}" unset RUSTFLAGS # transpiled code has tons of warnings; don't allow `-D warnings` @@ -52,6 +53,7 @@ main() { time "${c2rust_instrument}" \ --metadata "${metadata}" \ + --runtime "${runtime}" \ -- run "${profile_args[@]}" \ -- "${args[@]}" \ 1> instrument.out.log From 8d2cc30365db052171cbc92482f3abb0fad79761 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Thu, 28 Jul 2022 23:13:20 -0700 Subject: [PATCH 2/6] Move the `cargo add` runtime to before any other `cargo` commands so that they work correctly always. --- dynamic_instrumentation/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dynamic_instrumentation/src/main.rs b/dynamic_instrumentation/src/main.rs index d69316325a..4ff9d06f32 100644 --- a/dynamic_instrumentation/src/main.rs +++ b/dynamic_instrumentation/src/main.rs @@ -316,7 +316,7 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { // it usually isn't that slow. cmd.args(&["clean", "--package", root_package.name.as_str()]); })?; - + // TODO(kkysen) Once we upgrade to 1.62, we can just use `cargo` and not specify `+stable`. // The problem currently is that `+stable` doesn't work on a resolved `$CARGO`, // which happens when this runs inside of `cargo test`, From 5bea2fddd2ce7ab38a8d3562879e408cb3cf95d1 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 16 Aug 2022 13:33:45 -0700 Subject: [PATCH 3/6] Added (off by default) `--set-runtime` so that the runtime is not `cargo add`ed unless explicitly opted-into. --- dynamic_instrumentation/src/main.rs | 31 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/dynamic_instrumentation/src/main.rs b/dynamic_instrumentation/src/main.rs index 4ff9d06f32..e951afa202 100644 --- a/dynamic_instrumentation/src/main.rs +++ b/dynamic_instrumentation/src/main.rs @@ -52,6 +52,10 @@ struct Args { #[clap(long, value_parser)] runtime: Option, + /// Add the runtime as an optional dependency to the instrumented crate using `cargo add`. + #[clap(long)] + set_runtime: bool, + /// `cargo` args. cargo_args: Vec, } @@ -291,6 +295,7 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { let Args { metadata, runtime, + set_runtime, mut cargo_args, } = Args::parse(); @@ -316,23 +321,17 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { // it usually isn't that slow. cmd.args(&["clean", "--package", root_package.name.as_str()]); })?; - - // TODO(kkysen) Once we upgrade to 1.62, we can just use `cargo` and not specify `+stable`. - // The problem currently is that `+stable` doesn't work on a resolved `$CARGO`, - // which happens when this runs inside of `cargo test`, - // and our current nightly is 1.60, which is before `cargo add` got added - // (though it's been in `cargo-edit` for a while). - Cargo { - path: "cargo".into(), + + if set_runtime { + cargo.run(|cmd| { + cmd.args(&["add", "--optional", "c2rust-analysis-rt"]); + if let Some(runtime) = runtime { + // Since it's a local path, we don't need the internet, + // and running it offline saves a slow index sync. + cmd.args(&["--offline", "--path"]).arg(runtime); + } + })?; } - .run(|cmd| { - cmd.args(&["+stable", "add", "--optional", "c2rust-analysis-rt"]); - if let Some(runtime) = runtime { - // Since it's a local path, we don't need the internet, - // and running it offline saves a slow index sync. - cmd.args(&["--offline", "--path"]).arg(runtime); - } - })?; // Create and truncate the metadata file for the [`rustc_wrapper`]s to append to. OpenOptions::new() From 5c5d98895956a5c4f7dfc7aac0ba152cf21825f1 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 16 Aug 2022 13:36:24 -0700 Subject: [PATCH 4/6] Clarified documentation for `--runtime`. --- dynamic_instrumentation/src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dynamic_instrumentation/src/main.rs b/dynamic_instrumentation/src/main.rs index e951afa202..51ba179801 100644 --- a/dynamic_instrumentation/src/main.rs +++ b/dynamic_instrumentation/src/main.rs @@ -48,7 +48,8 @@ struct Args { #[clap(long, value_parser)] metadata: PathBuf, - /// Path to the `c2rust-analysis-rt` crate. + /// Path to the `c2rust-analysis-rt` crate if you want to use a local version of it (vs. the crates.io one). + /// This is not used unless `--set-runtime` is also passed. #[clap(long, value_parser)] runtime: Option, From 9d89818f44016348ff96a554c646f6ba53a03198 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 16 Aug 2022 15:42:59 -0700 Subject: [PATCH 5/6] Renamed `--runtime` to `--runtime-path`. --- dynamic_instrumentation/src/main.rs | 6 +++--- scripts/pdg.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dynamic_instrumentation/src/main.rs b/dynamic_instrumentation/src/main.rs index 51ba179801..aa25d85ed6 100644 --- a/dynamic_instrumentation/src/main.rs +++ b/dynamic_instrumentation/src/main.rs @@ -51,7 +51,7 @@ struct Args { /// Path to the `c2rust-analysis-rt` crate if you want to use a local version of it (vs. the crates.io one). /// This is not used unless `--set-runtime` is also passed. #[clap(long, value_parser)] - runtime: Option, + runtime_path: Option, /// Add the runtime as an optional dependency to the instrumented crate using `cargo add`. #[clap(long)] @@ -295,7 +295,7 @@ fn set_rust_toolchain() -> anyhow::Result<()> { fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { let Args { metadata, - runtime, + runtime_path, set_runtime, mut cargo_args, } = Args::parse(); @@ -326,7 +326,7 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> { if set_runtime { cargo.run(|cmd| { cmd.args(&["add", "--optional", "c2rust-analysis-rt"]); - if let Some(runtime) = runtime { + if let Some(runtime) = runtime_path { // Since it's a local path, we don't need the internet, // and running it offline saves a slow index sync. cmd.args(&["--offline", "--path"]).arg(runtime); diff --git a/scripts/pdg.sh b/scripts/pdg.sh index 9554cc45bf..2e9fc5754b 100755 --- a/scripts/pdg.sh +++ b/scripts/pdg.sh @@ -53,7 +53,7 @@ main() { time "${c2rust_instrument}" \ --metadata "${metadata}" \ - --runtime "${runtime}" \ + --runtime-path "${runtime}" \ -- run "${profile_args[@]}" \ -- "${args[@]}" \ 1> instrument.out.log From ba996b3c0082a52bc0c35cc700790be7e32bfc92 Mon Sep 17 00:00:00 2001 From: Khyber Sen Date: Tue, 16 Aug 2022 15:43:24 -0700 Subject: [PATCH 6/6] Forgot to add `--set-runtime` in `pdg.sh`. --- scripts/pdg.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/pdg.sh b/scripts/pdg.sh index 2e9fc5754b..78f99a3df4 100755 --- a/scripts/pdg.sh +++ b/scripts/pdg.sh @@ -53,6 +53,7 @@ main() { time "${c2rust_instrument}" \ --metadata "${metadata}" \ + --set-runtime \ --runtime-path "${runtime}" \ -- run "${profile_args[@]}" \ -- "${args[@]}" \