fuzz: psbt_base64_decode fails on Windows #32135

1. 
   maflcko commented at 9:41 am on March 25, 2025: member

   https://github.com/bitcoin/bitcoin/actions/runs/14046520411/job/39356515207

   0
   1Assertion failed: DecodeBase64PSBT(psbt, random_string, error) == error.empty(), file D:\a\bitcoin\bitcoin\src\test\fuzz\base_encode_decode.cpp, line 95
   2Error processing input "D:\\a\\_temp\\qa-assets\\fuzz_corpora\\psbt_base64_decode\\0041ae3fba09a33e18a330c95acce904552b7114"
   

2. maflcko added this to the milestone 29.0 on Mar 25, 2025
3. maflcko added the label Tests on Mar 25, 2025
4. maflcko added the label CI failed on Mar 25, 2025
5. maflcko added the label Windows on Mar 25, 2025
6. maflcko referenced this in commit c62dc520e0 on Mar 25, 2025
7. 
   maflcko commented at 10:56 am on March 25, 2025: member

   I pushed https://github.com/bitcoin-core/qa-assets/commit/c62dc520e0c7caee3e50c9ba8a88056c7f4d357f to temporarily unbreak the CI.

   It would be good if someone on Windows helped here to track this down.

8. 
   Sjors commented at 1:02 pm on March 25, 2025: member

   cc @hebasto
9. 
   hodlinator commented at 3:48 pm on March 25, 2025: contributor

   Been debugging this, and I suspect the issue is down to unspecified/differing evaluation order. I suspect MSVC is sometimes evaluating the RHS of the ==-expression before the LHS.

   0assert(DecodeBase64PSBT(psbt, random_string, error) == error.empty());
   

   Breaking out fixes the issue locally for me, probably by introducing a sequence-point.

   0bool success = DecodeBase64PSBT(psbt, random_string, error);
   1assert(success == error.empty());
   

   Edit: Tried things like breaking out both sides into lambdas which log messages to prove evaluation order, but cannot reproduce the failure, so it seems to be specific conditions causing the evaluation order to reverse.

10. 
    maflcko commented at 4:37 pm on March 25, 2025: member

    @hodlinator Thanks! When the evaluation order isn’t specified by the standard, it is unspecified. Mind submitting a fix?
11. 
    l0rinc commented at 5:48 pm on March 25, 2025: contributor

    Thanks a lot for checking @hodlinator!

    According to https://timsong-cpp.github.io/cppwp/n4861/basic.exec#intro.execution-10:

    evaluations of operands of individual operators […] are unsequenced. […] If a side effect on a memory location […] is unsequenced relative to […] a value computation using the value of any object in the same memory location […], the behavior is undefined

    It seems that because the operands of == are unsequenced, side effects can lead to undefined behavior - my bad, it didn’t even occur to me that this was UB - and makes me wonder how many other cases we have, if none of the linters were complaining. @hodlinator I can push a PR with your email - I introduced the faulure, I should fix it.

12. 
    hodlinator commented at 7:57 pm on March 25, 2025: contributor

    Since I introduced the idea to write the code like this, I’m happy to be part of resolving it. And happy to review your PR.
13. l0rinc referenced this in commit b1de59e896 on Mar 25, 2025
14. 
    l0rinc commented at 8:40 pm on March 25, 2025: contributor

    I have opened #32141 - but still can’t fuzz properly on mac, so would appreciate if someone could run it for a few minutes (especially on Windows).
15. fanquake closed this on Mar 27, 2025

    ---

16. fanquake referenced this in commit b413b088ae on Mar 27, 2025

Contributors
maflcko Sjors hodlinator l0rinc

Labels
Windows Tests CI failed

Milestone
29.0