Skip to content

Commit c20a5ce

Browse files
committed
Merge bitcoin#31901: contrib: Add deterministic-unittest-coverage
fa99c3b test: Exclude SeedStartup from coverage counts (MarcoFalke) fa579d6 contrib: Add deterministic-unittest-coverage (MarcoFalke) fa3940b contrib: deterministic-fuzz-coverage fixups (MarcoFalke) faf905b doc: Remove unused -fPIC (MarcoFalke) fa1e0a7 gitignore: target/ (MarcoFalke) Pull request description: The `contrib/devtools/test_deterministic_coverage.sh` script is problematic: * It is written in bash. This can lead to issues when running with the ancient bash version shipped by macOS by default, or can lead to other compatibility issues, such as bitcoin#31588 (comment). Also, pipefail isn't set, so IO errors may be silently ignored. * It is based on gcov. This can lead to issues, such as bitcoin#31588 (review) (possibly due to prefix-map), or bitcoin#31588 (comment) (gcovr processing error), or bitcoin#31588 (review) (gcovr assertion error). * The script is severely outdated, with the last update to `NON_DETERMINISTIC_TESTS` being in the prior decade. Instead of patching around all issues one-by-one, just provide a fresh rewrite, based on the recently added `deterministic-fuzz-coverage` tool based on clang, llvm-cov, and llvm-profdata. (Initial feedback indicates that this is a more promising attempt: bitcoin#31588 (comment) and bitcoin#31588 (comment)). The new tool also sets `RANDOM_CTX_SEED=21` as suggested by hodlinator in bitcoin#31588 (comment). ACKs for top commit: Prabhat1308: Concept ACK [`fa99c3b`](bitcoin@fa99c3b) hodlinator: re-ACK fa99c3b brunoerg: light ACK fa99c3b dergoegge: tACK fa99c3b janb84: Concept ACK [fa99c3b](bitcoin@fa99c3b) Tree-SHA512: 491d5e6413d929395a5c7caea54817bdc1a0e00562c9728a374d4e92f2e2017dba4a770ecdb2e7317e049df9fdeb390d83c90dff9aa5709f97aa3f6a0e70cdb4
2 parents a50af6e + fa99c3b commit c20a5ce

File tree

13 files changed

+227
-195
lines changed

13 files changed

+227
-195
lines changed

.gitignore

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
# Previous releases
1616
/releases
1717

18-
#build tests
19-
test/lint/test_runner/target/
18+
# cargo default target dir
19+
target/
2020

2121
/guix-build-*
2222

contrib/devtools/README.md

+31-7
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,39 @@ A tool to check for non-determinism in fuzz coverage. To get the help, run:
1111
RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- --help
1212
```
1313

14-
To execute the tool, compilation has to be done with the build options
15-
`-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++'
16-
-DBUILD_FOR_FUZZING=ON -DCMAKE_CXX_FLAGS='-fPIC -fprofile-instr-generate
17-
-fcoverage-mapping'`. Both llvm-profdata and llvm-cov must be installed. Also,
18-
the qa-assets repository must have been cloned. Finally, a fuzz target has to
19-
be picked before running the tool:
14+
To execute the tool, compilation has to be done with the build options:
2015

2116
```
22-
RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_dir $PWD/qa-assets/corpora-dir fuzz_target_name
17+
-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DBUILD_FOR_FUZZING=ON -DCMAKE_CXX_FLAGS='-fprofile-instr-generate -fcoverage-mapping'
18+
```
19+
20+
Both llvm-profdata and llvm-cov must be installed. Also, the qa-assets
21+
repository must have been cloned. Finally, a fuzz target has to be picked
22+
before running the tool:
23+
24+
```
25+
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
26+
```
27+
28+
deterministic-unittest-coverage
29+
===========================
30+
31+
A tool to check for non-determinism in unit-test coverage. To get the help, run:
32+
33+
```
34+
RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- --help
35+
```
36+
37+
To execute the tool, compilation has to be done with the build options:
38+
39+
```
40+
-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DCMAKE_CXX_FLAGS='-fprofile-instr-generate -fcoverage-mapping'
41+
```
42+
43+
Both llvm-profdata and llvm-cov must be installed.
44+
45+
```
46+
RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-unittest-coverage/Cargo.toml -- $PWD/build_dir <boost unittest filter>
2347
```
2448

