Merge bitcoin/bitcoin#32158: fuzz: Make partially_downloaded_block more deterministic

fa51310 contrib: Warn about using libFuzzer for coverage check (MarcoFalke)
fa17cdb test: Avoid script check worker threads while fuzzing (MarcoFalke)
fa900bb contrib: Only print fuzz output on failure (MarcoFalke)
fa82fe2 contrib: Use -Xdemangler=llvm-cxxfilt in deterministic-*-coverage (MarcoFalke)
fa7e931 contrib: Add optional parallelism to deterministic-fuzz-coverage (MarcoFalke)

Pull request description:

  This should make the `partially_downloaded_block` fuzz target even more deterministic.

  Follow-up to bitcoin/bitcoin#31841. Tracking issue: bitcoin/bitcoin#29018.

  This bundles several changes:

  * First, speed up the `deterministic-fuzz-coverage` helper by introducing parallelism.
  * Then, a fix to remove spawned test threads or spawn them deterministically. (While testing this, high parallelism and thread contention may be needed)

  ### Testing

  It can be tested via (setting 32 parallel threads):

  ```
  cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ partially_downloaded_block 32
  ```

  Locally, on a failure, the output would look like:

  ```diff
   ....
  -  150|      0|            m_worker_threads.emplace_back([this, n]() {
  -  151|      0|                util::ThreadRename(strprintf("scriptch.%i", n));
  +  150|      1|            m_worker_threads.emplace_back([this, n]() {
  +  151|      1|                util::ThreadRename(strprintf("scriptch.%i", n));
   ...
  ```

  This excerpt likely indicates that the script threads were started after the fuzz init function returned.

  Similarly, for the scheduler thread, it would look like:

  ```diff
   ...
     227|      0|        m_node.scheduler = std::make_unique<CScheduler>();
  -  228|      1|        m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
  +  228|      0|        m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
     229|      0|        m_node.validation_signals =
   ...
  ```

ACKs for top commit:
  Prabhat1308:
    re-ACK [`fa51310`](bitcoin/bitcoin@fa51310)
  hodlinator:
    re-ACK fa51310
  janb84:
    Re-ACK [fa51310](bitcoin/bitcoin@fa51310)

Tree-SHA512: 1a935eb19da98c7c3810b8bcc5287e5649ffb55bf50ab78c414a424fef8e703839291bb24040a552c49274a4a0292910a00359bdff72fa29a4f53ad36d7a8720

Author: fanquake

contrib/devtools/deterministic-fuzz-coverage/src/main.rs

@@ -2,11 +2,13 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://opensource.org/license/mit/.
 
+use std::collections::VecDeque;
 use std::env;
-use std::fs::{read_dir, File};
+use std::fs::{read_dir, DirEntry, File};
 use std::path::{Path, PathBuf};
 use std::process::{Command, ExitCode};
 use std::str;
+use std::thread;
 
 /// A type for a complete and readable error message.
 type AppError = String;
@@ -16,12 +18,14 @@ const LLVM_PROFDATA: &str = "llvm-profdata";
 const LLVM_COV: &str = "llvm-cov";
 const GIT: &str = "git";
 
