ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc

[fanquake]

master @ 65c05db.
Running time FILE_ENV="./ci/test/00_setup_env_native_fuzz.sh" ./ci/test_run_all.sh on Rawhide aarch64.

Run coincontrol with args ['/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/coincontrol')]crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0x52d000000406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0x52d000000406: note: pointer points here
 b9 c5 22 00 01 01  1a 6c 65 76 65 6c 64 62  2e 42 79 74 65 77 69 73  65 43 6f 6d 70 61 72 61  74 6f
             ^ 
    #0 0xaaaab69963e4 in crc32c::ExtendArm64(unsigned int, unsigned char const*, unsigned long) src/crc32c/src/crc32c_arm64.cc:101:26
    #1 0xaaaab68cd024 in leveldb::crc32c::Value(char const*, unsigned long) src/./leveldb/util/crc32c.h:20:60
    #2 0xaaaab68cd024 in leveldb::log::Reader::ReadPhysicalRecord(leveldb::Slice*) src/leveldb/db/log_reader.cc:246:29
    #3 0xaaaab68cb594 in leveldb::log::Reader::ReadRecord(leveldb::Slice*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) src/leveldb/db/log_reader.cc:72:38
    #4 0xaaaab68fb1b0 in leveldb::VersionSet::Recover(bool*) src/leveldb/db/version_set.cc:910:19
    #5 0xaaaab6878208 in leveldb::DBImpl::Recover(leveldb::VersionEdit*, bool*) src/leveldb/db/db_impl.cc:320:18
    #6 0xaaaab68a9ec8 in leveldb::DB::Open(leveldb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, leveldb::DB**) src/leveldb/db/db_impl.cc:1484:20
    #7 0xaaaab4ca3858 in CDBWrapper::CDBWrapper(DBParams const&) src/dbwrapper.cpp:247:30
    #8 0xaaaab4b53fb0 in kernel::BlockTreeDB::BlockTreeDB(DBParams const&) src/./node/blockstorage.h:53:23
    #9 0xaaaab4b53fb0 in std::__detail::_MakeUniq<kernel::BlockTreeDB>::__single_object std::make_unique<kernel::BlockTreeDB, DBParams>(DBParams&&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
    #10 0xaaaab4b53fb0 in ChainTestingSetup::ChainTestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/test/util/setup_common.cpp:198:51
    #11 0xaaaab4b5932c in TestingSetup::TestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&, bool, bool) src/test/util/setup_common.cpp:250:7
    #12 0xaaaab44ae3b8 in std::__detail::_MakeUniq<TestingSetup const>::__single_object std::make_unique<TestingSetup const, ChainType const&, std::vector<char const*, std::allocator<char const*>> const&>(ChainType const&, std::vector<char const*, std::allocator<char const*>> const&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
    #13 0xaaaab44ae3b8 in std::unique_ptr<TestingSetup const, std::default_delete<TestingSetup const>> MakeNoLogFileContext<TestingSetup const>(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/./test/util/setup_common.h:225:12
    #14 0xaaaab44a44e8 in wallet::(anonymous namespace)::initialize_coincontrol() src/wallet/test/fuzz/coincontrol.cpp:19:39
    #15 0xaaaab4ba0f94 in std::function<void ()>::operator()() const /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    #16 0xaaaab4ba0f94 in initialize() src/test/fuzz/fuzz.cpp:130:5
    #17 0xaaaab4ba2fc4 in LLVMFuzzerInitialize src/test/fuzz/fuzz.cpp:186:5
    #18 0xaaaab43962d4 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19862d4) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
    #19 0xaaaab43c0cc8 in main (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19b0cc8) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
    #20 0xffffaab57580  (/lib/aarch64-linux-gnu/libc.so.6+0x27580) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
    #21 0xffffaab57654 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x27654) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
    #22 0xaaaab438e26c in _start (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x197e26c) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)

SUMMARY: UndefinedBehaviorSanitizer: misaligned-pointer-use crc32c/src/crc32c_arm64.cc:101:26 in 

crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0x52d000000406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0x52d000000406: note: pointer points here
 b9 c5 22 00 01 01  1a 6c 65 76 65 6c 64 62  2e 42 79 74 65 77 69 73  65 43 6f 6d 70 61 72 61  74 6f
             ^ 
    #0 0xaaaab69963e4 in crc32c::ExtendArm64(unsigned int, unsigned char const*, unsigned long) src/crc32c/src/crc32c_arm64.cc:101:26
    #1 0xaaaab68cd024 in leveldb::crc32c::Value(char const*, unsigned long) src/./leveldb/util/crc32c.h:20:60
    #2 0xaaaab68cd024 in leveldb::log::Reader::ReadPhysicalRecord(leveldb::Slice*) src/leveldb/db/log_reader.cc:246:29
    #3 0xaaaab68cb594 in leveldb::log::Reader::ReadRecord(leveldb::Slice*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) src/leveldb/db/log_reader.cc:72:38
    #4 0xaaaab68fb1b0 in leveldb::VersionSet::Recover(bool*) src/leveldb/db/version_set.cc:910:19
    #5 0xaaaab6878208 in leveldb::DBImpl::Recover(leveldb::VersionEdit*, bool*) src/leveldb/db/db_impl.cc:320:18
    #6 0xaaaab68a9ec8 in leveldb::DB::Open(leveldb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, leveldb::DB**) src/leveldb/db/db_impl.cc:1484:20
    #7 0xaaaab4ca3858 in CDBWrapper::CDBWrapper(DBParams const&) src/dbwrapper.cpp:247:30
    #8 0xaaaab4b53fb0 in kernel::BlockTreeDB::BlockTreeDB(DBParams const&) src/./node/blockstorage.h:53:23
    #9 0xaaaab4b53fb0 in std::__detail::_MakeUniq<kernel::BlockTreeDB>::__single_object std::make_unique<kernel::BlockTreeDB, DBParams>(DBParams&&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
    #10 0xaaaab4b53fb0 in ChainTestingSetup::ChainTestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/test/util/setup_common.cpp:198:51
    #11 0xaaaab4b5932c in TestingSetup::TestingSetup(ChainType, std::vector<char const*, std::allocator<char const*>> const&, bool, bool) src/test/util/setup_common.cpp:250:7
    #12 0xaaaab44ae3b8 in std::__detail::_MakeUniq<TestingSetup const>::__single_object std::make_unique<TestingSetup const, ChainType const&, std::vector<char const*, std::allocator<char const*>> const&>(ChainType const&, std::vector<char const*, std::allocator<char const*>> const&) /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1070:34
    #13 0xaaaab44ae3b8 in std::unique_ptr<TestingSetup const, std::default_delete<TestingSetup const>> MakeNoLogFileContext<TestingSetup const>(ChainType, std::vector<char const*, std::allocator<char const*>> const&) src/./test/util/setup_common.h:225:12
    #14 0xaaaab44a44e8 in wallet::(anonymous namespace)::initialize_coincontrol() src/wallet/test/fuzz/coincontrol.cpp:19:39
    #15 0xaaaab4ba0f94 in std::function<void ()>::operator()() const /usr/bin/../lib/gcc/aarch64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    #16 0xaaaab4ba0f94 in initialize() src/test/fuzz/fuzz.cpp:130:5
    #17 0xaaaab4ba2fc4 in LLVMFuzzerInitialize src/test/fuzz/fuzz.cpp:186:5
    #18 0xaaaab43962d4 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19862d4) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
    #19 0xaaaab43c0cc8 in main (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x19b0cc8) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)
    #20 0xffffaab57580  (/lib/aarch64-linux-gnu/libc.so.6+0x27580) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
    #21 0xffffaab57654 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x27654) (BuildId: ee37e53ec1958e9bebf5b3d5f81a3039cf0a1851)
    #22 0xaaaab438e26c in _start (/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz+0x197e26c) (BuildId: 289091bbf42eee6985a3c25d293425265dca4365)

SUMMARY: UndefinedBehaviorSanitizer: misaligned-pointer-use crc32c/src/crc32c_arm64.cc:101:26 in 

Target ['/ci_container_base/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/coincontrol')] failed with exit code 1

CPU Info:

cat /proc/cpuinfo
processor	: 0
BogoMIPS	: 50.00
Features	: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs
CPU implementer	: 0x41
CPU architecture: 8
CPU variant	: 0x3
CPU part	: 0xd0c
CPU revision	: 1

[maflcko]

Same with ./ci/test/00_setup_env_native_asan.sh.

I somehow assumed that this was fixed by:

• ci: use LLVM/clang-16 in native_asan job #27360
• ci: Use TSan new runtime (llvm-16, take 3) #27298
• ci: use LLVM/clang-16 in native_fuzz (ASAN) job #27363

[maflcko]

I re-tried commit 7b45d17 and at least Asan is passing. Fuzz and Tsan are failing for different reasons.

[hebasto]

Should we use the -mstrict-align option?

[maflcko]

# git bisect bad
228d6a2969e4fcee573c9df7aad31550eab9c8d4 is the first bad commit
commit 228d6a2969e4fcee573c9df7aad31550eab9c8d4
Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date:   Mon Nov 20 13:37:44 2023 +0000

    build: Fix regression in "ARMv8 CRC32 intrinsics" test
    
    The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
    are optional extensions, so they get enabled with a `+crypto` for
    architecture flags.

 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[hebasto]

I can reproduce the issue on Debian 11, aarch64.

[hebasto]

A possible solution suggested in bitcoin-core/crc32c-subtree#6.

[theuni]

FWIW @hebasto's solution looks correct (and necessary) to me.

An alternative would be to use c++20's std::assume_aligned if we could guarantee the input's alignment, but looking at the usage that's definitely not the case.

[fanquake]

@hebasto Can you update bitcoin-core/crc32c-subtree#6 so that the commit includes a description of the problem, an explanation of the fix, and any further information. We should probably add a comment inline explaining why a memcpy is being added to that code.

[theuni]

It'd be worth taking upstream as well.

[fanquake]

It'd be worth taking upstream as well.

We could open a PR, but I'm not sure if we'll get any traction there. The last change upstream https://github.com/google/crc32c was ~ 3 years ago, and the readme recommends the use of https://github.com/google/highwayhash, which is actively maintained.

[hebasto]

Can you update bitcoin-core/crc32c-subtree#6 so that the commit includes a description of the problem, an explanation of the fix, and any further information. We should probably add a comment inline explaining why a memcpy is being added to that code.

Thanks! Done.