cmake: Installed static kernel library is unusable #30801

issue TheCharlatan openend this issue on September 3, 2024
  1. TheCharlatan commented at 3:48 pm on September 3, 2024: contributor

    Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It basically lacks all symbols, besides those defined in the kernel library target. This was a problem before where some libtool and symbol visibility issues led to the libsecp static library having to be linked too. Additionally installing libsecp was easily done though, by changing to the secp256k1 directory and running make install.

    Previously, e.g. on the v28 release branch, configuring with --with-experimental-kernel-lib --disable-shared, then doing a make install and make install in the secp256k1 directory provided working libraries. This could be verified by successfully compiling bitcoin-chainstate.cpp without the build system:

    0g++ -std=c++20 -o test_chainstate src/bitcoin-chainstate.cpp -I/home/drgrid/bitcoin/src -L/usr/local/lib -lbitcoinkernel -lsecp256k1
    

    When the same is attempted on current master, configuring with -DBUILD_KERNEL_LIB=ON -DBUILD_SHARRED_LIBS=OFF, then doing a cmake --install build, will make the same g++ command error with e.g:

    0/usr/bin/ld: ./build/src/kernel/./build/src/kernel/./src/hash.cpp:89:(.text+0x2f6): undefined reference to `CSHA256::Write(unsigned char const*, unsigned long)'
    1/usr/bin/ld: ./build/src/kernel/./build/src/kernel/./src/hash.cpp:89:(.text+0x306): undefined reference to `CSHA256::Finalize(unsigned char*)'
    

    When attempting to compile bitcoin-chainstate through cmake, the following is visible in the logs, which enumerates the missing libraries:

    0[100%] Linking CXX executable bitcoin-chainstate
    1cd /home/drgrid/bitcoin/build/src && /usr/bin/cmake -E cmake_link_script CMakeFiles/bitcoin-chainstate.dir/link.txt --verbose=1
    2/usr/bin/clang++ -O2 -g -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fPIE -pie "CMakeFiles/bitcoin-chainstate.dir/bitcoin-chainstate.cpp.o" -o bitcoin-chainstate  kernel/libbitcoinkernel.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 secp256k1/src/libsecp256k1.a  
    

    Evidently cmake is capable of correctly calculating the required libraries within the Bitcoin Core cmake project, but it is not clear to me what the best approach towards solving this for the installed library might be. Cmake seems to be inherently incapable of combining static libraries. A potential solution for this could be adding an ar hack that manually “re-ars” the archives into a single, static kernel library (though it is not clear how to recursively calculate the required libraries). Another solution could also be shipping all these small, additional libraries in a separate install step.

  2. fanquake added the label Build system on Sep 3, 2024
  3. ryanofsky commented at 4:00 pm on September 3, 2024: contributor

    I wonder if this approach would work: https://stackoverflow.com/a/78099611

    Instead of target_link_libraries(bitcoinkernel bitcoin_crypto) do add_library(bitcoinkernel $<TARGET_OBJECTS:bitcoin_crypto>.

    I’d also wonder if it’s really necessary to build a combined library, or better to just install the individual libraries we already have.

  4. hebasto commented at 4:09 pm on September 3, 2024: member

    I’d also wonder if it’s really necessary to build a combined library, or better to just install the individual libraries we already have.

    +1

  5. TheCharlatan commented at 4:11 pm on September 3, 2024: contributor

    Instead of target_link_libraries(bitcoinkernel bitcoin_crypto) do add_library(bitcoinkernel $<TARGET_OBJECTS:bitcoin_crypto>.

    He, I think I was reading the same stack overflow post. I think this doesn’t quite work, because those libraries have sub-dependencies that don’t get included easily. So we’d need more logic to get the correct crc32 and crypto extensions if we attempt that patch. I’m not sure that would be really desirable, since we’d be duplicating a bunch of implementation selection logic.

    0/usr/bin/ld: /usr/local/lib/libbitcoinkernel.a(sha256.cpp.o):(.debug_addr+0x20d0): undefined reference to `sha256_x86_shani::Transform(unsigned int*, unsigned char const*, unsigned long)'
    1/usr/bin/ld: /usr/local/lib/libbitcoinkernel.a(crc32c.cc.o): in function `leveldb::port::AcceleratedCRC32C(unsigned int, char const*, unsigned long)':
    2./build/./src/leveldb/port/port_stdcxx.h:140:(.text+0x66): undefined reference to `crc32c::Extend(unsigned int, unsigned char const*, unsigned long)'
    
  6. ryanofsky commented at 4:48 pm on September 3, 2024: contributor

    I see. It does seem like cmake doesn’t want to support “convenience libraries” (static libraries which are combinations of other static libraries) in the same way that libtool does. Which is kind of understandable because it would complicate dependency tracking and result in multiple libraries with duplicate copies of the same symbols which waste space and might not be safely linkable together.

    Maybe using TARGET_OBJECTS and manually specifying recursive library dependencies would be a workable initial solution, and later could decide if we want to automate this figure out recursive dependencies generically like https://stackoverflow.com/a/66575545. I think there is a probably a simpler implementation of the bundle_static_library function defined there which could be using TARGET_OBJECTS

    Also again it could be a good idea to abandon combined library approach and just install individual libraries. If the downstream project you are are trying to use the library from is not a cmake project we could generate a pkg-config .pc file that just tells it what compiler and linker flags to use so it does not have to be aware of names or structure of our libraries at all and can just apply the flags.

  7. fanquake referenced this in commit 11e2f9fff4 on Sep 12, 2024
  8. fanquake referenced this in commit 24817e8b15 on Sep 12, 2024
  9. fanquake commented at 3:21 pm on September 13, 2024: member
    @TheCharlatan what is the status of this now?
  10. TheCharlatan commented at 3:44 pm on September 13, 2024: contributor

    @TheCharlatan what is the status of this now?

    Just tested that this works with the kernel rust crate now for statically linking the library into a rust binary. So I think we are done here.

  11. TheCharlatan closed this on Sep 13, 2024


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-03 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me