+const DEFAULT_PAR: usize = 1;
+
 fn exit_help(err: &str) -> AppError {
     format!(
         r#"
 Error: {err}
 
-Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name
+Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name [parallelism={DEFAULT_PAR}]
 
 Refer to the devtools/README.md for more details."#
     )
@@ -63,7 +67,14 @@ fn app() -> AppResult {
         // Require fuzz target for now. In the future it could be optional and the tool could
         // iterate over all compiled fuzz targets
         .ok_or(exit_help("Must set fuzz target"))?;
-    if args.get(4).is_some() {
+    let par = match args.get(4) {
+        Some(s) => s
+            .parse::<usize>()
+            .map_err(|e| exit_help(&format!("Could not parse parallelism as usize ({s}): {e}")))?,
+        None => DEFAULT_PAR,
+    }
+    .max(1);
+    if args.get(5).is_some() {
         Err(exit_help("Too many args"))?;
     }
 
@@ -73,7 +84,7 @@ fn app() -> AppResult {
 
     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, par)
 }
 
 fn using_libfuzzer(fuzz_exe: &Path) -> Result<bool, AppError> {
@@ -94,10 +105,14 @@ fn deterministic_coverage(
     corpora_dir: &Path,
     fuzz_exe: &Path,
     fuzz_target: &str,
+    par: usize,
 ) -> 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");
+    if using_libfuzzer {
+        println!("Warning: The fuzz executable was compiled with libFuzzer as sanitizer.");
+        println!("This tool may be tripped by libFuzzer misbehavior.");
+        println!("It is recommended to compile without libFuzzer.");
+    }
     let corpus_dir = corpora_dir.join(fuzz_target);
     let mut entries = read_dir(&corpus_dir)
         .map_err(|err| {
@@ -110,10 +125,12 @@ fn deterministic_coverage(
         .map(|entry| entry.expect("IO error"))
         .collect::<Vec<_>>();
     entries.sort_by_key(|entry| entry.file_name());
-    let run_single = |run_id: u8, entry: &Path| -> Result<PathBuf, AppError> {
-        let cov_txt_path = build_dir.join(format!("fuzz_det_cov.show.{run_id}.txt"));
-        if !{
-            {
+    let run_single = |run_id: char, entry: &Path, thread_id: usize| -> Result<PathBuf, AppError> {
+        let cov_txt_path = build_dir.join(format!("fuzz_det_cov.show.t{thread_id}.{run_id}.txt"));
+        let profraw_file = build_dir.join(format!("fuzz_det_cov.t{thread_id}.{run_id}.profraw"));
+        let profdata_file = build_dir.join(format!("fuzz_det_cov.t{thread_id}.{run_id}.profdata"));
+        {
+            let output = {
                 let mut cmd = Command::new(fuzz_exe);
                 if using_libfuzzer {
                     cmd.arg("-runs=1");
@@ -123,11 +140,15 @@ fn deterministic_coverage(
             .env("LLVM_PROFILE_FILE", &profraw_file)
             .env("FUZZ", fuzz_target)
             .arg(entry)
-            .status()
-            .map_err(|e| format!("fuzz failed with {e}"))?
-            .success()
-        } {
-            Err("fuzz failed".to_string())?;
+            .output()
+            .map_err(|e| format!("fuzz failed: {e}"))?;
+            if !output.status.success() {
+                Err(format!(
+                    "fuzz failed!\nstdout:\n{}\nstderr:\n{}\n",
+                    String::from_utf8_lossy(&output.stdout),
+                    String::from_utf8_lossy(&output.stderr)
+                ))?;
+            }
         }
         if !Command::new(LLVM_PROFDATA)
             .arg("merge")
@@ -149,6 +170,8 @@ fn deterministic_coverage(
                 "--show-line-counts-or-regions",
                 "--show-branches=count",
                 "--show-expansions",
+                "--show-instantiation-summary",
+                "-Xdemangler=llvm-cxxfilt",
                 &format!("--instr-profile={}", profdata_file.display()),
             ])
             .arg(fuzz_exe)
@@ -187,20 +210,46 @@ 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 {
+    println!(
+        "Check each fuzz input individually ... ({} inputs with parallelism {par})",
+        entries.len()
+    );
+    let check_individual = |entry: &DirEntry, thread_id: usize| -> AppResult {
         let entry = entry.path();
         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)?;
+        let cov_txt_base = run_single('a', &entry, thread_id)?;
+        let cov_txt_repeat = run_single('b', &entry, thread_id)?;
         check_diff(
             &cov_txt_base,
             &cov_txt_repeat,
             &format!("The fuzz target input was {}.", entry.display()),
         )?;
-    }
+        Ok(())
+    };
+    thread::scope(|s| -> AppResult {
+        let mut handles = VecDeque::with_capacity(par);
+        let mut res = Ok(());
+        for (i, entry) in entries.iter().enumerate() {
+            println!("[{}/{}]", i + 1, entries.len());
+            handles.push_back(s.spawn(move || check_individual(entry, i % par)));
+            while handles.len() >= par || i == (entries.len() - 1) || res.is_err() {
+                if let Some(th) = handles.pop_front() {
+                    let thread_result = match th.join() {
+                        Err(_e) => Err("A scoped thread panicked".to_string()),
+                        Ok(r) => r,
+                    };
+                    if thread_result.is_err() {
+                        res = thread_result;
+                    }
+                } else {
+                    return res;
+                }
+            }
+        }
+        res
+    })?;
     // 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.
@@ -209,23 +258,23 @@ The coverage was not deterministic between runs.
         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)?;
+        let cov_txt_base = run_single('a', &corpus_dir, 0)?;
+        let cov_txt_repeat = run_single('b', &corpus_dir, 0)?;
         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}.");
+    println!("✨ Coverage test passed for {fuzz_target}. ✨");
     Ok(())
 }
 
 fn main() -> ExitCode {
     match app() {
         Ok(()) => ExitCode::SUCCESS,
         Err(err) => {
-            eprintln!("{}", err);
+            eprintln!("⚠️\n{}", err);
             ExitCode::FAILURE
         }
     }

contrib/devtools/deterministic-unittest-coverage/src/main.rs

@@ -71,7 +71,7 @@ fn app() -> AppResult {
 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| -> Result<PathBuf, AppError> {
+    let run_single = |run_id: char| -> Result<PathBuf, AppError> {
         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)
@@ -104,6 +104,8 @@ fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) -> Ap
                 "--show-line-counts-or-regions",
                 "--show-branches=count",
                 "--show-expansions",
+                "--show-instantiation-summary",
+                "-Xdemangler=llvm-cxxfilt",
                 &format!("--instr-profile={}", profdata_file.display()),
             ])
             .arg(test_exe)
@@ -129,18 +131,18 @@ fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) -> Ap
         }
         Ok(())
     };
-    let r0 = run_single(0)?;
-    let r1 = run_single(1)?;
+    let r0 = run_single('a')?;
+    let r1 = run_single('b')?;
     check_diff(&r0, &r1)?;
-    println!("The coverage was deterministic across two runs.");
+    println!("✨ The coverage was deterministic across two runs. ✨");
     Ok(())
 }
 
 fn main() -> ExitCode {
     match app() {
         Ok(()) => ExitCode::SUCCESS,
         Err(err) => {
-            eprintln!("{}", err);
+            eprintln!("⚠️\n{}", err);
             ExitCode::FAILURE
         }
     }

src/test/util/setup_common.cpp

@@ -60,6 +60,7 @@
 #include <walletinitinterface.h>
 
 #include <algorithm>
+#include <future>
 #include <functional>
 #include <stdexcept>
 
@@ -230,6 +231,12 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
             // Use synchronous task runner while fuzzing to avoid non-determinism
             G_FUZZING ? std::make_unique<ValidationSignals>(std::make_unique<util::ImmediateTaskRunner>()) :
                         std::make_unique<ValidationSignals>(std::make_unique<SerialTaskRunner>(*m_node.scheduler));
+        {
+            // Ensure deterministic coverage by waiting for m_service_thread to be running
+            std::promise<void> promise;
+            m_node.scheduler->scheduleFromNow([&promise] { promise.set_value(); }, 0ms);
+            promise.get_future().wait();
+        }
     }
 
     bilingual_str error{};
@@ -247,7 +254,8 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
             .check_block_index = 1,
             .notifications = *m_node.notifications,
             .signals = m_node.validation_signals.get(),
-            .worker_threads_num = 2,
+            // Use no worker threads while fuzzing to avoid non-determinism
+            .worker_threads_num = G_FUZZING ? 0 : 2,
         };
         if (opts.min_validation_cache) {
             chainman_opts.script_execution_cache_bytes = 0;