Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Nov 17, 2025

Also, add a test for creating a CScript from an empty byte vector.

To test: rm -rf ./bld-cmake && cmake -B ./bld-cmake -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++;-stdlib=libc++;-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG' -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DBUILD_KERNEL_LIB=ON -DENABLE_WALLET=OFF -DENABLE_IPC=OFF && cmake --build ./bld-cmake --parallel $( nproc ) && valgrind --tool=none ./bld-cmake/bin/test_kernel --catch_system_error=no

Before:

/cxx_build/include/c++/v1/span:451: libc++ Hardening assertion __count == 0 || std::to_address(__first) != nullptr failed: passed nullptr with non-zero length in span's constructor (iterator, len)

After: (Passes)

@DrahtBot DrahtBot added the Tests label Nov 17, 2025
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 17, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33886.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK TheCharlatan, stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fadb4f6

Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fadb4f6

It's unfortunate that we can't test the nullptr + non-zero length paths through the c++ wrapper, but removing these cases seems like the best approach for now.

@fanquake fanquake merged commit ead849c into bitcoin:master Nov 17, 2025
23 checks passed
@maflcko maflcko deleted the 2511-test-span-hard branch November 17, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants