docs: Exclude modules' `bench_impl.h` headers from coverage report

hebasto commented at 4:12 pm on August 10, 2025: member

Additionally, this comment has been addressed.

autotools, docs: Adjust help string for `--enable-coverage` option a9e955d3ea

hebasto commented at 4:16 pm on August 10, 2025: member

FWIW, on Ubuntu 24.04, when using GCC 13.3.0 and gcovr 7.0, I need to pass an extra --merge-mode-functions=separate option to generate a coverage report.

real-or-random added the label assurance on Aug 11, 2025

real-or-random added the label meta/development on Aug 11, 2025

real-or-random commented at 9:01 am on August 11, 2025: contributor

FWIW, on Ubuntu 24.04, when using GCC 13.3.0 and gcovr 7.0, I need to pass an extra --merge-mode-functions=separate option to generate a coverage report.

Let’s add it, and I also need --gcov-suspicious-hits-threshold=140737488355330 (because we hit some lines in int128_impl.h more than 2^32 times…) but this was introduced only in gcovr 8.3. I assume older versions error out when they get this flag?

doc: Exclude modules' `bench_impl.h` headers from coverage report 106a7cbf41

doc: Add `--merge-mode-functions=separate` option to `gcovr` invocations

Otherwise, commands fail with the error:
```
Stderr of gcov was >><< End of stderr
Exception was >>Got function secp256k1_scalar_split_lambda 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.<< End of stderr
```

1aecce5936

hebasto force-pushed on Aug 11, 2025

hebasto commented at 10:37 am on August 11, 2025: member

FWIW, on Ubuntu 24.04, when using GCC 13.3.0 and gcovr 7.0, I need to pass an extra --merge-mode-functions=separate option to generate a coverage report.

Let’s add it…

Thanks! Done.

… and I also need --gcov-suspicious-hits-threshold=140737488355330 (because we hit some lines in int128_impl.h more than 2^32 times…) but this was introduced only in gcovr 8.3. I assume older versions error out when they get this flag?

Yes, it outputs:

0usage: gcovr [options] [search_paths...]
1gcovr: error: unrecognized arguments: --gcov-suspicious-hits-threshold=140737488355330

real-or-random commented at 12:06 pm on August 11, 2025: contributor

Yes, it outputs:

0usage: gcovr [options] [search_paths...]
1gcovr: error: unrecognized arguments: --gcov-suspicious-hits-threshold=140737488355330

Then I guess the best thing for now is to add a note/comment that this should be omitted on gcovr before 8.3.

hebasto commented at 2:27 pm on August 11, 2025: member

FWIW, on Ubuntu 24.04, when using GCC 13.3.0 and gcovr 7.0, I need to pass an extra --merge-mode-functions=separate option to generate a coverage report.

Let’s add it, and I also need --gcov-suspicious-hits-threshold=140737488355330 (because we hit some lines in int128_impl.h more than 2^32 times…) but this was introduced only in gcovr 8.3. I assume older versions error out when they get this flag?

What GCC version are you using? Any other specific steps to reproduce the issue?

EDIT: nm, I can reproduce it on Fedora 42 with GCC 15.1.1.

real-or-random commented at 3:13 pm on August 11, 2025: contributor

I think an alternative for <8.3 is --gcov-ignore-parse-errors=suspicious_hits.warn, which should work on >=6.0 if I understand the changelog correctly.

Fwiw:

0gcovr 8.4.dev0+gfe536afac.d20250125
1gcc (GCC) 15.1.1 20250729

hebasto commented at 3:25 pm on August 11, 2025: member

I think an alternative for <8.3 is --gcov-ignore-parse-errors=suspicious_hits.warn, which should work on >=6.0 if I understand the changelog correctly.

It was introduced in https://github.com/gcovr/gcovr/issues/898 and has been available since version 8.0.

The --gcov-ignore-parse-errors=all option can be used instead.

hebasto commented at 3:34 pm on August 11, 2025: member

The --gcov-ignore-parse-errors=all option can be used instead.

