Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge bitcoin#19522: build: fix building libconsensus with reduced ex…
…ports for Darwin targets de4238f build: consolidate reduced export checks (fanquake) 012bdec build: add building libconsensus to end-of-configure output (fanquake) 8f360e3 build: remove ax_gcc_func_attribute macro (fanquake) f054a08 build: remove AX_GCC_FUNC_ATTRIBUTE test for dllimport (fanquake) 7cd0a69 build: test for __declspec(dllexport) in configure (fanquake) 1624e17 build: remove duplicate visibility attribute detection (fanquake) Pull request description: Darwin targets do not have a `protected` visibility function attribute, see [LLVM explanation](https://github.com/llvm/llvm-project/blob/8e9a505139fbef7d2e6e9d0adfe1efc87326f9ef/clang/lib/Basic/Targets/OSTargets.h#L131). This means that the `AX_GCC_FUNC_ATTRIBUTE` check for `visibility` fails: ```bash configure:24513: checking for __attribute__((visibility)) configure:24537: g++ -std=c++11 -o conftest -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -Wl,-headerpad_max_install_names conftest.cpp >&5 conftest.cpp:35:56: warning: target does not support 'protected' visibility; using 'default' [-Wunsupported-visibility] int foo_pro( void ) __attribute__((visibility("protected"))); ^ 1 warning generated. configure:24537: $? = 0 configure:24550: result: no ``` This leads to `EXPORT_SYMBOL` being [defined to nothing](https://github.com/bitcoin/bitcoin/blob/f4de89edfa8be4501534fec0c662c650a4ce7ef2/src/script/bitcoinconsensus.h#L29), as `HAVE_FUNC_ATTRIBUTE_VISIBILITY` is not defined, and when building with reduced exports, you end up with a libbitcoinconsensus.dylib that doesn't export any `_bitcoinconsensus_*` symbols. ```bash ➜ git:(master) nm -C src/.libs/libbitcoinconsensus.dylib | rg _bitcoinconsensus_ ➜ git:(master) ``` We do have a [second check](https://github.com/bitcoin/bitcoin/blob/f4de89edfa8be4501534fec0c662c650a4ce7ef2/configure.ac#L882) for the `visibility` attribute, which works for Darwin as it's only testing for default visibility, however the result of this check isn't used at all. It was added in #4725, along with the `--enable-reduce-exports` option, however when libbitcoinconsensus was added in #5235, it used the results of the added `AX_GCC_FUNC_ATTRIBUTE` calls. This PR removes our usage of the AX_GCC_FUNC_ATTRIBUTE macro entirely, in favour of our own checks in configure. This meant adding a check for `dllexport`, which I've tested as working with both [GCC](https://gcc.gnu.org/onlinedocs/gcc/Microsoft-Windows-Function-Attributes.html) and [Clang](https://releases.llvm.org/10.0.0/tools/clang/docs/AttributeReference.html#dllexport) when building for Windows. I haven't added an equivalent check for `dllimport`, as we weren't actually using the result of that check, we're just testing that `MSC_VER` was defined before using. With these changes building a libbitcoinconsensus with reduced exports, when targeting Darwin, works as expected: ```bash ./autogen.sh ./configure --disable-tests --disable-bench --with-utils=no --with-daemon=no --with-gui=no --disable-wallet --with-libs=yes --enable-reduce-exports make -j8 ... nm -C src/.libs/libbitcoinconsensus.dylib | rg _bitcoinconsensus_ 000000000000a340 T _bitcoinconsensus_verify_script 00000000000097e0 T _bitcoinconsensus_verify_script_with_amount 000000000000a3c0 T _bitcoinconsensus_version ``` ```python >>> import ctypes >>> consensus = ctypes.CDLL("src/.libs/libbitcoinconsensus.dylib") >>> print(consensus.bitcoinconsensus_version()) 1 >>> exit() ``` TODO: Modify a CI job to compile with --enable-reduce-exports and check for symbols in shared lib? ACKs for top commit: laanwj: Code review ACK de4238f Tree-SHA512: d148f3c55d14dac6e9e5b718cc65bb557bcf6f663218d24bc9044b86281bd5dd3d931ebea79c336a58e8ed50d683218c0a9e75494f2267b91097665043e252ae
- Loading branch information