ct: Use volatile “trick” in all fe/scalar cmov implementations #1257

1. 
   real-or-random commented at 7:04 am on April 1, 2023: contributor

   Apparently clang 15 is able to compile our cmov code into a branch, at least for fe_cmov and fe_storage_cmov. This commit makes the condition volatile in all cmov implementations (except ge but that one only calls into the fe impls).

   This is just a quick fix. We should still look into other methods, e.g., asm and #457. We should also consider not caring about constant-time in scalar_low_impl.h

   We should also consider testing on very new compilers in nightly CI, see #864 (comment)

2. ct: Use volatile "trick" in all fe/scalar cmov implementations

   Apparently clang 15 is able to compile our cmov code into a branch,
   at least for fe_cmov and fe_storage_cmov. This commit makes the
   condition volatile in all cmov implementations (except ge but that
   one only calls into the fe impls).
   
   This is just a quick fix. We should still look into other methods,
   e.g., asm and #457. We should also consider not caring about
   constant-time in scalar_low_impl.h
   
   We should also consider testing on very new compilers in nightly CI,
   see https://github.com/bitcoin-core/secp256k1/pull/864#issuecomment-769211867

   4a496a36fb
3. jonasnick approved
4. 
   jonasnick commented at 11:49 am on April 3, 2023: contributor

   ACK 4a496a36fb07d6cc8c99e591994f4ce0c3b1174c

   I can reproduce the issue in master with clang 15 and that this PR fixes it.

5. 
   sipa commented at 1:37 am on April 6, 2023: contributor

   Code review ACK
6. real-or-random merged this on Apr 6, 2023
7. real-or-random closed this on Apr 6, 2023

   ---

8. sipa referenced this in commit e1552d578e on Apr 11, 2023
9. fanquake cross-referenced this on Apr 12, 2023 from issue ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs by fanquake
10. sipa referenced this in commit c981671e9b on Apr 14, 2023
11. hebasto cross-referenced this on Apr 15, 2023 from issue release: Checking binary compatibility by real-or-random
12. real-or-random cross-referenced this on May 8, 2023 from issue Alternative cmov implementation by peterdettman
13. real-or-random cross-referenced this on May 10, 2023 from issue ct: Use more volatile by real-or-random
14. hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
15. RandyMcMillan referenced this in commit 3cc75121b3 on May 27, 2023
16. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
17. vmta referenced this in commit 8f03457eed on Jul 1, 2023
18. alokeutpal approved
19. jonasnick cross-referenced this on Jul 21, 2023 from issue Upstream PRs 1228, 1236, 1243, 1238, 1246, 1247, 1242, 1250, 1244, 1241, 1257, 1226, 1252, 1118, 1245, 1266, 1269 by jonasnick
20. jonasnick cross-referenced this on Jul 21, 2023 from issue Upstream PRs 1228, 1236, 1243, 1238, 1246, 1247, 1242, 1250, 1244, 1241, 1257, 1226, 1252, 1118, 1245, 1266, 1269 by jonasnick

Contributors
real-or-random jonasnick sipa alokeutpal