build: Detect if char equals int8

ken2812221 commented at 2:11 pm on July 1, 2018: contributor

Probably fixes #13576. I’m not able to test this. @stacepellegrino, can you test this?

fanquake added the label Build system on Jul 1, 2018

stacepellegrino commented at 2:48 pm on July 1, 2018: none

Just testing changes now. Performing build to see if it fixes https://github.com/bitcoin/bitcoin/issues/13576

stacepellegrino commented at 3:00 pm on July 1, 2018: none

Did not fix the problem…

 0# make
 1Making all in src
 2make[1]: Entering directory '/opt/local/src/bitcoin/src'
 3make[2]: Entering directory '/opt/local/src/bitcoin/src'
 4make[3]: Entering directory '/opt/local/src/bitcoin'
 5make[3]: Leaving directory '/opt/local/src/bitcoin'
 6  CXX      crypto/libbitcoinconsensus_la-aes.lo
 7  CXX      crypto/libbitcoinconsensus_la-chacha20.lo
 8  CXX      crypto/libbitcoinconsensus_la-hmac_sha256.lo
 9  CXX      crypto/libbitcoinconsensus_la-hmac_sha512.lo
10  CXX      crypto/libbitcoinconsensus_la-ripemd160.lo
11  CXX      crypto/libbitcoinconsensus_la-sha1.lo
12  CXX      crypto/libbitcoinconsensus_la-sha256.lo
13  CXX      crypto/libbitcoinconsensus_la-sha512.lo
14  CXX      crypto/libbitcoinconsensus_la-sha256_sse4.lo
15  CXX      libbitcoinconsensus_la-arith_uint256.lo
16  CXX      consensus/libbitcoinconsensus_la-merkle.lo
17In file included from ./script/script.h:11:0,
18                 from ./primitives/transaction.h:11,
19                 from ./consensus/merkle.h:11,
20                 from consensus/merkle.cpp:5:
21./serialize.h:195:39: error: redefinition of 'template<class Stream> void Serialize(Stream&, int8_t)'
22 template<typename Stream> inline void Serialize(Stream& s, int8_t a  ) { ser_wr
23                                       ^
24./serialize.h:193:39: note: 'template<class Stream> void Serialize(Stream&, char)' previously declared here
25 template<typename Stream> inline void Serialize(Stream& s, char a    ) { ser_wr
26                                       ^
27In file included from ./script/script.h:11:0,
28                 from ./primitives/transaction.h:11,
29                 from ./consensus/merkle.h:11,
30                 from consensus/merkle.cpp:5:
31./serialize.h:213:39: error: redefinition of 'template<class Stream> void Unserialize(Stream&, int8_t&)'
32 template<typename Stream> inline void Unserialize(Stream& s, int8_t& a  ) { a =
33                                       ^
34./serialize.h:211:39: note: 'template<class Stream> void Unserialize(Stream&, char&)' previously declared here
35 template<typename Stream> inline void Unserialize(Stream& s, char& a    ) { a =
36                                       ^
37Makefile:8138: recipe for target 'consensus/libbitcoinconsensus_la-merkle.lo' failed
38make[2]: *** [consensus/libbitcoinconsensus_la-merkle.lo] Error 1
39make[2]: Leaving directory '/opt/local/src/bitcoin/src'
40Makefile:9824: recipe for target 'all-recursive' failed
41make[1]: *** [all-recursive] Error 1
42make[1]: Leaving directory '/opt/local/src/bitcoin/src'
43Makefile:757: recipe for target 'all-recursive' failed
44make: *** [all-recursive] Error 1

stacepellegrino commented at 3:06 pm on July 1, 2018: none

The configure script generates the following as part of its output…

checking for if type char equals int8_t... no

ken2812221 commented at 3:09 pm on July 1, 2018: contributor

Can you provide your config.log?

stacepellegrino commented at 3:26 pm on July 1, 2018: none

Here it is…

config.log

Detect if char equals int8_t 49d1f4cdde

ken2812221 force-pushed on Jul 1, 2018

ken2812221 commented at 3:42 pm on July 1, 2018: contributor

I forgot to specify the error message for static_assert, would u mind to test this again?

stacepellegrino commented at 4:03 pm on July 1, 2018: none

That works! Please update the main branch of source with this change.

However, my build failed with…

CXX compat/libbitcoin_util_a-glibc_sanity.o In file included from compat/glibc_sanity.cpp:12:0: compat/glibc_sanity.cpp: In function ‘bool {anonymous}::sanity_test_fdelt()’: compat/glibc_sanity.cpp:53:5: error: ‘memset’ was not declared in this scope FD_ZERO(&fds); ^ Makefile:6248: recipe for target ‘compat/libbitcoin_util_a-glibc_sanity.o’ failed make[2]: *** [compat/libbitcoin_util_a-glibc_sanity.o] Error 1 make[2]: Leaving directory ‘/opt/local/src/bitcoin/src’ Makefile:9824: recipe for target ‘all-recursive’ failed make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory ‘/opt/local/src/bitcoin/src’ Makefile:757: recipe for target ‘all-recursive’ failed make: *** [all-recursive] Error 1

Shall I open this as a new issue?

stacepellegrino commented at 4:39 pm on July 1, 2018: none

Checking for type char equals int8_t fixed.

Opened a new build issue #13581

in configure.ac:814 in 49d1f4cdde

807@@ -808,6 +808,14 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <sys/types.h>
808  [ AC_MSG_RESULT(no)]
809 )
810 
811+AC_MSG_CHECKING(for if type char equals int8_t)
812+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdint.h>
813+  #include <type_traits>]],
814+ [[ static_assert(std::is_same<int8_t, char>::value, ""); ]])],

Empact commented at 7:06 pm on July 1, 2018:

nit: may as well provide a message for the static_assert?

Empact commented at 7:06 pm on July 1, 2018: member

utACK 49d1f4c

jb55 approved

jb55 commented at 5:07 pm on July 3, 2018: member

utACK 49d1f4cdde6d3289cb8c18ad35fc739371e25388

laanwj commented at 9:43 am on July 4, 2018: member

Can someone please explain why this is needed? I’ve been compiling bitcoin core on platforms that have char=uint8_t (such as ARM) as well as char=int8_t (such as x86) for years without needing any special define logic for this. What is different on this platform?

laanwj requested review from theuni on Jul 4, 2018

luke-jr commented at 10:12 am on July 4, 2018: member

Apparently ARM and x86 have int8_t = signed char, whereas SunOS has int8_t = char?

laanwj commented at 11:16 am on July 5, 2018: member

Thanks @luke-jr, I understand then. utACK 49d1f4cdde6d3289cb8c18ad35fc739371e25388

in src/serialize.h:193 in 49d1f4cdde

188@@ -189,7 +189,9 @@ template<typename X> const X& ReadWriteAsHelper(const X& x) { return x; }
189         SerializationOp(s, CSerActionUnserialize());                  \
190     }
191 
192+#ifndef CHAR_EQUALS_INT8
193 template<typename Stream> inline void Serialize(Stream& s, char a    ) { ser_writedata8(s, a); } // TODO Get rid of bare char

laanwj commented at 11:20 am on July 5, 2018:

I like the TODO here - not sure its still the goal, certainly not in this PR, would have to carefully consider the consequences for serialization.

laanwj merged this on Jul 5, 2018

laanwj closed this on Jul 5, 2018

laanwj referenced this in commit 40334c71d6 on Jul 5, 2018

ken2812221 deleted the branch on Jul 5, 2018

PastaPastaPasta referenced this in commit da9156f1a8 on Jul 7, 2020

PastaPastaPasta referenced this in commit f350352a12 on Jul 7, 2020

PastaPastaPasta referenced this in commit 06a1e20d8a on Jul 8, 2020

gades referenced this in commit d29ea73888 on Jun 26, 2021

DrahtBot locked this on Sep 8, 2021

build: Detect if char equals int8_t #13580