In the existing code, the compiler is allowed to allocate the RSI register
for outputs m0, m1, or m2, which are written to before the input in RSI is
read from. Fix this by marking them as early clobber.
Reported by ehoffman2 in https://github.com/bitcoin-core/secp256k1/issues/766
- secp256k1_scalar_cadd_bit
- secp256k1_modinvXX_normalize_YY
- secp256k1_modinvXX_divsteps_ZZ
- ECMULT_CONST_TABLE_GET_GE
Even though those code loations are not problematic right now
(with current compilers).
8e142ca4102ade1b90dcb06d6c78405ef3220599 Move `SECP256K1_INLINE` macro definition out from `include/secp256k1.h` (Hennadii Stepanov)
77445898a5852ecd38ab95cfb329333a82673115 Remove `SECP256K1_INLINE` usage from examples (Hennadii Stepanov)
Pull request description:
From [IRC](https://gnusha.org/secp256k1/2023-01-31.log):
> 06:29 \< hebasto\> What are reasons to define the `SECP256K1_INLINE` macro in user's `include/secp256k1.h` header, while it is used internally only?
> 06:32 \< hebasto\> I mean, any other (or a new dedicated) header in `src` looks more appropriate, no?
> 06:35 \< sipa\> I think it may just predate any "utility" internal headers.
> 06:42 \< sipa\> I think it makes sense to move it to util.h
Pros:
- it is a step in direction to better organized headers (in context of #924, #1039)
Cons:
- code duplication for `SECP256K1_GNUC_PREREQ` macro
ACKs for top commit:
sipa:
utACK 8e142ca4102ade1b90dcb06d6c78405ef3220599
real-or-random:
utACK 8e142ca410
Tree-SHA512: 180e0ba7c2ef242b765f20698b67d06c492b7b70866c21db27c18d8b2e85c3e11f86c6cb99ffa88bbd23891ce3ee8a24bc528f2c91167ec2fddc167463f78eac
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
After this commit, int128.h and int128_impl.h are included as follows:
- .c files which use int128 include int128_impl.h (after util.h)
- .h files which use int128 include int128.h (after util.h)
This list is exhaustive. util.h needs to included first because it sets
up necessary #defines.
There were several places where the code was non-constant time
for invalid secret inputs. These are harmless under sane use
but get in the way of automatic const-time validation.
(Nonce overflow in signing is not addressed, nor is s==0 in
signing)
The constraint "n" is a more obscure setting which is needed for
special cases that don't apply to our use.
Use of "n" exposed some bugs in pre-release versions of clang.
This fixes#594
Identifiers starting with an underscore and followed immediately by a capital letter are reserved by the C++ standard.
The only header guards not fixed are those in the headers auto-generated from java.
This has the effect of making `secp256k1_scalar_mul_shift_var` constant
time in both input scalars. Keep the _var name because it is NOT constant
time in the shift amount.
As used in `secp256k1_scalar_split_lambda_var`, the shift is always
the constant 272, so this function becomes constant time, and it
loses the `_var` suffix.
Designed with clear separation of the wNAF conversion, precomputation
and exponentiation (since the precomp at least we will probably want
to separate in the API for users who reuse points a lot.
Future work:
- actually separate precomp in the API
- do multiexp rather than single exponentiation