Taken this:

 0$ gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
 1(INFO) Reading coverage data...
 2(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_accum_mul(secp256k1_uint128 *r, uint64_t a, uint64_t b) {'.
 3(WARNING) Ignoring suspicious hits in line '   *r += (uint128_t)a * b;'.
 4(WARNING) Ignoring suspicious hits in line '}'.
 5(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE uint64_t secp256k1_u128_to_u64(const secp256k1_uint128 *a) {'.
 6(WARNING) Ignoring suspicious hits in line '   return (uint64_t)(*a);'.
 7(WARNING) Ignoring suspicious hits in line '                for (p = 0; p < 16; ++p) { /* p loops over the bit positions in mul2[j]. */'.
 8(WARNING) Ignoring suspicious hits in line '                for (p = 0; p < 16; ++p) { /* p loops over the bit positions in mul2[j]. */'.
 9(WARNING) Ignoring suspicious hits in line '                    int bitpos = j * 16 - i + p; /* bitpos is the correspond bit position in m. */'.
10(WARNING) Ignoring suspicious hits in line '                    if (bitpos >= 0 && bitpos < 256) {'.
11(WARNING) Ignoring suspicious hits in line '                    if (bitpos >= 0 && bitpos < 256) {'.
12(WARNING) Ignoring suspicious hits in line '                    if (bitpos >= 0 && bitpos < 256) {'.
13(WARNING) Ignoring suspicious hits in line '                        sub |= ((m[bitpos >> 4] >> (bitpos & 15)) & 1) << p;'.
14(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_accum_mul(secp256k1_uint128 *r, uint64_t a, uint64_t b) {'.
15(WARNING) Ignoring suspicious hits in line '   *r += (uint128_t)a * b;'.
16(WARNING) Ignoring suspicious hits in line '}'.
17(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_rshift(secp256k1_uint128 *r, unsigned int n) {'.
18(WARNING) Ignoring suspicious hits in line '   *r >>= n;'.
19(WARNING) Ignoring suspicious hits in line '}'.
20(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE uint64_t secp256k1_u128_to_u64(const secp256k1_uint128 *a) {'.
21(WARNING) Ignoring suspicious hits in line '   return (uint64_t)(*a);'.
22(INFO) Writing coverage report...
23------------------------------------------------------------------------------
24                           GCC Code Coverage Report
25<snip>

real-or-random commented at 7:51 am on August 12, 2025: contributor

lgtm but I suggest adding a note like this:

On gcovr >=8.3, --gcov-ignore-parse-errors=all can be replaced with --gcov-suspicious-hits-threshold=140737488355330.

Then we have a built-in reminder that we can remove the old argument in the future.

doc: Add `--gcov-ignore-parse-errors=all` option to `gcovr` invocations

Otherwise, commands might fail due to bugs in the `gcov` tool.

0458def51e

hebasto force-pushed on Aug 12, 2025

hebasto commented at 8:12 am on August 12, 2025: member

lgtm but I suggest adding a note like this:

On gcovr >=8.3, --gcov-ignore-parse-errors=all can be replaced with --gcov-suspicious-hits-threshold=140737488355330.

Then we have a built-in reminder that we can remove the old argument in the future.

Thanks! The note has been added.

real-or-random approved

real-or-random commented at 11:00 am on August 12, 2025: contributor

utACK 0458def51e18b7500dac8aec019b637dfade0667

real-or-random commented at 11:09 am on August 12, 2025: contributor

@josibake Want to review this (with whatever tool versions you have on your system)?

josibake commented at 8:42 am on August 13, 2025: member

Nice! Thanks for adding this, I ran into this recently and added --gcov-ignore-parse-errors=all after some googling. When I run with --gcov-ignore-parse-errors=all with gcovr@8.3, I see the following warnings:

 0gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
 1(INFO) Reading coverage data...
 2(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_accum_mul(secp256k1_uint128 *r, uint64_t a, uint64_t b) {'.
 3(WARNING) Ignoring suspicious hits in line '   *r += (uint128_t)a * b;'.
 4(WARNING) Ignoring suspicious hits in line '}'.
 5(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE void secp256k1_u128_rshift(secp256k1_uint128 *r, unsigned int n) {'.
 6(WARNING) Ignoring suspicious hits in line '   *r >>= n;'.
 7(WARNING) Ignoring suspicious hits in line '}'.
 8(WARNING) Ignoring suspicious hits in line 'static SECP256K1_INLINE uint64_t secp256k1_u128_to_u64(const secp256k1_uint128 *a) {'.
 9(WARNING) Ignoring suspicious hits in line '   return (uint64_t)(*a);'.
10(INFO) Writing coverage report...
11------------------------------------------------------------------------------
12                           GCC Code Coverage Report
13Directory: .
14------------------------------------------------------------------------------
15<snip>

The warning goes away when running with --gcov-suspicious-hits-threshold=140737488355330, i.e.:

0gcovr --gcov-suspicious-hits-threshold=140737488355330 --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary
1(INFO) Reading coverage data...
2(INFO) Writing coverage report...
3------------------------------------------------------------------------------
4                           GCC Code Coverage Report
5Directory: .
6------------------------------------------------------------------------------
7<snip>

Also confirmed I got the same coverage report with both flags.

josibake approved

josibake commented at 8:44 am on August 13, 2025: member

ACK https://github.com/bitcoin-core/secp256k1/pull/1722/commits/0458def51e18b7500dac8aec019b637dfade0667

Tested with gcovr@8.3 and confirmed I get a warning with the old flag (--gcovr-ignore-parse-errors=all), and that those warnings go away when running with the new flag suggest for gcovr >= 8.3

real-or-random merged this on Aug 13, 2025

real-or-random closed this on Aug 13, 2025

hebasto deleted the branch on Aug 13, 2025

docs: Exclude modules’ bench_impl.h headers from coverage report #1722

docs: Exclude modules’ `bench_impl.h` headers from coverage report #1722