The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31588.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #31901 (contrib: Add deterministic-unittest-coverage by maflcko)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

lgtm ACK 517d30b

I haven't tested this (I am using a modified script locally anyway), but the code changes look harmless and reasonable.

@fjahr: Executing the script with the commands mentioned in the PR description works for me on Ubuntu 22.04.03 LTS, didn't try it on any other machine yet. Which operating system are you running? (Some macOS I presume?) Some versions from my machine that would be relevant for comparison:

$ cat /etc/lsb-release | tail -n1
DISTRIB_DESCRIPTION="Ubuntu 22.04.3 LTS"
$ gcovr --version | head -n1
gcovr 5.0
$ gcc --version | head -n1
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
$ bash --version
GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
$ wc --version
wc (GNU coreutils) 8.32

Another potential cause for failure that comes to my mind is that you possibly compile with clang. In that case, the GCOV_EXECUTABLE variable has to be changed manually in the script:

Yeah, I'm using macOS Sequoia 15.3 and I am also using clang but I have already been using GCOV_EXECUTABLE from the start.

Checking some of the other versions:

$ gcovr --version | head -n1
gcovr 8.3
$ clang --version | head -n1
Homebrew clang version 19.1.6
$ bash --version
GNU bash, version 5.2.37(1)-release (aarch64-apple-darwin24.0.0)

The built-in wc doesn't seem to provide version information but it is supposed to be taken from BSD tools.

I tried compiling with GCC 14 as well now. It's not giving me any loud errors but it's still not working either:

$ GCOV_EXECUTABLE="gcov-14" BUILDDIR="build" contrib/devtools/test_deterministic_coverage.sh
[2025-02-08 00:00:50] Measuring coverage, run #1 of 2
(INFO) Reading coverage data...
(INFO) Writing coverage report...
Error: Spurious gcovr output. Make sure the correct GCOV_EXECUTABLE variable is set in contrib/devtools/test_deterministic_coverage.sh ("gcov" for gcc, "llvm-cov gcov" for clang).

@fjahr: Note that GCOV_EXECUTABLE is hardcoded in the script currently, so setting it as environment variable won't have any effect. Can you try again by modifying it directly in the file? It's not very user friendly that this is necessary, can add a commit that allows passing it via environment, if reviewers feel that this is still in scope of this PR.

Note that GCOV_EXECUTABLE is hardcoded in the script currently, so setting it as environment variable won't have any effect.

Ah, I had indeed missed that. I tried this for llvm but it didn't change the outcome, seems to be the same issue. I didn't investigate further there and instead tried again with gcc-14, given this is at least a bit closer to your tooling. I get another error there with this. It appears the compile time macros are confusing gcov and it sees secp256k1_scalar_split_lambda defined twice. I'm not even sure if it should be considering secp at all since this is just to get coverage of our code, right?

Here is the error I'm getting:

$ BUILDDIR="build" contrib/devtools/test_deterministic_coverage.sh
[2025-02-09 14:45:24] Measuring coverage, run #1 of 2
(INFO) Reading coverage data...
(ERROR) Traceback (most recent call last):
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/workers.py", line 95, in worker
    work(*args, **kwargs)
    ~~~~^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/read.py", line 471, in process_datafile
    done = run_gcov_and_process_files(
        abs_filename,
    ...<3 lines>...
        chdir=wd,
    )
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/read.py", line 836, in run_gcov_and_process_files
    process_gcov_json_data(gcov_filename, covdata, options)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/read.py", line 210, in process_gcov_json_data
    insert_file_coverage(covdata, file_cov, get_merge_mode_from_options(options))
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/merging.py", line 250, in insert_file_coverage
    return _insert_coverage_item(
        target.data, file.filename, file, merge_file, options, None
    )
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/merging.py", line 222, in _insert_coverage_item
    merged_item = merge_item(target_dict[key], new_item, options, context)
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/merging.py", line 295, in merge_file
    raise AssertionError("\n".join(message)) from None
AssertionError: Got function secp256k1_scalar_split_lambda in /Users/XXX/projects/clones/bitcoin/src/secp256k1/src/scalar_impl.h on multiple lines: 67, 142.
	You can run gcovr with --merge-mode-functions=MERGE_MODE.
	The available values for MERGE_MODE are described in the documentation.
