-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ci: replace UBSAN job by ASAN and add extra sanitizers: detect_leaks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1 #7503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
c5cc656
f2fd2b4
c01c844
a90f9a7
5a7fdfe
bfacbd7
d1b8b48
738269b
4ceb947
064cb63
df045ac
8d903aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,9 @@ export LC_ALL=C.UTF-8 | |
| source ./ci/test/00_setup_env.sh | ||
|
|
||
| # Configure sanitizers options | ||
| export ASAN_OPTIONS="" | ||
| export ASAN_OPTIONS="detect_leaks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Retracted — not actionable for this PR The cumulative verifier recheck confirmed this belongs to the separate, unchanged fuzz path. PR #7503 does not advertise bitcoin#30665 as a backport, and leak detection remains enabled by default there, so requiring this hunk would expand the PR into adjacent work. No change is requested. The final review summary has been corrected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in this update — Missing prerequisite hunk from bitcoin#30665 no longer present. Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread. |
||
| export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan" | ||
| export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1" | ||
| export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:second_deadlock_stack=1" | ||
| export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" | ||
|
|
||
| if [ "$BUILD_TARGET" = "aarch64-linux" ]; then | ||
|
|
@@ -33,7 +33,11 @@ elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then | |
| elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then | ||
| source ./ci/test/00_setup_env_native_tsan.sh | ||
| elif [ "$BUILD_TARGET" = "linux64_ubsan" ]; then | ||
| source ./ci/test/00_setup_env_native_ubsan.sh | ||
| # TODO: remove it when #7503 will get merged. That's a temporary workaround to check asan on CI | ||
| # Compatibility for pull_request_target workflows that still request the legacy target. | ||
| # Their default-branch container setup does not grant SYS_PTRACE. | ||
| export ASAN_OPTIONS="detect_leaks=0:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" | ||
| source ./ci/test/00_setup_env_native_asan.sh | ||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Drop the temporary legacy-workflow workaround before merge This branch exists only so the pre-merge source: ['codex'] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in this update — Drop the temporary legacy-workflow workaround before merge no longer present. Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correction: the immediately preceding automated “resolved” reply was posted in error. This finding remains valid at exact head
Canonical final verifier status: STILL_VALID (suggestion, non-blocking). |
||
| elif [ "$BUILD_TARGET" = "linux64_valgrind" ]; then | ||
| source ./ci/test/00_setup_env_native_valgrind.sh | ||
| elif [ "$BUILD_TARGET" = "mac" ]; then | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,12 +6,15 @@ | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| export LC_ALL=C.UTF-8 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" | ||||||||||||||||||||||
| export NO_DEPENDS=1 | ||||||||||||||||||||||
| export CONTAINER_NAME=ci_native_asan | ||||||||||||||||||||||
| export PACKAGES="clang-19 llvm-19 libclang-rt-19-dev python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" | ||||||||||||||||||||||
| # Reuses the depends built for the linux64 target, which uses the defaults. | ||||||||||||||||||||||
| export DEP_OPTS="" | ||||||||||||||||||||||
| export TEST_RUNNER_EXTRA="--timeout-factor=4" # Increase timeout because sanitizers slow down | ||||||||||||||||||||||
| export FUNCTIONAL_TESTS_CONFIG="--exclude wallet_multiwallet.py" # Temporarily suppress ASan heap-use-after-free (see issue #14163) | ||||||||||||||||||||||
| export RUN_BENCH=true | ||||||||||||||||||||||
| export TEST_RUNNER_EXTRA="-j3" | ||||||||||||||||||||||
|
Comment on lines
13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Append the ASAN worker cap instead of replacing existing options Line 13 sets
Suggested change
source: ['codex']
Comment on lines
13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: New worker-count commit clobbers TEST_RUNNER_EXTRA instead of appending to it Commit
Suggested change
source: ['claude'] |
||||||||||||||||||||||
| export GOAL="install" | ||||||||||||||||||||||
| export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 \ | ||||||||||||||||||||||
| CPPFLAGS=-DDEBUG_LOCKORDER \ | ||||||||||||||||||||||
| --with-sanitizers=address,float-divide-by-zero,integer,undefined CC=clang CXX=clang++" | ||||||||||||||||||||||
| export BITCOIN_CONFIG="--enable-zmq --enable-crash-hooks --with-gui=qt5 \ | ||||||||||||||||||||||
| --with-sanitizers=address,float-divide-by-zero,integer,undefined \ | ||||||||||||||||||||||
| CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' \ | ||||||||||||||||||||||
| CC='clang-19 -ftrivial-auto-var-init=pattern' CXX='clang++-19 -ftrivial-auto-var-init=pattern'" | ||||||||||||||||||||||
| export PYZMQ=true | ||||||||||||||||||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -393,7 +393,8 @@ if test "$use_sanitizers" != ""; then | |
| dnl fail if a bad argument is passed, e.g. -fsanitize=undfeined | ||
| AX_CHECK_COMPILE_FLAG( | ||
| [-fsanitize=$use_sanitizers], | ||
| [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers"], | ||
| [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers" | ||
| SANITIZER_CFLAGS="-fsanitize=$use_sanitizers"], | ||
|
Comment on lines
+396
to
+397
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This assigns AGENTS.md reference: AGENTS.md:L150-L152 Useful? React with 👍 / 👎. |
||
| [AC_MSG_ERROR([compiler did not accept requested flags])]) | ||
|
|
||
| dnl Some compilers (e.g. GCC) require additional libraries like libasan, | ||
|
|
@@ -2079,6 +2080,9 @@ CPPFLAGS_TEMP="$CPPFLAGS" | |
| unset CPPFLAGS | ||
| CPPFLAGS="$CPPFLAGS_TEMP" | ||
|
|
||
| if test -n "$use_sanitizers"; then | ||
| export SECP_CFLAGS="$SECP_CFLAGS $SANITIZER_CFLAGS" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Useful? React with 👍 / 👎. |
||
| fi | ||
| ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh --disable-openssl-tests" | ||
| AC_CONFIG_SUBDIRS([src/dashbls src/secp256k1]) | ||
|
|
||
|
|
@@ -2139,7 +2143,7 @@ echo " target os = $host_os" | |
| echo " build os = $build_os" | ||
| echo | ||
| echo " CC = $CC" | ||
| echo " CFLAGS = $DEBUG_CFLAGS $PTHREAD_CFLAGS $BACKTRACE_FLAGS $CFLAGS" | ||
| echo " CFLAGS = $DEBUG_CFLAGS $PTHREAD_CFLAGS $SANITIZER_CFLAGS $BACKTRACE_FLAGS $CFLAGS" | ||
| echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" | ||
| echo " CXX = $CXX" | ||
| echo " CXXFLAGS = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $CORE_CXXFLAGS $BACKTRACE_FLAGS $CXXFLAGS" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,4 @@ | ||
| # Suppress warnings triggered in dependencies | ||
| leak:libQt5Widgets | ||
| leak:QDBusConnectionPrivate | ||
| leak:QLayoutPrivate |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,25 +12,27 @@ unsigned-integer-overflow:*/include/c++/ | |
| unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange | ||
| unsigned-integer-overflow:leveldb/ | ||
| unsigned-integer-overflow:minisketch/ | ||
| unsigned-integer-overflow:secp256k1/ | ||
| unsigned-integer-overflow:secp256k1* | ||
| unsigned-integer-overflow:test/fuzz/crypto_diff_fuzz_chacha20.cpp | ||
| implicit-integer-sign-change:*/include/boost/ | ||
| implicit-integer-sign-change:*/include/c++/ | ||
| implicit-integer-sign-change:*/new_allocator.h | ||
| implicit-integer-sign-change:crc32c/ | ||
| implicit-integer-sign-change:minisketch/ | ||
| implicit-integer-sign-change:secp256k1/ | ||
| implicit-integer-sign-change:secp256k1* | ||
| implicit-signed-integer-truncation:*/include/c++/ | ||
| implicit-signed-integer-truncation:leveldb/ | ||
| implicit-signed-integer-truncation:secp256k1/ | ||
| implicit-signed-integer-truncation:secp256k1* | ||
| implicit-signed-integer-truncation,implicit-integer-sign-change:secp256k1_modinv64_posdivsteps_62_var | ||
| implicit-unsigned-integer-truncation:*/include/c++/ | ||
| implicit-unsigned-integer-truncation:leveldb/ | ||
| implicit-unsigned-integer-truncation:secp256k1/ | ||
| implicit-unsigned-integer-truncation:secp256k1* | ||
| implicit-signed-integer-truncation,implicit-integer-sign-change:secp256k1* | ||
| implicit-unsigned-integer-truncation:test/fuzz/crypto_diff_fuzz_chacha20.cpp | ||
| shift-base:*/include/c++/ | ||
| shift-base:leveldb/ | ||
| shift-base:minisketch/ | ||
| shift-base:secp256k1/ | ||
| shift-base:secp256k1* | ||
|
Comment on lines
+15
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Keep secp256k1 suppressions scoped away from first-party fuzz files LLVM 19's source: ['claude', 'codex'] |
||
| shift-base:test/fuzz/crypto_diff_fuzz_chacha20.cpp | ||
| # Unsigned integer overflow occurs when the result of an unsigned integer | ||
| # computation cannot be represented in its type. Unlike signed integer overflow, | ||
|
|
@@ -46,6 +48,13 @@ unsigned-integer-overflow:compressor.cpp | |
| unsigned-integer-overflow:crypto/ | ||
| unsigned-integer-overflow:hash.cpp | ||
| unsigned-integer-overflow:lcg.h | ||
| # The quorum snapshot skip list stores offsets relative to the first skipped | ||
| # index, and the index wraps around the sorted MN list, so an offset can be | ||
| # negative. Encoding wraps size_t and narrows into vector<int>, decoding wraps | ||
| # it back; the two cancel and the absolute index is recovered exactly. Per | ||
| # symbol rather than per file so the rest of llmq/utils.cpp stays covered. | ||
| unsigned-integer-overflow:BuildNewQuorumQuarterMembers | ||
| unsigned-integer-overflow:GetQuorumQuarterMembersBySnapshot | ||
| unsigned-integer-overflow:policy/fees.cpp | ||
| unsigned-integer-overflow:prevector.h | ||
| unsigned-integer-overflow:EvalScript | ||
|
|
@@ -54,6 +63,7 @@ unsigned-integer-overflow:xoroshiro128plusplus.h | |
| implicit-integer-sign-change:addrman.h | ||
| implicit-integer-sign-change:compat/stdin.cpp | ||
| implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx | ||
| implicit-integer-sign-change:GetQuorumQuarterMembersBySnapshot | ||
| implicit-integer-sign-change:compressor.h | ||
| implicit-integer-sign-change:crypto/ | ||
| implicit-integer-sign-change:policy/fees.cpp | ||
|
|
@@ -66,6 +76,7 @@ implicit-integer-sign-change:util/strencodings.cpp | |
| implicit-integer-sign-change:util/strencodings.h | ||
| implicit-integer-sign-change:validation.cpp | ||
| implicit-signed-integer-truncation,implicit-integer-sign-change:test/skiplist_tests.cpp | ||
| implicit-signed-integer-truncation,implicit-integer-sign-change:BuildNewQuorumQuarterMembers | ||
| implicit-signed-integer-truncation:addrman.h | ||
| implicit-signed-integer-truncation:crypto/ | ||
| implicit-unsigned-integer-truncation:crypto/ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ASan functional-test job runs through
test-src.ymlincontainer-slim, but the only image updated to register an unversioned/usr/bin/llvm-symbolizeriscontrib/containers/ci/ci.Dockerfile;ci-slim.Dockerfilestill lacks the matching alternative setup. When ASan/LSan/UBSan reports in these functional tests, the runtime can fall back to raw addresses, which makes suppression matching unreliable and leaves the new job noisy or failing; use the full CI image here or add the same symbolizer setup to the slim image.Useful? React with 👍 / 👎.