Issue compiling on OpenBSD 6.7

[fanquake]

Since the secp256k1 subtree update in bitcoin/bitcoin#19228, it hasn't been possible to cleanly compile Bitcoin Core on OpenBSD. Building fails with (excerpt from just building libsecp, cc is Clang 8.0.1):

./autogen.sh
./configure CC=cc MAKE=gmake
gmake -j6 V=1
...
gmake
gcc -I. -I./src -Wall -Wextra -Wno-unused-function -g -c src/gen_context.c -o gen_context.o
In file included from src/gen_context.c:16:
src/util.h:179: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'uint128_t'
gmake: *** [Makefile:1689: gen_context.o] Error 1

libsecp does some native compiler detection in configure, and then uses that compiler specifically for the ecmult precomputation. i.e:

secp256k1/configure.ac

Lines 186 to 188 in 3f4a5a1

	if test x"$use_ecmult_static_precomputation" != x"no"; then
	# Temporarily switch to an environment for the native compiler
	save_cross_compiling=$cross_compiling

and then:

secp256k1/Makefile.am

Lines 129 to 130 in 3f4a5a1

	gen_%.o: src/gen_%.c src/libsecp256k1-config.h
	$(CC_FOR_BUILD) $(CPPFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@

The problem is that this ends up picking up and using OpenBSDs GCC 4.2.1, which barfs when it gets to util.h:

secp256k1/src/util.h

Line 179 in 3f4a5a1

SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;

I realise there might not be much that can be done here, given that it's the AX_PROG_CC_FOR_BUILD macro that is returning gcc to use. However thought I'd open an issue here for any thoughts, before we update our docs. I've suggested that affected users can just pass CC_FOR_BUILD=cc to configure when building. Issue here: bitcoin/bitcoin#19559.

[real-or-random]

I realise there might not be much that can be done here, given that it's the AX_PROG_CC_FOR_BUILD macro that is returning gcc to use.

Okay, I believe that's really just a bug. The problem is that all of this is a little bit hacky (because autoconf is a huge hack for the C mess and is hacky by itself) and annoying. I just don't see why this issue didn't appear in earlier revisions.

So we call AC_CHECK_TYPES([__int128]) [1], which defines HAVE___INT128 if the type exists. This check is performed against the selected CC but the result is not only used for CC but also for CC_FOR_BUILD, which should be entirely independent.

The definition is imported here then:
https://github.com/bitcoin-core/secp256k1/blame/3f4a5a10e43bfc8dae5b978cb39aa2dfbaf4d713/src/gen_context.c#L10
I believe the goal of including the basic config below that line is to make sure we compile gen_context with the most simple config that should run everywhere. (And performance does not matter anyway, it returns immediately and its running time is just compilation time in the end.)

So I believe the simple way to fix this is to add #undef HAS___INT128 to the list of undefs in src/basic-config.h. Can you check if this solves the issue?

[1] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Generic-Types.html#Generic-Types

[fanquake]

Can you check if this solves the issue?

Sure. I assume you meant #undef HAVE___INT128. I tested with 3f4a5a1 + this diff:

diff --git a/src/basic-config.h b/src/basic-config.h
index e9be39d..3ad77c2 100644
--- a/src/basic-config.h
+++ b/src/basic-config.h
@@ -9,6 +9,7 @@
 
 #ifdef USE_BASIC_CONFIG
 
+#undef HAVE___INT128
 #undef USE_ASM_X86_64
 #undef USE_ECMULT_STATIC_PRECOMPUTATION
 #undef USE_ENDOMORPHISM

./autogen.sh
./configure CC=cc MAKE=gmake
....
Build Options:
  with endomorphism       = no
  with ecmult precomp     = yes
  with external callbacks = no
  with benchmarks         = yes
  with coverage           = no
  module ecdh             = no
  module recovery         = no

  asm                     = x86_64
  bignum                  = no
  field                   = 64bit
  scalar                  = 64bit
  ecmult window size      = 15
  ecmult gen prec. bits   = 4

  valgrind                = no
  CC                      = cc
  CFLAGS                  = -O2 -fvisibility=hidden -std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings -W -g
  CPPFLAGS                = 
  LDFLAGS                 = 
...
gmake check -j6
...
PASS: exhaustive_tests
PASS: tests

Everything seems to be working ok.

[real-or-random]

Ok, thanks, this PR should fix it then. We could also let the autoconf script to do the following

• If CC_FOR_BUILD is set externally, use this
• Otherwise try to use CC as CC_FOR_BUILD
• Only if this fails, use the autodetected

It's somewhat surprising that GCC is used when you set CC to clang. We could also just note this in the "Build steps" section in the README.

[fanquake]

Ok, thanks, this PR should fix it then.

Great. Thanks for the quick followup and fix.