GCOV source files of merge source is/are:
	/Users/XXX/projects/clones/bitcoin/src/tests.c##63f67a257b368dda78c4068542388473.gcov.json.gz
and of merge target is/are:
	/Users/XXX/projects/clones/bitcoin/src/tests_exhaustive.c##a46642766247b4021fb41c87898a129a.gcov.json.gz

(ERROR) Error occurred while reading reports:
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/__main__.py", line 384, in main
    covdata = gcovr_formats.read_reports(options)
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/__init__.py", line 99, in read_reports
    covdata = GcovHandler(options).read_report()
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/__init__.py", line 233, in read_report
    return read_report(self.options)
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/read.py", line 96, in read_report
    contexts = pool.wait()
  File "/opt/homebrew/Cellar/gcovr/8.3/libexec/lib/python3.13/site-packages/gcovr/formats/gcov/workers.py", line 181, in wait
    raise RuntimeError(
        "Worker thread raised exception, workers canceled."
    ) from None
RuntimeError: Worker thread raised exception, workers canceled.

Error: gcovr failed. Output written to build/deterministic_coverage_gcovr_output.tmp. Exiting.

lgtm ACK 459683b

Only change is a small mktemp cleanup.

I still haven't tested this (I am using a modified script locally anyway), but the code changes look harmless and reasonable. As for the scope, I wonder how much effort should be spent on this script, given that:

• No one(?) is using it right now unmodified, or has been in the past years?
• It is written in bash, causing compat issues with the ancient version of bash shipped on macOS, as well as the general bash issues (like error handling).
• In light of [POC] cmake: Introduce LLVM's Source-based Code Coverage reports #31394, it is unclear whether or not it would be stale again anyway.

Given all the GCC/lcov/gcov problems, I wonder if it wouldn't be easier to just go with clang/llvm:

• Require compilation with -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DCMAKE_CXX_FLAGS='-fPIC -fprofile-instr-generate -fcoverage-mapping'
• apply the below diff

diff --git a/contrib/devtools/test_deterministic_coverage.sh b/contrib/devtools/test_deterministic_coverage.sh
index 885396bb25..48d331ae70 100755
--- a/contrib/devtools/test_deterministic_coverage.sh
+++ b/contrib/devtools/test_deterministic_coverage.sh
@@ -8,9 +8,7 @@
 
 export LC_ALL=C
 
