ci: Drop valgrind fuzz from GHA matrix

maflcko commented at 4:12 pm on February 3, 2026: member

The valgrind fuzz task is problematic, because:

It is redundant with the msan fuzz task, which has std lib hardening enabled, so often UB is diagnosed before it even happens in the valgrind task.
All issues so far found by the valgrind fuzz task were also found by the hardened msan fuzz task.
All other issues were false-positives, which are hard to debug, and confusing and tedious to work around.

I don’t think there is any value in asking pull request authors to debug valgrind false-positives that they triggered by accident. So remove the task for now.

I know that there are some devs, who like to keep the task, but if the task is kept, it should come with clear instructions on how to deal with false-postives in pull requests.

I am not proposing to remove the config itself, and I am happy to continue maintaining it, like it was done before. However, as of now, running it in the GHA matrix is of negative or questionable benefit.

ci: Drop valgrind fuzz from GHA matrix faa4ab113c

DrahtBot added the label Tests on Feb 3, 2026

DrahtBot commented at 4:12 pm on February 3, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, fanquake

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

fanquake commented at 4:14 pm on February 3, 2026: member

It is redundant with the msan fuzz task,

I don’t think this is 100% true. Its existence still currently serves as somewhat of a sanity check, if MSAN was to silently stop working, or an upstream Clang became broken.

Can you point to some of the recent false-positives?

Also came up recently in #34304.

maflcko commented at 4:24 pm on February 3, 2026: member

I don’t think this is 100% true. Its existence still currently serves as somewhat of a sanity check, if MSAN was to silently stop working, or an upstream Clang became broken.

Right. We should always keep double and maybe triple backups for all QA efforts, but I think the existing effort to keep the task mainained is sufficient for that imo.

Can you point to some of the recent false-positives?

Well, the clang ones have never been fixed up to today (https://github.com/bitcoin/bitcoin/issues/29635). And the GCC ones aren’t trivial to work around (https://github.com/bitcoin/bitcoin/pull/31282). The most recent one is #34299 (comment), which I am happy to look into, but it seems a bit odd to block the pull request based on an false-positive that just happened due to some specific ordering of compiler optimization passes on that specific code.

Another case is 326c23ab46e24fcf8114439d6e19f23ef1106968 / #25665 (comment)

Also came up recently in #34304.

Right, and I don’t really want to put oil in any fire. I just genuinely don’t see the benefit in the larger context here.

l0rinc commented at 4:53 pm on February 3, 2026: contributor

Isn’t this the same as #34304? I’m still in favor of it, but I think it needs some explanation.

maflcko commented at 5:34 pm on February 3, 2026: member

Isn’t this the same as #34304? I’m still in favor of it, but I think it needs some explanation.

Have you seen the pull request description and read all the previous comments? I am happy to answer questions, but the ones you raised are already answered.

it seems a bit odd to block the pull request based on an false-positive that just happened due to some specific ordering of compiler optimization passes on that specific code.

To clarify, one workaround would be -DAPPEND_CXXFLAGS='-O1 -g2' in the latest case. I don’t know which other workarounds exist, but it just seems odd to ask a random pull request author to imagine several odd workarounds and then randomly pick the least odd one.

It seems worth to do it, if there ever was an actual bug that it found. However, absent that, it doesn’t seem productive to load off the false-positives onto random pull requests.

Again, of course I’ll continue to maintain the CI config and I welcome others to do so as well.

l0rinc commented at 6:20 pm on February 3, 2026: contributor

ACK faa4ab113cc9e300b3b8dce0c774d0a33a555883

Have you seen the pull request description and read all the previous comments?

You can always assume that safely.

maflcko commented at 10:47 am on February 4, 2026: member

Just for reference, I don’t intent that this is a permanent decision. It is trivial to revert the commit and re-evaluate this in the future. For example, one could say that every false-positive is plainly added to a suppressions file. However, this requires more work:

Document the expectation.
Add suppressions logic to the fuzz CI (currently there is none at all), so that the current file is read and new suppressions are generated.

Personally I don’t like putting compiler-version-specific or stdlib-version-specific suppressions into a file that is also used by devs using other compilers or other stdlib versions, but I won’t mind if others want it to be this way.

My only point is that the current state with the questionable benefit and all the false-positives, without any concept of how to approach them is not useful, so I think temporarily removing it and properly re-evaluating it makes the most sense for now.

willcl-ark commented at 2:17 pm on February 4, 2026: member

In case we do opt to drop this job in this repo I have enabled a CI job running valgrind fuzz on master over on my cdash instance.

As with the guix-* jobs, the “notes” section contains (some relevant) log outputs. I haven’t seen a valgrind job fail yet, but in theory it should report errors to the dashboard via:

https://github.com/willcl-ark/nix-guix-ci/blob/050bf9e16b5d5329deb173d015c608d1cbb03480/scripts/valgrind-fuzz.cmake#L65-L85

maflcko commented at 7:51 am on February 5, 2026: member

In case we do opt to drop this job in this repo I have enabled a CI job running valgrind fuzz on master over on my cdash instance

Thx. I guess that can’t hurt. Though, I think it is a bit unrelated to this pull request. I’d say the CI config itself should be kept and I’ll continue to maintain it. However, the config is great at finding upstream valgrind bugs, but less great at finding Bitcoin Core bugs, so the only goal here would be to (temporarily?) remove it from the GHA matrix, so that the red CI is unblocked, and the false-positive can be properly handled.

willcl-ark commented at 7:55 am on February 5, 2026: member

In case we do opt to drop this job in this repo I have enabled a CI job running valgrind fuzz on master over on my cdash instance

Thx. I guess that can’t hurt. Though, I think it is a bit unrelated to this pull request. I’d say the CI config itself should be kept and I’ll continue to maintain it. However, the config is great at finding upstream valgrind bugs, but less great at finding Bitcoin Core bugs, so the only goal here would be to (temporarily?) remove it from the GHA matrix, so that the red CI is unblocked, and the false-positive can be properly handled.

yes, i agree :)

maflcko commented at 6:16 am on February 6, 2026: member

The underlying bug here hasn’t been fixed for 3 years now and hasn’t seen any update in almost 2 years: https://bugs.kde.org/show_bug.cgi?id=472329

fanquake commented at 10:27 am on February 6, 2026: member

ACK faa4ab113cc9e300b3b8dce0c774d0a33a555883 - hopefully we can revisit re-adding soon. To be clear, I don’t agree with the rationale from #34304, or the initial changes there. The case here, and the fact that it is causing disruption in this repo, is more pressing.

fanquake merged this on Feb 6, 2026

fanquake closed this on Feb 6, 2026

maflcko deleted the branch on Feb 6, 2026

ci: Drop valgrind fuzz from GHA matrix #34492

Reviews