2549
clang-format-diff.py

contrib/devtools/deterministic-fuzz-coverage/Cargo.lock

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

+13-16
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,25 @@
55
use std::env;
66
use std::fs::{read_dir, File};
77
use std::path::Path;
8-
use std::process::{exit, Command, Stdio};
8+
use std::process::{exit, Command};
99
use std::str;
1010

1111
const LLVM_PROFDATA: &str = "llvm-profdata";
1212
const LLVM_COV: &str = "llvm-cov";
13-
const DIFF: &str = "diff";
13+
const GIT: &str = "git";
1414

1515
fn exit_help(err: &str) -> ! {
1616
eprintln!("Error: {}", err);
1717
eprintln!();
18-
eprintln!("Usage: program ./build_dir ./qa-assets-corpora-dir fuzz_target");
18+
eprintln!("Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name");
1919
eprintln!();
2020
eprintln!("Refer to the devtools/README.md for more details.");
2121
exit(1)
2222
}
2323

2424
fn sanity_check(corpora_dir: &Path, fuzz_exe: &Path) {
25-
for tool in [LLVM_PROFDATA, LLVM_COV, DIFF] {
26-
let output = Command::new(tool).arg("--version").output();
25+
for tool in [LLVM_PROFDATA, LLVM_COV, GIT] {
26+
let output = Command::new(tool).arg("--help").output();
2727
match output {
2828
Ok(output) if output.status.success() => {}
2929
_ => {
@@ -135,7 +135,7 @@ fn deterministic_coverage(
135135
.expect("merge failed")
136136
.success());
137137
let cov_file = File::create(&cov_txt_path).expect("Failed to create coverage txt file");
138-
let passed = Command::new(LLVM_COV)
138+
assert!(Command::new(LLVM_COV)
139139
.args([
140140
"show",
141141
"--show-line-counts-or-regions",
@@ -144,34 +144,31 @@ fn deterministic_coverage(
144144
&format!("--instr-profile={}", profdata_file.display()),
145145
])
146146
.arg(fuzz_exe)
147-
.stdout(Stdio::from(cov_file))
147+
.stdout(cov_file)
148148
.spawn()
149149
.expect("Failed to execute llvm-cov")
150150
.wait()
151151
.expect("Failed to execute llvm-cov")
152-
.success();
153-
if !passed {
154-
panic!("Failed to execute llvm-profdata")
155-
}
152+
.success());
156153
cov_txt_path
157154
};
158155
let check_diff = |a: &Path, b: &Path, err: &str| {
159-
let same = Command::new(DIFF)
160-
.arg("--unified")
156+
let same = Command::new(GIT)
157+
.args(["--no-pager", "diff", "--no-index"])
161158
.arg(a)
162159
.arg(b)
163160
.status()
164-
.expect("Failed to execute diff command")
161+
.expect("Failed to execute git command")
165162
.success();
166163
if !same {
167164
eprintln!();
168-
eprintln!("The coverage was not determinstic between runs.");
165+
eprintln!("The coverage was not deterministic between runs.");
169166
eprintln!("{}", err);
170167
eprintln!("Exiting.");
171168
exit(1);
172169
}
173170
};
174-
// First, check that each fuzz input is determinisic running by itself in a process.
171+
// First, check that each fuzz input is deterministic running by itself in a process.
175172
//
176173
// This can catch issues and isolate where a single fuzz input triggers non-determinism, but
177174
// all other fuzz inputs are deterministic.

contrib/devtools/deterministic-unittest-coverage/Cargo.lock

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[package]
2+
name = "deterministic-unittest-coverage"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
[dependencies]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or https://opensource.org/license/mit/.
4+
5+
use std::env;
6+
use std::fs::File;
7+
use std::path::Path;
8+
use std::process::{exit, Command};
9+
use std::str;
10+
11+
const LLVM_PROFDATA: &str = "llvm-profdata";
12+
const LLVM_COV: &str = "llvm-cov";
13+
const GIT: &str = "git";
14+
15+
fn exit_help(err: &str) -> ! {
16+
eprintln!("Error: {}", err);
17+
eprintln!();
18+
eprintln!("Usage: program ./build_dir boost_unittest_filter");
19+
eprintln!();
20+
eprintln!("Refer to the devtools/README.md for more details.");
21+
exit(1)
22+
}
23+
24+
fn sanity_check(test_exe: &Path) {
25+
for tool in [LLVM_PROFDATA, LLVM_COV, GIT] {
26+
let output = Command::new(tool).arg("--help").output();
27+
match output {
28+
Ok(output) if output.status.success() => {}
29+
_ => {
30+
exit_help(&format!("The tool {} is not installed", tool));
31+
}
32+
}
33+
}
34+
if !test_exe.exists() {
35+
exit_help(&format!(
36+
"Test executable ({}) not found",
37+
test_exe.display()
38+
));
39+
}
40+
}
41+
42+
fn main() {
43+
// Parse args
44+
let args = env::args().collect::<Vec<_>>();
45+
let build_dir = args
46+
.get(1)
47+
.unwrap_or_else(|| exit_help("Must set build dir"));
48+
if build_dir == "--help" {
49+
exit_help("--help requested")
50+
}
51+
let filter = args
52+
.get(2)
53+
// Require filter for now. In the future it could be optional and the tool could provide a
54+
// default filter.
55+
.unwrap_or_else(|| exit_help("Must set boost test filter"));
56+
if args.get(3).is_some() {
57+
exit_help("Too many args")
58+
}
59+
60+
let build_dir = Path::new(build_dir);
61+
let test_exe = build_dir.join("src/test/test_bitcoin");
62+
63+
sanity_check(&test_exe);
64+
65+
deterministic_coverage(build_dir, &test_exe, filter);
66+
}
67+
68+
fn deterministic_coverage(build_dir: &Path, test_exe: &Path, filter: &str) {
69+
let profraw_file = build_dir.join("test_det_cov.profraw");
70+
let profdata_file = build_dir.join("test_det_cov.profdata");
71+
let run_single = |run_id: u8| {
72+
let cov_txt_path = build_dir.join(format!("test_det_cov.show.{run_id}.txt"));
73+
assert!(Command::new(test_exe)
74+
.env("LLVM_PROFILE_FILE", &profraw_file)
75+
.env("BOOST_TEST_RUN_FILTERS", filter)
76+
.env("RANDOM_CTX_SEED", "21")
77+
.status()
78+
.expect("test failed")
79+
.success());
80+
assert!(Command::new(LLVM_PROFDATA)
81+
.arg("merge")
82+
.arg("--sparse")
83+
.arg(&profraw_file)
84+
.arg("-o")
85+
.arg(&profdata_file)
86+
.status()
87+
.expect("merge failed")
88+
.success());
89+
let cov_file = File::create(&cov_txt_path).expect("Failed to create coverage txt file");
90+
assert!(Command::new(LLVM_COV)
91+
.args([
92+
"show",
93+
"--show-line-counts-or-regions",
94+
"--show-branches=count",
95+
"--show-expansions",
96+
&format!("--instr-profile={}", profdata_file.display()),
97+
])
98+
.arg(test_exe)
99+
.stdout(cov_file)
100+
.status()
101+
.expect("llvm-cov failed")
102+
.success());
103+
cov_txt_path
104+
};
105+
let check_diff = |a: &Path, b: &Path| {
106+
let same = Command::new(GIT)
107+
.args(["--no-pager", "diff", "--no-index"])
108+
.arg(a)
109+
.arg(b)
110+
.status()
111+
.expect("Failed to execute git command")
112+
.success();
113+
if !same {
114+
eprintln!();
115+
eprintln!("The coverage was not deterministic between runs.");
116+
eprintln!("Exiting.");
117+
exit(1);
118+
}
119+
};
120+
let r0 = run_single(0);
121+
let r1 = run_single(1);
122+
check_diff(&r0, &r1);
123+
println!("The coverage was deterministic across two runs.");
124+
}

0 commit comments

Comments
 (0)