Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't compile bitcoin for SmartOS - recipe for target 'compat/libbitcoin_util_a-glibc_sanity.o' failed #13581

Closed
stacepellegrino opened this issue Jul 1, 2018 · 22 comments

Comments

@stacepellegrino
Copy link

@stacepellegrino stacepellegrino commented Jul 1, 2018

After fixing #13580 my build still fails 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

I have attached
config.log for your review.

@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 1, 2018

# make V=1
Making all in src
make[1]: Entering directory '/opt/local/src/bitcoin/src'
make[2]: Entering directory '/opt/local/src/bitcoin/src'
make[3]: Entering directory '/opt/local/src/bitcoin'
make[3]: Leaving directory '/opt/local/src/bitcoin'
g++ -std=c++11 -DHAVE_CONFIG_H -I. -I../src/config   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I. -I/opt/local/include/db4/ -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/opt/local/include -I./leveldb/include -I./leveldb/helpers/memenv -I/opt/local/include -I/opt/local/include -I./secp256k1/include -I./univalue/include -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS  -Wstack-protector -fstack-protector-all -Wall -Wextra -Wformat -Wvla -Wno-unused-parameter    -fPIE -g -O2 -MT compat/libbitcoin_util_a-glibc_sanity.o -MD -MP -MF compat/.deps/libbitcoin_util_a-glibc_sanity.Tpo -c -o compat/libbitcoin_util_a-glibc_sanity.o `test -f 'compat/glibc_sanity.cpp' || echo './'`compat/glibc_sanity.cpp
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
@Empact
Copy link
Member

@Empact Empact commented Jul 1, 2018

I'll take this.

@Empact
Copy link
Member

@Empact Empact commented Jul 1, 2018

Could you try building my branch at #13584?

@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 1, 2018

Just doing a cloning your branch to build now.

@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 1, 2018

What commands do I use to fetch, merge and build your branch?

@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 2, 2018

Just figured it out...

# git remote show origin
# git merge master
# git config --global user.email stacey.pellegrino@gmail.com
# git config --global user.name "Stacey Pellegrino"
# git commit
@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 2, 2018

I don't think the branch pulled as I am still getting the same build error. What are the commands needed to pull your branch?

@Empact
Copy link
Member

@Empact Empact commented Jul 2, 2018

Not at a computer to confirm, but I think this should work:
git remote add Empact https://github.com/Empact/bitcoin
git fetch Empact
git checkout -b cstring Empact/cstring
make

@stacepellegrino
Copy link
Author

@stacepellegrino stacepellegrino commented Jul 2, 2018

Build now does not fail when compiling compat/libbitcoin_util_a-glibc_sanity.o now. @Empact 's fix solved the issue.

However, build now fails with the following:

leveldb/util/env_posix.cc: In member function 'virtual leveldb::Status leveldb::{anonymous}::PosixSequentialFile::Read(std::size_t, leveldb::Slice*, char*)':
leveldb/util/env_posix.cc:105:51: error: 'fread_unlocked' was not declared in this scope
     size_t r = fread_unlocked(scratch, 1, n, file_);
                                                   ^
leveldb/util/env_posix.cc: In member function 'virtual leveldb::Status leveldb::{anonymous}::PosixWritableFile::Append(const leveldb::Slice&)':
leveldb/util/env_posix.cc:234:66: error: 'fwrite_unlocked' was not declared in this scope
     size_t r = fwrite_unlocked(data.data(), 1, data.size(), file_);
                                                                  ^
