From fa7d448375b19b0ff75fba84c41450b25a76a784 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 14 Mar 2025 19:45:20 +0100 Subject: [PATCH 1/2] contrib: Make deterministic-coverage error messages more readable --- contrib/devtools/README.md | 8 +- .../deterministic-fuzz-coverage/src/main.rs | 157 ++++++++++-------- .../src/main.rs | 114 ++++++++----- 3 files changed, 164 insertions(+), 115 deletions(-) diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 636067f7f2f..707a91c866d 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -8,7 +8,7 @@ deterministic-fuzz-coverage A tool to check for non-determinism in fuzz coverage. To get the help, run: ``` -RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- --help +cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- --help ``` To execute the tool, compilation has to be done with the build options: @@ -22,7 +22,7 @@ repository must have been cloned. Finally, a fuzz target has to be picked before running the tool: ``` -RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_dir $PWD/qa-assets/fuzz_corpora fuzz_target_name +cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_dir $PWD/qa-assets/fuzz_corpora fuzz_target_name ``` deterministic-unittest-coverage @@ -31,7 +31,7 @@ deterministic-unittest-coverage A tool to check for non-determinism in unit-test coverage. To get the help, run: ``` -RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- --help +cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- --help ``` To execute the tool, compilation has to be done with the build options: @@ -43,7 +43,7 @@ To execute the tool, compilation has to be done with the build options: Both llvm-profdata and llvm-cov must be installed. ``` -RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- $PWD/build_dir +cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- $PWD/build_dir ``` clang-format-diff.py diff --git a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs index 0e477ad03b5..69e733ec2b2 100644 --- a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs +++ b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs @@ -4,87 +4,99 @@ use std::env; use std::fs::{read_dir, File}; -use std::path::Path; -use std::process::{exit, Command}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitCode}; use std::str; +/// A type for a complete and readable error message. +type AppError = String; +type AppResult = Result<(), AppError>; + +fn main() -> ExitCode { + match app() { + Ok(()) => ExitCode::SUCCESS, + Err(err) => { + eprintln!("{}", err); + ExitCode::FAILURE + } + } +} + const LLVM_PROFDATA: &str = "llvm-profdata"; const LLVM_COV: &str = "llvm-cov"; const GIT: &str = "git"; -fn exit_help(err: &str) -> ! { - eprintln!("Error: {}", err); - eprintln!(); - eprintln!("Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name"); - eprintln!(); - eprintln!("Refer to the devtools/README.md for more details."); - exit(1) +fn exit_help(err: &str) -> AppError { + format!( + r#" +Error: {err} + +Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name + +Refer to the devtools/README.md for more details."# + ) } -fn sanity_check(corpora_dir: &Path, fuzz_exe: &Path) { +fn sanity_check(corpora_dir: &Path, fuzz_exe: &Path) -> AppResult { for tool in [LLVM_PROFDATA, LLVM_COV, GIT] { let output = Command::new(tool).arg("--help").output(); match output { Ok(output) if output.status.success() => {} - _ => { - exit_help(&format!("The tool {} is not installed", tool)); - } + _ => Err(exit_help(&format!("The tool {} is not installed", tool)))?, } } if !corpora_dir.is_dir() { - exit_help(&format!( + Err(exit_help(&format!( "Fuzz corpora path ({}) must be a directory", corpora_dir.display() - )); + )))?; } if !fuzz_exe.exists() { - exit_help(&format!( + Err(exit_help(&format!( "Fuzz executable ({}) not found", fuzz_exe.display() - )); + )))?; } + Ok(()) } -fn main() { +fn app() -> AppResult { // Parse args let args = env::args().collect::>(); - let build_dir = args - .get(1) - .unwrap_or_else(|| exit_help("Must set build dir")); + let build_dir = args.get(1).ok_or(exit_help("Must set build dir"))?; if build_dir == "--help" { - exit_help("--help requested") + Err(exit_help("--help requested"))?; } - let corpora_dir = args - .get(2) - .unwrap_or_else(|| exit_help("Must set fuzz corpora dir")); + let corpora_dir = args.get(2).ok_or(exit_help("Must set fuzz corpora dir"))?; let fuzz_target = args .get(3) // Require fuzz target for now. In the future it could be optional and the tool could // iterate over all compiled fuzz targets - .unwrap_or_else(|| exit_help("Must set fuzz target")); + .ok_or(exit_help("Must set fuzz target"))?; if args.get(4).is_some() { - exit_help("Too many args") + Err(exit_help("Too many args"))?; } let build_dir = Path::new(build_dir); let corpora_dir = Path::new(corpora_dir); let fuzz_exe = build_dir.join("bin/fuzz"); - sanity_check(corpora_dir, &fuzz_exe); + sanity_check(corpora_dir, &fuzz_exe)?; - deterministic_coverage(build_dir, corpora_dir, &fuzz_exe, fuzz_target); + deterministic_coverage(build_dir, corpora_dir, &fuzz_exe, fuzz_target) } -fn using_libfuzzer(fuzz_exe: &Path) -> bool { +fn using_libfuzzer(fuzz_exe: &Path) -> Result { println!("Check if using libFuzzer ..."); let stderr = Command::new(fuzz_exe) .arg("-help=1") // Will be interpreted as option (libfuzzer) or as input file .env("FUZZ", "addition_overflow") // Any valid target .output() - .expect("fuzz failed") + .map_err(|e| format!("fuzz failed with {e}"))? .stderr; - let help_output = str::from_utf8(&stderr).expect("The -help=1 output must be valid text"); - help_output.contains("libFuzzer") + let help_output = str::from_utf8(&stderr) + .map_err(|e| format!("The libFuzzer -help=1 output must be valid text ({e})"))?; + Ok(help_output.contains("libFuzzer")) } fn deterministic_coverage( @@ -92,25 +104,25 @@ fn deterministic_coverage( corpora_dir: &Path, fuzz_exe: &Path, fuzz_target: &str, -) { - let using_libfuzzer = using_libfuzzer(fuzz_exe); +) -> AppResult { + let using_libfuzzer = using_libfuzzer(fuzz_exe)?; let profraw_file = build_dir.join("fuzz_det_cov.profraw"); let profdata_file = build_dir.join("fuzz_det_cov.profdata"); let corpus_dir = corpora_dir.join(fuzz_target); let mut entries = read_dir(&corpus_dir) - .unwrap_or_else(|err| { + .map_err(|err| { exit_help(&format!( "The fuzz target's input directory must exist! ({}; {})", corpus_dir.display(), err )) - }) + })? .map(|entry| entry.expect("IO error")) .collect::>(); entries.sort_by_key(|entry| entry.file_name()); - let run_single = |run_id: u8, entry: &Path| { + let run_single = |run_id: u8, entry: &Path| -> Result { let cov_txt_path = build_dir.join(format!("fuzz_det_cov.show.{run_id}.txt")); - assert!({ + if !{ { let mut cmd = Command::new(fuzz_exe); if using_libfuzzer { @@ -122,20 +134,26 @@ fn deterministic_coverage( .env("FUZZ", fuzz_target) .arg(entry) .status() - .expect("fuzz failed") + .map_err(|e| format!("fuzz failed with {e}"))? .success() - }); - assert!(Command::new(LLVM_PROFDATA) + } { + Err("fuzz failed".to_string())?; + } + if !Command::new(LLVM_PROFDATA) .arg("merge") .arg("--sparse") .arg(&profraw_file) .arg("-o") .arg(&profdata_file) .status() - .expect("merge failed") - .success()); - let cov_file = File::create(&cov_txt_path).expect("Failed to create coverage txt file"); - assert!(Command::new(LLVM_COV) + .map_err(|e| format!("{LLVM_PROFDATA} merge failed with {e}"))? + .success() + { + Err(format!("{LLVM_PROFDATA} merge failed"))?; + } + let cov_file = File::create(&cov_txt_path) + .map_err(|e| format!("Failed to create coverage txt file ({e})"))?; + if !Command::new(LLVM_COV) .args([ "show", "--show-line-counts-or-regions", @@ -146,27 +164,31 @@ fn deterministic_coverage( .arg(fuzz_exe) .stdout(cov_file) .spawn() - .expect("Failed to execute llvm-cov") + .map_err(|e| format!("{LLVM_COV} show failed with {e}"))? .wait() - .expect("Failed to execute llvm-cov") - .success()); - cov_txt_path + .map_err(|e| format!("{LLVM_COV} show failed with {e}"))? + .success() + { + Err(format!("{LLVM_COV} show failed"))?; + }; + Ok(cov_txt_path) }; - let check_diff = |a: &Path, b: &Path, err: &str| { + let check_diff = |a: &Path, b: &Path, err: &str| -> AppResult { let same = Command::new(GIT) .args(["--no-pager", "diff", "--no-index"]) .arg(a) .arg(b) .status() - .expect("Failed to execute git command") + .map_err(|e| format!("{GIT} diff failed with {e}"))? .success(); if !same { - eprintln!(); - eprintln!("The coverage was not deterministic between runs."); - eprintln!("{}", err); - eprintln!("Exiting."); - exit(1); + Err(format!( + r#" +The coverage was not deterministic between runs. +{err}"# + ))?; } + Ok(()) }; // First, check that each fuzz input is deterministic running by itself in a process. // @@ -177,27 +199,32 @@ fn deterministic_coverage( // their overall coverage trace remains the same across runs and thus remains undetected. for entry in entries { let entry = entry.path(); - assert!(entry.is_file()); - let cov_txt_base = run_single(0, &entry); - let cov_txt_repeat = run_single(1, &entry); + if !entry.is_file() { + Err(format!("{} should be a file", entry.display()))?; + } + let cov_txt_base = run_single(0, &entry)?; + let cov_txt_repeat = run_single(1, &entry)?; check_diff( &cov_txt_base, &cov_txt_repeat, &format!("The fuzz target input was {}.", entry.display()), - ); + )?; } // Finally, check that running over all fuzz inputs in one process is deterministic as well. // This can catch issues where mutable global state is leaked from one fuzz input execution to // the next. { - assert!(corpus_dir.is_dir()); - let cov_txt_base = run_single(0, &corpus_dir); - let cov_txt_repeat = run_single(1, &corpus_dir); + if !corpus_dir.is_dir() { + Err(format!("{} should be a folder", corpus_dir.display()))?; + } + let cov_txt_base = run_single(0, &corpus_dir)?; + let cov_txt_repeat = run_single(1, &corpus_dir)?; check_diff( &cov_txt_base, &cov_txt_repeat, &format!("All fuzz inputs in {} were used.", corpus_dir.display()), - ); + )?; } println!("Coverage test passed for {fuzz_target}."); + Ok(()) } diff --git a/contrib/devtools/deterministic-unittest-coverage/src/main.rs b/contrib/devtools/deterministic-unittest-coverage/src/main.rs index 82863557a70..0d173ac4975 100644 --- a/contrib/devtools/deterministic-unittest-coverage/src/main.rs +++ b/contrib/devtools/deterministic-unittest-coverage/src/main.rs @@ -4,90 +4,110 @@ use std::env; use std::fs::File; -use std::path::Path; -use std::process::{exit, Command}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitCode}; use std::str; const LLVM_PROFDATA: &str = "llvm-profdata"; const LLVM_COV: &str = "llvm-cov"; const GIT: &str = "git"; -fn exit_help(err: &str) -> ! { - eprintln!("Error: {}", err); - eprintln!(); - eprintln!("Usage: program ./build_dir boost_unittest_filter"); - eprintln!(); - eprintln!("Refer to the devtools/README.md for more details."); - exit(1) +/// A type for a complete and readable error message. +type AppError = String; +type AppResult = Result<(), AppError>; + +fn main() -> ExitCode { + match app() { + Ok(()) => ExitCode::SUCCESS, + Err(err) => { + eprintln!("{}", err); + ExitCode::FAILURE + } + } } -fn sanity_check(test_exe: &Path) { +fn exit_help(err: &str) -> AppError { + format!( + r#" +Error: {err} + +Usage: program ./build_dir boost_unittest_filter + +Refer to the devtools/README.md for more details."# + ) +} + +fn sanity_check(test_exe: &Path) -> AppResult { for tool in [LLVM_PROFDATA, LLVM_COV, GIT] { let output = Command::new(tool).arg("--help").output(); match output { Ok(output) if output.status.success() => {} - _ => { - exit_help(&format!("The tool {} is not installed", tool)); - } + _ => Err(exit_help(&format!("The tool {} is not installed", tool)))?, } } if !test_exe.exists() { - exit_help(&format!( + Err(exit_help(&format!( "Test executable ({}) not found", test_exe.display() - )); + )))?; } + Ok(()) } -fn main() { +fn app() -> AppResult { // Parse args let args = env::args().collect::>(); - let build_dir = args - .get(1) - .unwrap_or_else(|| exit_help("Must set build dir")); + let build_dir = args.get(1).ok_or(exit_help("Must set build dir"))?; if build_dir == "--help" { - exit_help("--help requested") + Err(exit_help("--help requested"))?; } let filter = args .get(2) // Require filter for now. In the future it could be optional and the tool could provide a // default filter. - .unwrap_or_else(|| exit_help("Must set boost test filter")); + .ok_or(exit_help("Must set boost test filter"))?; if args.get(3).is_some() { - exit_help("Too many args") + Err(exit_help("Too many args"))?; } let build_dir = Path::new(build_dir); let test_exe = build_dir.join("bin/test_bitcoin"); - sanity_check(&test_exe); + sanity_check(&test_exe)?; - deterministic_coverage(build_dir, &test_exe, filter); + deterministic_coverage(build_dir, &test_exe, filter) } -fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) { +fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) -> AppResult { let profraw_file = build_dir.join("test_det_cov.profraw"); let profdata_file = build_dir.join("test_det_cov.profdata"); - let run_single = |run_id: u8| { + let run_single = |run_id: u8| -> Result { let cov_txt_path = build_dir.join(format!("test_det_cov.show.{run_id}.txt")); - assert!(Command::new(test_exe) + if !Command::new(test_exe) .env("LLVM_PROFILE_FILE", &profraw_file) .env("BOOST_TEST_RUN_FILTERS", filter) .env("RANDOM_CTX_SEED", "21") .status() - .expect("test failed") - .success()); - assert!(Command::new(LLVM_PROFDATA) + .map_err(|e| format!("test failed with {e}"))? + .success() + { + Err("test failed".to_string())?; + } + if !Command::new(LLVM_PROFDATA) .arg("merge") .arg("--sparse") .arg(&profraw_file) .arg("-o") .arg(&profdata_file) .status() - .expect("merge failed") - .success()); - let cov_file = File::create(&cov_txt_path).expect("Failed to create coverage txt file"); - assert!(Command::new(LLVM_COV) + .map_err(|e| format!("{LLVM_PROFDATA} merge failed with {e}"))? + .success() + { + Err(format!("{LLVM_PROFDATA} merge failed"))?; + } + let cov_file = File::create(&cov_txt_path) + .map_err(|e| format!("Failed to create coverage txt file ({e})"))?; + if !Command::new(LLVM_COV) .args([ "show", "--show-line-counts-or-regions", @@ -98,27 +118,29 @@ fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) { .arg(test_exe) .stdout(cov_file) .status() - .expect("llvm-cov failed") - .success()); - cov_txt_path + .map_err(|e| format!("{LLVM_COV} show failed with {e}"))? + .success() + { + Err(format!("{LLVM_COV} show failed"))?; + } + Ok(cov_txt_path) }; - let check_diff = |a: &Path, b: &Path| { + let check_diff = |a: &Path, b: &Path| -> AppResult { let same = Command::new(GIT) .args(["--no-pager", "diff", "--no-index"]) .arg(a) .arg(b) .status() - .expect("Failed to execute git command") + .map_err(|e| format!("{GIT} diff failed with {e}"))? .success(); if !same { - eprintln!(); - eprintln!("The coverage was not deterministic between runs."); - eprintln!("Exiting."); - exit(1); + Err("The coverage was not deterministic between runs.".to_string())?; } + Ok(()) }; - let r0 = run_single(0); - let r1 = run_single(1); - check_diff(&r0, &r1); + let r0 = run_single(0)?; + let r1 = run_single(1)?; + check_diff(&r0, &r1)?; println!("The coverage was deterministic across two runs."); + Ok(()) } From faf298b6da865e57a4fb3992938e3f0f25e3b767 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sat, 15 Mar 2025 09:09:42 +0100 Subject: [PATCH 2/2] contrib: Print deterministic-coverage runs --- contrib/devtools/deterministic-fuzz-coverage/src/main.rs | 2 ++ contrib/devtools/deterministic-unittest-coverage/src/main.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs index 69e733ec2b2..423393181ca 100644 --- a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs +++ b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs @@ -197,6 +197,7 @@ The coverage was not deterministic between runs. // // Also, This can catch issues where several fuzz inputs are non-deterministic, but the sum of // their overall coverage trace remains the same across runs and thus remains undetected. + println!("Check each fuzz input individually ..."); for entry in entries { let entry = entry.path(); if !entry.is_file() { @@ -213,6 +214,7 @@ The coverage was not deterministic between runs. // Finally, check that running over all fuzz inputs in one process is deterministic as well. // This can catch issues where mutable global state is leaked from one fuzz input execution to // the next. + println!("Check all fuzz inputs in one go ..."); { if !corpus_dir.is_dir() { Err(format!("{} should be a folder", corpus_dir.display()))?; diff --git a/contrib/devtools/deterministic-unittest-coverage/src/main.rs b/contrib/devtools/deterministic-unittest-coverage/src/main.rs index 0d173ac4975..2bae255e2b5 100644 --- a/contrib/devtools/deterministic-unittest-coverage/src/main.rs +++ b/contrib/devtools/deterministic-unittest-coverage/src/main.rs @@ -82,6 +82,7 @@ fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) -> Ap let profraw_file = build_dir.join("test_det_cov.profraw"); let profdata_file = build_dir.join("test_det_cov.profdata"); let run_single = |run_id: u8| -> Result { + println!("Run with id {run_id}"); let cov_txt_path = build_dir.join(format!("test_det_cov.show.{run_id}.txt")); if !Command::new(test_exe) .env("LLVM_PROFILE_FILE", &profraw_file)