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

cmake: Regression in feature_reindex.py when configuring with -DSANITIZERS=integer #261

Closed
wants to merge 3 commits into from

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Jul 5, 2024

I've added a new CI test scipt 00_setup_env_native_ubsan-i_reindex.sh. It fails both in the GHA CI and locally.

Dropping -DSANITIZERS=integer makes it pass.

The new test also passes on the Autotools-based branch.


NOTE: The first commit is the cmake-staging branch rebased on top of the bitcoin@aa61d4f and squashed.

This test fails both in the GHA CI and locally.

Dropping `-fsanitize=integer` makes it pass.
@hebasto hebasto added the bug Something isn't working label Jul 5, 2024
@hebasto
Copy link
Owner Author

hebasto commented Jul 5, 2024

@maflcko @fanquake @theuni @TheCharlatan @vasild @m3dwards @achow101 @laanwj

I'm asking for your help.

@hebasto hebasto added the help wanted Extra attention is needed label Jul 5, 2024
@hebasto hebasto added this to the Ready for master milestone Jul 5, 2024
@maflcko
Copy link

maflcko commented Jul 5, 2024

Looks like a normal timeout? Does it happen with other sanitizers/valgrind? Does it happen when the timeout-factor is increased? What is the compiler invocation (the line in top or htop) for autotools vs cmake? (Just asking to make sure the -O2 flag is really applied to the compiler)

@hebasto
Copy link
Owner Author

hebasto commented Jul 5, 2024

Does it happen when the timeout-factor is increased?

It does.

@hebasto
Copy link
Owner Author

hebasto commented Jul 5, 2024

CMake's compile command:

/usr/bin/ccache /usr/bin/clang++-18  -I/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src -I/ci_container_base/src -O2 -g -std=c++20 -fPIE -Werror -fsanitize=integer -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -MD -MT src/CMakeFiles/bitcoind.dir/bitcoind.cpp.o -MF CMakeFiles/bitcoind.dir/bitcoind.cpp.o.d -o CMakeFiles/bitcoind.dir/bitcoind.cpp.o -c /ci_container_base/src/bitcoind.cpp

CMake's link command:

/usr/bin/clang++-18 -O2 -g -fsanitize=integer -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fPIE -pie CMakeFiles/bitcoind.dir/bitcoind.cpp.o CMakeFiles/bitcoind.dir/init/bitcoind.cpp.o -o bitcoind  libbitcoin_node.a libbitcoin_common.a libbitcoin_consensus.a secp256k1/src/libsecp256k1.a util/libbitcoin_util.a crypto/libbitcoin_crypto.a crypto/libbitcoin_crypto_sse41.a crypto/libbitcoin_crypto_avx2.a crypto/libbitcoin_crypto_x86_shani.a ../libleveldb.a ../libcrc32c.a ../libcrc32c_sse42.a ../libminisketch.a univalue/libunivalue.a /usr/lib/x86_64-linux-gnu/libevent_pthreads.so /usr/lib/x86_64-linux-gnu/libevent.so

Autotools' compile command:

/usr/bin/ccache clang++-18 -std=c++20 -DHAVE_CONFIG_H -I. -I../src/config  -fmacro-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -DHAVE_BUILD_INFO -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include  -g -O2 -fdebug-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -Werror -fsanitize=integer -fPIE  -c -o bitcoind-bitcoind.o `test -f 'bitcoind.cpp' || echo './'`bitcoind.cpp

Autotools' link command:

/usr/bin/ccache clang++-18 -std=c++20 -g -O2 -fdebug-prefix-map=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -Werror -fsanitize=integer -fPIE -Wl,-z -Wl,relro -Wl,-z -Wl,now -Wl,-z -Wl,separate-code -pie -fsanitize=integer -o bitcoind bitcoind-bitcoind.o init/bitcoind-bitcoind.o  -lpthread libbitcoin_node.a libbitcoin_common.a libbitcoin_util.a ./.libs/libunivalue.a libbitcoin_consensus.a crypto/.libs/libbitcoin_crypto_base.a crypto/.libs/libbitcoin_crypto_sse41.a crypto/.libs/libbitcoin_crypto_avx2.a crypto/.libs/libbitcoin_crypto_x86_shani.a leveldb/.libs/libleveldb.a crc32c/.libs/libcrc32c.a crc32c/.libs/libcrc32c_sse42.a leveldb/.libs/libmemenv.a secp256k1/.libs/libsecp256k1.a -levent_pthreads -levent -levent -pthread

@maflcko
Copy link

maflcko commented Jul 5, 2024

If the other sanitizers and valgrind are fine, you can temporarily disable Isan, if it is a blocker for you. I can take another look later.

@hebasto
Copy link
Owner Author

hebasto commented Jul 5, 2024

Does it happen with other sanitizers/valgrind?

It doesn't happen with -fsanitize=undefined.

@maflcko
Copy link

maflcko commented Jul 8, 2024

Possibly a non-cmake bug. I'll keep looking.

@maflcko
Copy link

maflcko commented Jul 10, 2024

Looks like a bug where the reindex fails to continue. Not sure why, but this is not related to cmake, as I could reproduce.

@maflcko
Copy link

maflcko commented Jul 10, 2024

See bitcoin#30424

@hebasto
Copy link
Owner Author

hebasto commented Jul 11, 2024

@mzumsande

Your fix suggested in bitcoin#30424 (comment) and applied to this branch works.

Thank you!

@hebasto
Copy link
Owner Author

hebasto commented Jul 12, 2024

Closing.

Please refer to bitcoin#30435.

@hebasto hebasto closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants