Debian stable is now bullseye. This results in an update of compiler versions, which make some minor fixes necessary. See commit messages for details.
Tip: If you want to build the docker image locally, use --pull to force docker to pull the latest Debian image as a base, e.g., docker build . -f ci/linux-debian.Dockerfile --pull.
5269 | random_scalar_order_test(&msg);
5270 | random_scalar_order_test(&key);
5271 | secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &pubj, &key);
5272 | secp256k1_ge_set_gej(&pub, &pubj);
5273 | getrec = secp256k1_testrand_bits(1);
5274 | - random_sign(&sigr, &sigs, &key, &msg, getrec?&recid:NULL);
fwiw, assigning the pointer to a variable also seems to solve this, Isolated case: https://godbolt.org/z/q1T6eMccT
I see, yeah. I tried a variant of this, which didn't work. I think both "fixes" are fine, I'm going to keep mine if noone objects.
Interestingly, your test case shows that the warning appears also on clang 12. But I can't get valgrind to complain on clang 12 output...
clang 7 to 11 (and maybe earlier versions) warn about recid being
potentially unitiliazed in "CHECK(recid >= 0 [...]", which was mitigated
in commit 3d2cf6c5bd35b0d72716b47bdd7e3892388aafc4 by initializing recid
to make clang happy but VG_UNDEF'ing the variable after initializiation
in order to ensure valgrind's memcheck analysis will still be sound and
complain if recid is not actually written to when creating a signature.
However, it turns out that at least for binaries produced by clang 11
(but not clang 7), valgrind complains about a branch on unitialized data
in the recid variable in that line before *and* after the aforementioned
commit. While the complaint after the commit could be spurious (clang
knows that recid is initialized, so it's fine to access it even though
the access is stupid), the complaint before the commit indicates a real
problem: it might be the case that clang is performing a wrong
optimization that leads to a situation where recid is really not
guaranteed to be initialized when it's accessed. As a result, clang
warns about this and generates code that just accesses the variable.
I'm not going to bother with this further because this is fixed in
clang 12 and the problem is just in our test code, not in the tested
code.
This commit rewrites the code in a way that groups the signing together
with the CHECK such that it's very easy to figure out for clang that
recid will be initialized properly. This seems to circument the issue.
ACK 5d5c74a057f3951677691113747952f4cbdde86b
Can confirm that clang 7, 11 and 12 don't complain.
ACK 5d5c74a
It seems that removing the variable initialization and the valgrind macro even clang 12 warns, but this fixes it anyway.