Use preprocessor macros instead of autoconf to detect endianness

real-or-random commented at 2:27 pm on August 7, 2020: contributor

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 .

real-or-random force-pushed on Aug 7, 2020

real-or-random cross-referenced this on Aug 7, 2020 from issue Get rid of autoconf check for endianness by real-or-random

gmaxwell commented at 5:46 pm on August 7, 2020: contributor

I like this approach.

gmaxwell commented at 1:36 am on August 8, 2020: contributor

Travis has a zero log apparently unexplained failure on s390 … the only test that really mattered for this pr. :)

I tested it by hand on s390 and it works. sooo ?!

sipa commented at 2:40 am on August 8, 2020: contributor

I checked this PR a few hours ago, and all tests had finished, except the s390x one, which hadn’t started.

So I think there may be some provisioning issue that prevents travis from assigning an s390x worker, and it somehow timed out?

JeremyRubin commented at 4:43 am on August 8, 2020: none

This is also the case on Bitcoin Core right now too :/

real-or-random commented at 6:25 pm on August 8, 2020: contributor

Yeah, first I felt brave when I squashed #696 and then merged it immediately. And then I got a notification from Travis that I broke master… :/ But yes, it’s an issue on their side…

sipa cross-referenced this on Aug 10, 2020 from issue [do not merge] Test impact of secp256k1 endianness detection change by sipa

sipa commented at 10:10 pm on August 10, 2020: contributor

Opened a PR for Bitcoin Core https://github.com/bitcoin/bitcoin/pull/19695 to test if this may impact any of the systems tested by its CI (which includes an MSVC based one).

sipa cross-referenced this on Aug 10, 2020 from issue Report build configuration in test/bench output by sipa

gmaxwell commented at 1:46 am on August 11, 2020: contributor

It’s _MSC_VER not _MSVC_VER (thanks to sipa abusing bitcoin’s CI setup to test this.)

Use preprocessor macros instead of autoconf to detect endianness

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.

0dccf98a21

real-or-random force-pushed on Aug 11, 2020

real-or-random commented at 9:26 am on August 11, 2020: contributor

Fixed. @sipa can you retest on Core?

sipa commented at 6:49 pm on August 11, 2020: contributor

@real-or-random Updated it.

real-or-random commented at 10:17 am on August 12, 2020: contributor

@real-or-random Updated it.

It works. :tada:

in src/util.h:193 in 0dccf98a21

188+#endif
189+#if defined(_MSC_VER) && defined(_WIN32) && !defined(SECP256K1_LITTLE_ENDIAN)
190+# define SECP256K1_LITTLE_ENDIAN
191+#endif
192+#if defined(SECP256K1_LITTLE_ENDIAN) == defined(SECP256K1_BIG_ENDIAN)
193+# error Please make sure that either SECP256K1_LITTLE_ENDIAN or SECP256K1_BIG_ENDIAN is set, see src/util.h.

sipa commented at 9:47 pm on August 12, 2020:

I never knew that you could put an error message after #error without quotes around it.

In retrospect, not sure what I’d expect it to do. Give an error?

sipa commented at 9:48 pm on August 12, 2020: contributor

ACK 0dccf98a21beb245f6cd9ed76fb7368529df09c7

gmaxwell commented at 10:33 pm on August 12, 2020: contributor

ACK 0dccf98a21beb245f6cd9ed76fb7368529df09c7

real-or-random merged this on Aug 13, 2020

real-or-random closed this on Aug 13, 2020

gmaxwell commented at 9:36 pm on August 13, 2020: contributor

This broke the build on GCC 2.95 on Debian i386 for me and on Haiku (BeOS).

sipa commented at 11:10 pm on August 13, 2020: contributor

@gmaxwell Can you give the list of preprocessor defines on those?

gmaxwell commented at 11:50 pm on August 13, 2020: contributor

 0    $ gcc -dM -E -x c -
 1    #define __linux__ 1 
 2    #define linux 1 
 3    #define __i386__ 1 
 4    #define __i386 1 
 5    #define __i686 1 
 6    #define __GNUC_MINOR__ 95 
 7    #define pentiumpro 1 
 8    #define __pentiumpro 1 
 9    #define i386 1 
10    #define i686 1 
11    #define __pentiumpro__ 1 
12    #define __unix 1 
13    #define __unix__ 1 
14    #define __i686__ 1 
15    #define __GNUC__ 2 
16    #define __linux 1 
17    #define __ELF__ 1 
18    #define unix 1

From the haiku I can’t easily copy and paste, but it includes,

0__i586 __i386 _X86_ __i386__ __i586__ __pentium __pentium__

among other less relevant ones.

sipa cross-referenced this on Aug 14, 2020 from issue Add fallback LE/BE for architectures with known endianness + SHA256 selftest by sipa

sipa commented at 6:52 pm on August 14, 2020: contributor

@gmaxwell See #799

real-or-random referenced this in commit 54caf2e74f on Sep 9, 2020

jasonbcox referenced this in commit 59ff264215 on Sep 27, 2020

deadalnix referenced this in commit 1bf8cda399 on Sep 28, 2020

apoelstra cross-referenced this on Oct 9, 2020 from issue Upstream PRs #696 #795 #793 #787 #798 #805 #648 #806 #799 #699 #797 by jonasnick

elichai cross-referenced this on Dec 29, 2020 from issue update libsecp, remove endomorphism feature flag and release 0.20.0 by apoelstra

real-or-random cross-referenced this on Mar 9, 2021 from issue Reduce cryptography usage in --cfg=fuzzing by TheBlueMatt

Use preprocessor macros instead of autoconf to detect endianness #787