diff options
author | fanquake <fanquake@gmail.com> | 2024-02-29 16:07:50 -0500 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2024-02-29 16:14:41 -0500 |
commit | dfc35c99340da3236e2841b348580f0e881762ce (patch) | |
tree | ad823ecdd28108fba4f032f83c61f949875e04fc | |
parent | be5399e7858da853e9417c28327b9c3478eb7238 (diff) | |
parent | f8a06f7a02be83e9b76a1b31f1b66a965dbedfce (diff) |
Merge bitcoin/bitcoin#29407: build: remove confusing and inconsistent disable-asm option
f8a06f7a02be83e9b76a1b31f1b66a965dbedfce doc: remove references to disable-asm option now that it's gone (Cory Fields)
376f0f6d0798c10f09266d609afea3ada1b99f9b build: remove confusing and inconsistent disable-asm option (Cory Fields)
Pull request description:
1. It didn't actually disable asm usage in our code. Regardless of the setting, asm is used in random.cpp and support/cleanse.cpp.
2. The value wasn't forwarded to libsecp as a user might have reasonably expected.
3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm actually did in practice.
If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new configure option that actually does what it says.
Additionally, this is one of the last (THE last?) remaining uses of autoconf defines in our crypto code. As such it seems like low-hanging fruit.
ACKs for top commit:
fanquake:
ACK f8a06f7a02be83e9b76a1b31f1b66a965dbedfce
Tree-SHA512: 4a99c2130225acbe9dc7399ed572a04ca155cbfa3eef8178a632ba533017d264691e6482cceb1d8f9c5d768619d99a2466dea4b82b27b18b872bceae91b92fbb
-rw-r--r-- | configure.ac | 16 | ||||
-rw-r--r-- | doc/developer-notes.md | 7 | ||||
-rw-r--r-- | doc/fuzzing.md | 7 | ||||
-rw-r--r-- | src/Makefile.am | 2 | ||||
-rw-r--r-- | src/crypto/sha256.cpp | 8 |
5 files changed, 4 insertions, 36 deletions
diff --git a/configure.ac b/configure.ac index 078d968b0c..65c390651f 100644 --- a/configure.ac +++ b/configure.ac @@ -249,16 +249,6 @@ AC_ARG_ENABLE([threadlocal], [use_thread_local=$enableval], [use_thread_local=auto]) -AC_ARG_ENABLE([asm], - [AS_HELP_STRING([--disable-asm], - [disable assembly routines (enabled by default)])], - [use_asm=$enableval], - [use_asm=yes]) - -if test "$use_asm" = "yes"; then - AC_DEFINE([USE_ASM], [1], [Define this symbol to build in assembly routines]) -fi - AC_ARG_ENABLE([zmq], [AS_HELP_STRING([--disable-zmq], [disable ZMQ notifications])], @@ -460,8 +450,6 @@ enable_sse41=no enable_avx2=no enable_x86_shani=no -if test "$use_asm" = "yes"; then - dnl Check for optional instruction set support. Enabling these does _not_ imply that all code will dnl be compiled with them, rather that specific objects/libs may use them after checking for runtime dnl compatibility. @@ -600,8 +588,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ) CXXFLAGS="$TEMP_CXXFLAGS" -fi - CORE_CPPFLAGS="$CORE_CPPFLAGS -DHAVE_BUILD_INFO" AC_ARG_WITH([utils], @@ -1817,7 +1803,6 @@ AM_CONDITIONAL([ENABLE_AVX2], [test "$enable_avx2" = "yes"]) AM_CONDITIONAL([ENABLE_X86_SHANI], [test "$enable_x86_shani" = "yes"]) AM_CONDITIONAL([ENABLE_ARM_CRC], [test "$enable_arm_crc" = "yes"]) AM_CONDITIONAL([ENABLE_ARM_SHANI], [test "$enable_arm_shani" = "yes"]) -AM_CONDITIONAL([USE_ASM], [test "$use_asm" = "yes"]) AM_CONDITIONAL([WORDS_BIGENDIAN], [test "$ac_cv_c_bigendian" = "yes"]) AM_CONDITIONAL([USE_NATPMP], [test "$use_natpmp" = "yes"]) AM_CONDITIONAL([USE_UPNP], [test "$use_upnp" = "yes"]) @@ -1972,7 +1957,6 @@ echo " with fuzz binary = $enable_fuzz_binary" echo " with bench = $use_bench" echo " with upnp = $use_upnp" echo " with natpmp = $use_natpmp" -echo " use asm = $use_asm" echo " USDT tracing = $use_usdt" echo " sanitizers = $use_sanitizers" echo " debug enabled = $enable_debug" diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 8c3845c66c..13b9016d40 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -577,13 +577,6 @@ export UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:prin See the CI config for more examples, and upstream documentation for more information about any additional options. -There are a number of known problems when using the `address` sanitizer. The -address sanitizer is known to fail in -[sha256_sse4::Transform](/src/crypto/sha256_sse4.cpp) which makes it unusable -unless you also use `--disable-asm` when running configure. We would like to fix -sanitizer issues, so please send pull requests if you can fix any errors found -by the address sanitizer (or any other sanitizer). - Not all sanitizer options can be enabled at the same time, e.g. trying to build with `--with-sanitizers=address,thread` will fail in the configure script as these sanitizers are mutually incompatible. Refer to your compiler manual to diff --git a/doc/fuzzing.md b/doc/fuzzing.md index a4b0198dd9..c9fb918c8f 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -127,11 +127,6 @@ The default Clang/LLVM version supplied by Apple on macOS does not include fuzzing libraries, so macOS users will need to install a full version, for example using `brew install llvm`. -Should you run into problems with the address sanitizer, it is possible you -may need to run `./configure` with `--disable-asm` to avoid errors -with certain assembly code from Bitcoin Core's code. See [developer notes on sanitizers](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#sanitizers) -for more information. - You may also need to take care of giving the correct path for `clang` and `clang++`, like `CC=/path/to/clang CXX=/path/to/clang++` if the non-systems `clang` does not come first in your path. @@ -139,7 +134,7 @@ You may also need to take care of giving the correct path for `clang` and Full configure that was tested on macOS with `brew` installed `llvm`: ```sh -./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined --disable-asm CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++ +./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++ ``` Read the [libFuzzer documentation](https://llvm.org/docs/LibFuzzer.html) for more information. This [libFuzzer tutorial](https://github.com/google/fuzzing/blob/master/tutorial/libFuzzerTutorial.md) might also be of interest. diff --git a/src/Makefile.am b/src/Makefile.am index 3e8870c828..3e24ea5a7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -50,10 +50,8 @@ LIBBITCOIN_WALLET_TOOL=libbitcoin_wallet_tool.a endif LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE) -if USE_ASM LIBBITCOIN_CRYPTO_SSE4 = crypto/libbitcoin_crypto_sse4.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE4) -endif if ENABLE_SSE41 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41) diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 36ef6d9a1a..4c7bb6f20f 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -26,13 +26,11 @@ #endif #if defined(__x86_64__) || defined(__amd64__) || defined(__i386__) -#if defined(USE_ASM) namespace sha256_sse4 { void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks); } #endif -#endif namespace sha256d64_sse41 { @@ -574,7 +572,7 @@ bool SelfTest() { } #if !defined(DISABLE_OPTIMIZED_SHA256) -#if defined(USE_ASM) && (defined(__x86_64__) || defined(__amd64__) || defined(__i386__)) +#if (defined(__x86_64__) || defined(__amd64__) || defined(__i386__)) /** Check whether the OS has enabled AVX registers. */ bool AVXEnabled() { @@ -597,7 +595,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64_8way = nullptr; #if !defined(DISABLE_OPTIMIZED_SHA256) -#if defined(USE_ASM) && defined(HAVE_GETCPUID) +#if defined(HAVE_GETCPUID) bool have_sse4 = false; bool have_xsave = false; bool have_avx = false; @@ -654,7 +652,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem ret += ",avx2(8way)"; } #endif -#endif // defined(USE_ASM) && defined(HAVE_GETCPUID) +#endif // defined(HAVE_GETCPUID) #if defined(ENABLE_ARM_SHANI) bool have_arm_shani = false; |