-# Use GCOV_EXECUTABLE="gcov" if compiling with gcc.
-# Use GCOV_EXECUTABLE="llvm-cov gcov" if compiling with clang.
-GCOV_EXECUTABLE="gcov"
+BUILD_DIR="./bld-cmake"
 
 # Disable tests known to cause non-deterministic behaviour and document the source or point of non-determinism.
 NON_DETERMINISTIC_TESTS=(
@@ -34,7 +32,9 @@ NON_DETERMINISTIC_TESTS=(
     "wallet_tests/wallet_disableprivkeys"                     # validation.cpp: if (signals.CallbacksPending() > 10)
 )
 
-TEST_BITCOIN_BINARY="src/test/test_bitcoin"
+TEST_BITCOIN_BINARY="${BUILD_DIR}/src/test/test_bitcoin"
+LLVM_PROFRAW_FILE="${BUILD_DIR}/test_det_cov.profraw"
+LLVM_PROFDATA_FILE="${BUILD_DIR}/test_det_cov.profdata"
 
 print_usage() {
     echo "Usage: $0 [custom test filter (default: all but known non-deterministic tests)] [number of test runs (default: 2)]"
@@ -65,18 +65,17 @@ if [[ $# != 0 ]]; then
 fi
 if [[ ${BOOST_TEST_RUN_FILTERS} == "" ]]; then
     BOOST_TEST_RUN_FILTERS="$(IFS=":"; echo "!${NON_DETERMINISTIC_TESTS[*]}" | sed 's/:/:!/g')"
-else
-    echo "Using Boost test filter: ${BOOST_TEST_RUN_FILTERS}"
-    echo
 fi
+echo "Using Boost test filter: ${BOOST_TEST_RUN_FILTERS}"
+echo
 
-if ! command -v gcov > /dev/null; then
-    echo "Error: gcov not installed. Exiting."
+if ! command -v llvm-profdata > /dev/null; then
+    echo "Error: llvm-profdata not installed. Exiting."
     exit 1
 fi
 
-if ! command -v gcovr > /dev/null; then
-    echo "Error: gcovr not installed. Exiting."
+if ! command -v llvm-cov > /dev/null; then
+    echo "Error: llvm-cov not installed. Exiting."
     exit 1
 fi
 
@@ -85,52 +84,41 @@ if [[ ! -e ${TEST_BITCOIN_BINARY} ]]; then
     exit 1
 fi
 
-get_file_suffix_count() {
-    find src/ -type f -name "*.$1" | wc -l
-}
-
-if [[ $(get_file_suffix_count gcno) == 0 ]]; then
-    echo "Error: Could not find any *.gcno files. The *.gcno files are generated by the compiler. Run \"cmake -B build -DCMAKE_BUILD_TYPE=Coverage\" and re-compile."
-    exit 1
-fi
-
-get_covr_filename() {
-    echo "gcovr.run-$1.txt"
-}
+
+
+
+
 
 TEST_RUN_ID=0
 while [[ ${TEST_RUN_ID} -lt ${N_TEST_RUNS} ]]; do
     TEST_RUN_ID=$((TEST_RUN_ID + 1))
     echo "[$(date +"%Y-%m-%d %H:%M:%S")] Measuring coverage, run #${TEST_RUN_ID} of ${N_TEST_RUNS}"
-    find src/ -type f -name "*.gcda" -exec rm {} \;
-    if [[ $(get_file_suffix_count gcda) != 0 ]]; then
-        echo "Error: Stale *.gcda files found. Exiting."
-        exit 1
-    fi
-    TEST_OUTPUT_TEMPFILE=$(mktemp)
-    if ! BOOST_TEST_RUN_FILTERS="${BOOST_TEST_RUN_FILTERS}" ${TEST_BITCOIN_BINARY} > "${TEST_OUTPUT_TEMPFILE}" 2>&1; then
+
+    TEST_OUTPUT_TEMPFILE="$BUILD_DIR/test_det_cov_out.tmp"
+    LLVM_PROFILE_FILE="${LLVM_PROFRAW_FILE}" BOOST_TEST_RUN_FILTERS="${BOOST_TEST_RUN_FILTERS}" ${TEST_BITCOIN_BINARY} > "${TEST_OUTPUT_TEMPFILE}" 2>&1
+    if [[ $? -ne 0 ]]; then
         cat "${TEST_OUTPUT_TEMPFILE}"
         rm "${TEST_OUTPUT_TEMPFILE}"
         exit 1
     fi
     rm "${TEST_OUTPUT_TEMPFILE}"
-    if [[ $(get_file_suffix_count gcda) == 0 ]]; then
-        echo "Error: Running the test suite did not create any *.gcda files. The gcda files are generated when the instrumented test programs are executed. Run \"cmake -B build -DCMAKE_BUILD_TYPE=Coverage\" and re-compile."
-        exit 1
-    fi
-    GCOVR_TEMPFILE=$(mktemp)
-    if ! gcovr --gcov-executable "${GCOV_EXECUTABLE}" -r src/ > "${GCOVR_TEMPFILE}"; then
-        echo "Error: gcovr failed. Output written to ${GCOVR_TEMPFILE}. Exiting."
+
+    if [[ ! -f ${LLVM_PROFRAW_FILE} ]]; then
+        echo "Error: Profiling data not generated. Ensure the binary is compiled with Clang coverage flags."
         exit 1
     fi
-    GCOVR_FILENAME=$(get_covr_filename ${TEST_RUN_ID})
-    mv "${GCOVR_TEMPFILE}" "${GCOVR_FILENAME}"
-    if grep -E "^TOTAL *0 *0 " "${GCOVR_FILENAME}"; then
-        echo "Error: Spurious gcovr output. Make sure the correct GCOV_EXECUTABLE variable is set in $0 (\"gcov\" for gcc, \"llvm-cov gcov\" for clang)."
+
+    llvm-profdata merge --sparse "${LLVM_PROFRAW_FILE}" -o "${LLVM_PROFDATA_FILE}"
+    if [[ $? -ne 0 ]]; then
+        echo "Error: llvm-profdata failed to generate profiling data. Exiting."
         exit 1
     fi
+
+    COVERAGE_FILENAME="${BUILD_DIR}/test_det_cov.run-${TEST_RUN_ID}.txt"
+    llvm-cov show "${TEST_BITCOIN_BINARY}" -instr-profile="${LLVM_PROFDATA_FILE}" > "${COVERAGE_FILENAME}"
+
     if [[ ${TEST_RUN_ID} != 1 ]]; then
-        COVERAGE_DIFF=$(diff -u "$(get_covr_filename 1)" "${GCOVR_FILENAME}")
+        COVERAGE_DIFF=$(diff -u "${BUILD_DIR}/test_det_cov.run-1.txt" "${COVERAGE_FILENAME}")
         if [[ ${COVERAGE_DIFF} != "" ]]; then
             echo
             echo "The line coverage is non-deterministic between runs. Exiting."
@@ -142,7 +130,7 @@ while [[ ${TEST_RUN_ID} -lt ${N_TEST_RUNS} ]]; do
             echo "${COVERAGE_DIFF}"
             exit 1
         fi
-        rm "${GCOVR_FILENAME}"
+        rm "${COVERAGE_FILENAME}"
     fi
 done
 

But no strong opinion. I think anything is fine here and I've already reviewed this pull.

Succeeded with maflcko's diff applied to master (modified build dir).

Got a big diff, indicating a lot of test non-determinism:

[2025-02-10 22:32:56] Measuring coverage, run #1 of 2
[2025-02-10 22:35:26] Measuring coverage, run #2 of 2

The line coverage is non-deterministic between runs. Exiting.

The test suite must be deterministic in the sense that the set of lines executed at least
once must be identical between runs. This is a necessary condition for meaningful
coverage measuring.

--- ./build/test_det_cov.run-1.txt	2025-02-10 22:35:26.017552349 +0100
+++ ./build/test_det_cov.run-2.txt	2025-02-10 22:37:53.626966257 +0100
@@ -1980,19 +1980,19 @@
   751|       |
   752|       |    // Loop through the addrman table until we find an appropriate entry
   753|     78|    double chance_factor = 1.0;
-  754|  85.7k|    while (1) {
+  754|  85.3k|    while (1) {
... ~39k+ lines omitted

Don't have a strong preference either really (GCC/LLVM). Seems like a cool concept to have this if only people thought to chip away at what it finds, and CI stats somewhere graphed the overall size of the diff.

I wonder if it wouldn't be easier to just go with clang/llvm

I've tested the suggested diff and it worked for me. llvm/clang is my default anyway so I would give this a Concept ACK if you want to follow this approach @theStack

Got a big diff, indicating a lot of test non-determinism:

I haven't looked in detail, but I presume much is due to randomness. On one hand, one could go with #31486 for the unit tests as well. On the other hand, some unit tests are fuzz tests (drawing inputs from a random seed, which is recorded), so they'd be a bit more limited if the randomness was made deterministic. But I think either way is fine.

Got a big diff, indicating a lot of test non-determinism:

I haven't looked in detail, but I presume much is due to randomness. On one hand, one could go with #31486 for the unit tests as well. On the other hand, some unit tests are fuzz tests (drawing inputs from a random seed, which is recorded), so they'd be a bit more limited if the randomness was made deterministic. But I think either way is fine.

This gives me an 8.4K line diff instead of 39k, so there is some hope:

RANDOM_CTX_SEED=21 ./contrib/devtools/test_deterministic_coverage.sh

I wonder if it wouldn't be easier to just go with clang/llvm

I've tested the suggested diff and it worked for me. llvm/clang is my default anyway so I would give this a Concept ACK if you want to follow this approach @theStack

Concept ACK on switching over to clang/llvm as well considering all the reported problems. Note though that this PR merely tries to restore the previous behaviour of the script for CMake builds (and minor improvements that are enabled by that), so I think a deeper change is out of scope.
@maflcko: Do you want to open a PR? Happy to close this one and test then. I'd rather not include a diff here that I don't understand myself, and there are seemingly additional changes needed, like documenting the needed build flags somewhere.

Sure, but it may take some time until I get around to it.

In any case, I am sure this script is broken or at least stale for years and shouldn't be a blocker for 29.0 (even if it was used, it is a dev-only tool). So I'll be removing the milestone for now.

Done in #31901

Maybe this can be closed, given that #31901 has 3 acks and is likely close to merge?

Closing in favour of #31901.