leveldb/util/env_posix.cc: In member function 'virtual leveldb::Status leveldb::{anonymous}::PosixWritableFile::Flush()':
leveldb/util/env_posix.cc:251:30: error: 'fflush_unlocked' was not declared in this scope
     if (fflush_unlocked(file_) != 0) {
                              ^
leveldb/util/env_posix.cc: In member function 'virtual leveldb::Status leveldb::{anonymous}::PosixWritableFile::Sync()':
leveldb/util/env_posix.cc:290:30: error: 'fflush_unlocked' was not declared in this scope
     if (fflush_unlocked(file_) != 0 ||
                              ^
Makefile:5044: recipe for target 'leveldb/util/leveldb_libleveldb_a-env_posix.o' failed
make[2]: *** [leveldb/util/leveldb_libleveldb_a-env_posix.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

Creating a new issue regarding this build failure.

@Empact
Copy link
Member

@Empact Empact commented Jul 2, 2018

@stacepellegrino could you re-open this? We can close it when a fix is merged.

@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Jul 9, 2018

@Empact Are you still working on a "more targeted fix"

@Empact
Copy link
Member

@Empact Empact commented Jul 9, 2018

Sorry I forgot that PR fixed this issue. I can go with either restoring the specific include or merging the existing work, which I just reopened.

The "more target fix" I've been working on relates to applying iwyu to bitcoin, which I intend to use as a external / automated check on the include work, e.g. #13584. I'll post the related iwyu output today.

@Empact
Copy link
Member

@Empact Empact commented Jul 9, 2018

Here's a PR to fix just this instance: #13619. It may or may not be enough to get past this and onto #13585, but @stacepellegrino will have to confirm.

@Empact
Copy link
Member

@Empact Empact commented Jul 10, 2018

@stacepellegrino I don't think you have your system properly setup, e.g. see this output from configure:

configure:28711: g++ -std=c++11 -o conftest -g -O2  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/opt/local/include  conftest.cpp -L/opt/local/lib -lboost_system -lboost_filesystem -lboost_program_options -lboost_thread -lboost_chrono  >&5
conftest.cpp: In function 'int main()':
conftest.cpp:85:5: error: 'choke' was not declared in this scope
     choke;
     ^
configure:28711: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "Bitcoin Core"
| #define PACKAGE_TARNAME "bitcoin"
| #define PACKAGE_VERSION "0.16.99"
| #define PACKAGE_STRING "Bitcoin Core 0.16.99"
| #define PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues"
| #define PACKAGE_URL "https://bitcoincore.org/"
| #define HAVE_CXX11 1
| #define STDC_HEADERS 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_STDLIB_H 1
@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Jul 11, 2018

I believe this can be fixed by adding the missing include:

diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp
index 20d2ad3cb6..a99f260871 100644
--- a/src/compat/glibc_sanity.cpp
+++ b/src/compat/glibc_sanity.cpp
@@ -7,6 +7,7 @@
 #endif
 
 #include <cstddef>
+#include <cstring>
 
 #if defined(HAVE_SYS_SELECT_H)
 #include <sys/select.h>
@Empact
Copy link
Member

@Empact Empact commented Jul 11, 2018

@MarcoFalke agreed, similarly: #13619

But I'm doubting this SmartOS thread - if configure can not run successfully, then are there not some basic platform issues to address first? Seems configure should run cleanly before we tackle build errors.

@theuni
Copy link
Member

@theuni theuni commented Jul 11, 2018

@Empact sadly this is just a bug in SmartOS's libc. glibc even makes a note about not using memset for this reason.

We can work around it by splitting glibc_sanity.cpp into two pieces: One for sanity_test_memcpy and one for sanity_test_fdelt. Once split, the file with sanity_test_fdelt can safely include string.h.

@bahamas10
Copy link

@bahamas10 bahamas10 commented Jan 10, 2019

I ran into this issue as well and was able to fix it using the diff provided by @MarcoFalke

I provisioned a fresh SmartOS 16Q4 LTS instance to do this:

$ uname -v
joyent_20180927T004151Z
$ cat /etc/product
Name: Joyent Instance
Image: base-64-lts 16.4.1
Documentation: https://docs.joyent.com/images/smartos/base

Installed deps:

sudo pkgin in build-essential autoconf pkg-config boost

Pulled the source and checked out the latest version (at the time of this writing):

cd ~
git clone https://github.com/bitcoin/bitcoin.git
cd bitcoin/
git checkout v0.17.1

Autogen + configure:

./autogen.sh
./configure --disable-wallet --without-gui --without-miniupnpc --disable-tests

Patch the source:

vim ./src/compat/glibc_sanity.cpp
$ git diff
diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp
index 1ef66e27b..0166fea90 100644
--- a/src/compat/glibc_sanity.cpp
+++ b/src/compat/glibc_sanity.cpp
@@ -7,6 +7,7 @@
 #endif

 #include <cstddef>
+#include <cstring>

 #if defined(HAVE_SYS_SELECT_H)
 #include <sys/select.h>

Compiled:

make 

Ensured it worked:

$ ./src/bitcoin-cli -version
Bitcoin Core RPC client version v0.17.1.0-ef70f9b52-dirty
@Empact
Copy link
Member

@Empact Empact commented Jan 11, 2019

@bahamas10 would you try building the code on this branch?: https://github.com/Empact/bitcoin/tree/glibc-sanity-string

@bahamas10
Copy link

@bahamas10 bahamas10 commented Jan 11, 2019

@Empact Tested. I used the same instance I used in my previous comment with the same dependencies already installed:

git clone https://github.com/Empact/bitcoin.git bitcoin-smartos-fix
cd bitcoin-smartos-fix/
git checkout glibc-sanity-string
./autogen.sh 
./configure --disable-wallet --without-gui --without-miniupnpc --disable-tests
make
$ make
...
  CXXLD    bench/bench_bitcoin
Undefined                       first referenced
 symbol                             in file
getnameinfo                         libbitcoin_common.a(libbitcoin_common_a-netaddress.o)  (symbol belongs to implicit dependency /lib/amd64/libsocket.so.1)
ld: fatal: symbol referencing errors. No output written to bench/bench_bitcoin
collect2: error: ld returned 1 exit status
Makefile:3991: recipe for target 'bench/bench_bitcoin' failed
make[2]: *** [bench/bench_bitcoin] Error 1
make[2]: Leaving directory '/home/dave/dev/bitcoin-smartos-fix/src'
Makefile:10520: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/dave/dev/bitcoin-smartos-fix/src'
Makefile:773: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

I haven't looked into this tonight (I can look more tomorrow), but i'm willing to bet there probably needs to be a -lsocket somewhere.

@bahamas10
Copy link

@bahamas10 bahamas10 commented Jan 11, 2019

@Empact I was able to get this all to compile by adding -lsocket to the ./configure step

$ LIBS='-lsocket' ./configure --disable-wallet --without-gui --without-miniupnpc --disable-tests
$ make
...
$ ./src/bitcoind -version
Bitcoin Core Daemon version v0.17.99.0-effb4fdae
Copyright (C) 2009-2019 The Bitcoin Core developers
...
@laanwj laanwj closed this in dd8cf82 Sep 18, 2019
@Empact
Copy link
Member

@Empact Empact commented Sep 18, 2019

#15146 has been merged, could you try again with current master? We may need to re-open for the -lsocket issue, etc.

sidhujag added a commit to syscoin/syscoin that referenced this issue Sep 23, 2019
b4fd0ca Include cstring for sanity_test_fdelt if required (Ben Woosley)
7fb886b [moveonly] Split glibc sanity_test_fdelt out (Ben Woosley)

Pull request description:

  SmartOS FD_ZERO is implemented in a way that requires
  an external declaration of memcpy. We can not simply
  include cstring in the existing file because
  sanity_test_memcpy is attempting to replace memcpy.

  Instead split glibc_sanity into fdelt and memcpy files,
  and include <cstring> in glibc_sanity/fdelt.cpp.

  Fixes bitcoin#13581, see also bitcoin#13619

ACKs for top commit:
  laanwj:
    Code review an lightly tested (but not on SmartOS) ACK b4fd0ca

Tree-SHA512: 231306da291ad9eca8ba91bea1e9c27b6c2e96e484d1602e1c2cf27761202f9287ce0bc19fefd000943d2b449d0e5929cd39e2f7e09cf930d89fa520228ccbec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants