This compiler flag is available for clang but not gcc.
Test plan:
```
autogen.sh
./configure
make check
CC=clang ./configure
make check
```
If a variable is used uninitialized, the warning should look something
like:
```
CC src/tests-tests.o
src/tests.c:4336:15: warning: variable 'recid' may be uninitialized when used here [-Wconditional-uninitialized]
CHECK(recid >= 0 && recid < 4);
^~~~~
./src/util.h:54:18: note: expanded from macro 'CHECK'
if (EXPECT(!(cond), 0)) { \
^~~~
./src/util.h:41:39: note: expanded from macro 'EXPECT'
^
src/tests.c:4327:14: note: initialize the variable 'recid' to silence this warning
int recid;
^
= 0
1 warning generated.
```
This commits simply uses CC as CC_FOR_BUILD and the same for
corresponding flags if we're not cross-compiling. This has a number of
benefits in this common case:
- It avoids strange cases where very old compilers are used (#768).
- Flags are consistently set for CC and CC_FOR_BUILD.
- ./configure is faster.
- You get compiler x consistently if you set CC=x; we got this wrong
in CI in the past.
./configure warns if a _FOR_BUILD variable is set but ignored because
we're not cross-compiling.
The change exposed that //-style comments are used in gen_context.c,
which is also fixed by this commit.
This commit also reorganizes code in configure.ac to have a cleaner
separation of sections.
Valgrind is typically installed using brew on macOS. This commit
makes ./configure detect this case set the appropriate include
directory (in the same way as we already do for openssl and gmp).
412bf874d0 configure: Allow specifying --with[out]-valgrind explicitly (Luke Dashjr)
Pull request description:
ACKs for top commit:
sipa:
ACK 412bf874d0. Tested by running configure on a system with and without valgrind, and with no argument, with `--with-valgrind`, and with `--without-valgrind`.
real-or-random:
ACK 412bf874d0
jonasnick:
ACK 412bf874d0
Tree-SHA512: 92417609751e5af813faff1661055cd37f3d00dbcf109a8f14f8ba59d9f3d620c9c6b67d2b1629b6ab75e2afcd47d2b3898a0427931567fb505bc92fa5ee3532
0dccf98a21 Use preprocessor macros instead of autoconf to detect endianness (Tim Ruffing)
Pull request description:
This does not fix any particular issue but it's preferable to not
rely on autoconf. This avoids endianness mess for users on BE hosts
if they use their build without autoconf.
The macros are carefully written to err on the side of the caution,
e.g., we #error if the user manually configures a different endianness
than what we detect.
Supersedes #770 .
ACKs for top commit:
sipa:
ACK 0dccf98a21
gmaxwell:
ACK 0dccf98a21
Tree-SHA512: 6779458de5cb6eaef2ac37f9d4b8fa6c9b299f58f6e5b72f2b0d7e36c12ea06074e483acfb85085a147e0f4b51cd67d897f61a67250ec1cea284a0f7680eb2e8
This does not fix any particular issue but it's preferable to not
rely on autoconf. This avoids endianness mess for users on BE hosts
if they use their build without autoconf.
The macros are carefully written to err on the side of the caution,
e.g., we #error if the user manually configures a different endianness
than what we detect.
Instead of supporting configuration of the field and scalar size independently,
both are now controlled by the availability of a 64x64->128 bit multiplication
(currently only through __int128). This is autodetected from the C code through
__SIZEOF_INT128__, but can be overridden using configure's
--with-test-override-wide-multiply, or by defining
USE_FORCE_WIDEMUL_{INT64,INT128} manually.
ca739cba23 Compile with optimization flag -O2 by default instead of -O3 (Jonas Nick)
83fb1bcef4 Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual) (Jonas Nick)
ecba8138ec Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables (Jonas Nick)
613c34cd86 Remove test in configure.ac because it doesn't have an effect (Jonas Nick)
Pull request description:
Right now, it's not easy to reduce the optimization level with `CFLAGS` because `configure` overwrites any optimization flag with `-O3`. The [automake documentation](https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html) states that:
> The reason ‘$(CPPFLAGS)’ appears after ‘$(AM_CPPFLAGS)’ or ‘$(mumble_CPPFLAGS)’ in the compile command is that users should always have the last say.
and also that it's incorrect to redefine CFLAGS in the first place
> You should never redefine a user variable such as CPPFLAGS in Makefile.am. [...] You should not add options to these user variables within configure either, for the same reason
With this PR `CFLAGS` is still redefined, but user-provided flags appear after the default `CFLAGS` which means that they override the default flags (at least in clang and gcc). Otherwise, the default configuration is not changed. This also means that if CFLAGS are defined by the user, then -g is not added (which does not seem to make much sense). In order to keep the `-O3` despite the reordering we need to explicitly tell autoconf to not append `-O2` by setting the default to `-g` with `: ${CFLAGS="-g"}` as per [the manual](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#C-Compiler) (EDIT: link fix).
ACKs for top commit:
real-or-random:
ACK ca739cba23
theuni:
ACK ca739cba23.
elichai:
ACK ca739cba23
Tree-SHA512: be92589faa461d245203385d44b489c7d6917b0c68472b8d7576806c0250cf5ff61d5c99ce04eebb8ff5279b9987185d4e5d2da979683fb1c489fdf3e5b59630
Valgrind does bit-level tracking of the "uninitialized" status of memory,
property tracks memory which is tainted by any uninitialized memory, and
warns if any branch or array access depends on an uninitialized bit.
That is exactly the verification we need on secret data to test for
constant-time behaviour. All we need to do is tell valgrind our
secret key is actually uninitialized memory.
This adds a valgrind_ctime_test which is compiled if valgrind is installed:
Run it with libtool --mode=execute:
$ libtool --mode=execute valgrind ./valgrind_ctime_test
ECDSA signing has a retry loop for the exceptionally unlikely case
that S==0. S is not a secret at this point and this case is so
rare that it will never be observed but branching on it will trip
up tools analysing if the code is constant time with respect to
secrets.
Derandomized ECDSA can also loop on k being zero or overflowing,
and while k is a secret these cases are too rare (1:2^255) to
ever observe and are also of no concern.
This adds a function for marking memory as no-longer-secret and
sets it up for use with the valgrind memcheck constant-time
test.
make ECMULT_GEN_PREC_BITS configurable
ecmult_static_context.h: add compile time config assertion (#3) - Prevents accidentally using a file which was generated with a
different configuration.
README: mention valgrind issue
With --with-ecmult-gen-precision=8, valgrind needs a max stack size
adjustment to not run into a stack switching heuristic:
http://valgrind.org/docs/manual/manual-core.html
> -max-stackframe= [default: 2000000]
> The maximum size of a stack frame. If the stack pointer moves by more than this amount then Valgrind will assume that the program is switching to a different stack.
You may need to use this option if your program has large stack-allocated arrays.
basic-config: undef ECMULT_WINDOW_SIZE before (re-)defining it
This makes WINDOW_G a configurable value in the range of [2..24].
The upper limit of 24 is a defensive choice. The code is probably
correct for values up to 27 but those larger values yield in huge
tables (>= 256MiB), which are i) unlikely to be really beneficial
in practice and ii) increasingly difficult to test.
a34bcaa Actually pass CFLAGS_FOR_BUILD and LDFLAGS_FOR_BUILD to linker (Tim Ruffing)
2d5f4ce configure: Use CFLAGS_FOR_BUILD when checking native compiler (Tim Ruffing)
Pull request description:
This fixes a bug where configure would fail or disable static
ecmult tables because it wrongly checks the native compiler using
the target CFLAGS (instead of the native CFLAGS_FOR_BUILD).
Moreover, this commit adds tests to figure out whether the native
compiler supports the warning flags passed during the build, and it
contains a few minor improvements to the code that checks the native
compiler.
Tree-SHA512: 31a92a5516cf2f9801c918edfba0059aa4f8549b0c1de94fc166b5e92ad1868a480c48cdc5ff62679ba20e26f4a0e2948c71fd2b3e80766673d2bf7512da3875
3965027 Summarize build options in configure script (Evan Klitzke)
Pull request description:
This is a trivial build system change to summarize the build options after running configure.
Example output:
```
$ ./configure
....
<many lines omitted>
...
config.status: src/libsecp256k1-config.h is unchanged
config.status: executing depfiles commands
config.status: executing libtool commands
Build Options:
with endomorphism = no
with ecmult precomp = yes
with jni = no
module ecdh = no
module recovery = no
asm = x86_64
bignum = gmp
field = 64bit
scalar = 64bit
CC = gcc
CFLAGS = -g -O2 -W -std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings -fvisibility=hidden -O3
CPPFLAGS =
LDFLAGS =
```
I tried to just include the configure options that looked interesting; let me know if there are any I didn't include that I should have.
Tree-SHA512: 428381654d772f76bc81210d39ba5c3f07a94dc6a6378a02ccc6f23ebce7f501896268bcd2e94e2b0d8aea54c9c70c44a9238a0f0960600f463b1e2847c7ed1f
270f6c8 Portability fix for the configure scripts generated (Pierre Pronchery)
Pull request description:
Found thanks to the developer checks from the pkgsrc software
distribution (for NetBSD, SmartOS, Minix, MacOS X, Linux, and more).
Tree-SHA512: 2589545aa4d0620db66e79df1dc148a487384b5169ba7323937490d802973388859d30d45b35ee3e614be6d49cb694f37f585a16caa87ad1e500a0b7368dcc0a
This fixes a bug where configure would fail or disable static
ecmult tables because it wrongly checks the native compiler using
the target CFLAGS (instead of the native CFLAGS_FOR_BUILD), and
similar for CPPFLAGS and LDFLAGS.
Moreover, this commit adds tests to figure out whether the native
compiler supports the warning flags passed during the build, and it
contains a few minor improvements to the code that checks the native
compiler.
57752d2 [build] Set --enable-jni to no by default instead of auto. (Karl-Johan Alm)
Pull request description:
Having `--enable-jni` be `auto` doesn't make a lot of sense, and results in things like https://github.com/bitcoin/bitcoin/pull/11056.
Tree-SHA512: 27d6ea041f5d6e249857869ab87b8f7b1f6d18ec5ec82d2c46e692cd690b9f5c5857886725901a29d3539d427d8b6154d0c7909cfa2ce30bb3d4460c05708386
We observe that when changing the b-value in the elliptic curve formula
`y^2 = x^3 + ax + b`, the group law is unchanged. Therefore our functions
for secp256k1 will be correct if and only if they are correct when applied
to the curve defined by `y^2 = x^3 + 4` defined over the same field. This
curve has a point P of order 199.
This commit adds a test which computes the subgroup generated by P and
exhaustively checks that addition of every pair of points gives the correct
result.
Unfortunately we cannot test const-time scalar multiplication by the same
mechanism. The reason is that these ecmult functions both compute a wNAF
representation of the scalar, and this representation is tied to the order
of the group.
Testing with the incomplete version of gej_add_ge (found in 5de4c5dff^)
shows that this detects the incompleteness when adding P - 106P, which
is exactly what we expected since 106 is a cube root of 1 mod 199.
Jonas Nick
Pieter Wuille
PiRK
Tim Ruffing
Elichai Turkel
Luke Dashjr
fanquake
Gregory Maxwell
djb
DesWurstes
Evan Klitzke
Pierre Pronchery
Pieter Wuille
Karl-Johan Alm
Andrew Poelstra
Cory Fields