ci: Use clang-15 in tsan task #26768

maflcko commented at 2:32 PM on December 29, 2022: member

Generally it is best to use the latest clang version for sanitizers, because it comes with the most features and bugfixes.

So bump to clang-15, the latest release, for the tsan task.

The task was using clang-13 (instead of 14) due to a bug, see #24572#issue-1169970859. Bumping to 15 will hopefully fix this bug, as well as #26759 (comment)

DrahtBot commented at 2:32 PM on December 29, 2022: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto

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

DrahtBot added the label Tests on Dec 29, 2022

in ci/test/00_setup_env_native_tsan.sh:14 in fad3f96a4b outdated

  13 | +export DOCKER_NAME_TAG=debian:bookworm  # For clang-15
  14 | +export PACKAGES="clang-15 llvm-15 libc++abi-15-dev libc++-15-dev python3-zmq"
  15 | +export DEP_OPTS="CC=clang-15 CXX='clang++-15 -stdlib=libc++'"
  16 |  export GOAL="install"
  17 | -export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread CC=clang-13 CXX='clang++-13 -stdlib=libc++'"
  18 | +export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread CC=clang-15 CXX='clang++-15 -stdlib=libc++'"

hebasto commented at 2:44 PM on December 29, 2022:

While touching this line, why not

export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread"

?

The config.site has already contained correct definitions of CC and CXX variables.

maflcko commented at 3:17 PM on December 29, 2022:

Thanks, done

hebasto commented at 2:44 PM on December 29, 2022: member

Concept ACK.

in .cirrus.yml:231 in fad3f96a4b outdated

 230 | @@ -231,7 +231,7 @@ task:
 231 |    name: '[TSan, depends, gui] [jammy]'

hebasto commented at 2:47 PM on December 29, 2022:

  name: '[TSan, depends, gui] [bookworm]'

maflcko commented at 3:17 PM on December 29, 2022:

Thanks, done

maflcko force-pushed on Dec 29, 2022

maflcko commented at 3:16 PM on December 29, 2022: member

Temporarily set NO_QR=1 due to a libqrencode compile failure:

qrinput.c:1338:44: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
        if(input->head->mode != QR_MODE_STRUCTURE || input->head->mode != QR_MODE_ECI) {
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
split.c:287:11: warning: call to undeclared function 'strdup'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        newstr = strdup(str);
                 ^
split.c:287:9: error: incompatible integer to pointer conversion assigning to 'char *' from 'int' [-Wint-conversion]
        newstr = strdup(str);
               ^ ~~~~~~~~~~~
1 warning and 1 error generated.
make[3]: *** [Makefile:635: split.lo] Error 1
make[3]: *** Waiting for unfinished jobs....
1 warning generated.
make[3]: Leaving directory '/tmp/cirrus-ci-build/depends/work/build/x86_64-pc-linux-gnu/qrencode/3.4.4-90541ff189b'

https://cirrus-ci.com/task/4504254476451840?logs=ci#L1259

in ci/test/00_setup_env_native_tsan.sh:12 in fa13f22e19 outdated

   6 | @@ -7,8 +7,8 @@
   7 |  export LC_ALL=C.UTF-8
   8 |  
   9 |  export CONTAINER_NAME=ci_native_tsan
  10 | -export DOCKER_NAME_TAG=ubuntu:22.04
  11 | -export PACKAGES="clang-13 llvm-13 libc++abi-13-dev libc++-13-dev python3-zmq"
  12 | -export DEP_OPTS="CC=clang-13 CXX='clang++-13 -stdlib=libc++'"
  13 | +export DOCKER_NAME_TAG=debian:bookworm  # For clang-15
  14 | +export PACKAGES="clang-15 llvm-15 libc++abi-15-dev libc++-15-dev python3-zmq"
  15 | +export DEP_OPTS="NO_QT=1 CC=clang-15 CXX='clang++-15 -stdlib=libc++'"

hebasto commented at 3:23 PM on December 29, 2022:

Did you mean

export DEP_OPTS="NO_QR=1 CC=clang-15 CXX='clang++-15 -stdlib=libc++'"

?

maflcko commented at 3:30 PM on December 29, 2022:

Thanks, fixed

ci: Use clang-15 in tsan task faa00ca78e

maflcko force-pushed on Dec 29, 2022

in ci/test/00_setup_env_native_tsan.sh:12 in faa00ca78e

   6 | @@ -7,8 +7,8 @@
   7 |  export LC_ALL=C.UTF-8
   8 |  
   9 |  export CONTAINER_NAME=ci_native_tsan
  10 | -export DOCKER_NAME_TAG=ubuntu:22.04
  11 | -export PACKAGES="clang-13 llvm-13 libc++abi-13-dev libc++-13-dev python3-zmq"
  12 | -export DEP_OPTS="CC=clang-13 CXX='clang++-13 -stdlib=libc++'"
  13 | +export DOCKER_NAME_TAG=debian:bookworm  # For clang-15
  14 | +export PACKAGES="clang-15 llvm-15 libc++abi-15-dev libc++-15-dev python3-zmq"
  15 | +export DEP_OPTS="CC=clang-15 CXX='clang++-15 -stdlib=libc++' NO_QR=1"  # qr disabled due to libqrencode 3.4.4 compile failure, https://github.com/bitcoin/bitcoin/pull/26768#issuecomment-1367403430

hebasto commented at 5:54 PM on December 29, 2022:

FWIW, bumping qrencode package version doesn't help. Maybe

--- a/depends/packages/qrencode.mk
+++ b/depends/packages/qrencode.mk
@@ -10,6 +10,7 @@ $(package)_config_opts += --disable-gprof --disable-gcov --disable-mudflap
 $(package)_config_opts += --disable-dependency-tracking --enable-option-checking
 $(package)_config_opts_linux=--with-pic
 $(package)_config_opts_android=--with-pic
+$(package)_cflags = -Wno-error=int-conversion
 endef
 
 define $(package)_preprocess_cmds

and drop NO_QR=1?

maflcko commented at 6:37 PM on December 29, 2022:

Not sure. I'd prefer to keep build system changes separate from CI-only changes.

hebasto approved

hebasto commented at 5:54 PM on December 29, 2022: member

ACK faa00ca78e27a40cbafe701348e6b3674e3ddeed

maflcko merged this on Dec 29, 2022

maflcko closed this on Dec 29, 2022

maflcko deleted the branch on Dec 29, 2022

hebasto commented at 9:31 PM on December 29, 2022: member

https://api.cirrus-ci.com/v1/task/6394543940042752/logs/ci.log:

ThreadSanitizer: CHECK failed: sanitizer_common.h:511 "((i)) < ((size_))" (0xffffffff, 0x1b) (tid=40779)
    [#0](/bitcoin-bitcoin/0/) __tsan::CheckUnwind() <null> (bitcoind+0x145acb) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#1](/bitcoin-bitcoin/1/) __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) <null> (bitcoind+0xc7704) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#2](/bitcoin-bitcoin/2/) __tsan::ScopedReportBase::AddLocation(unsigned long, unsigned long) <null> (bitcoind+0x15721f) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#3](/bitcoin-bitcoin/3/) __tsan::ReportRace(__tsan::ThreadState*, __tsan::RawShadow*, __tsan::Shadow, __tsan::Shadow, unsigned long) <null> (bitcoind+0x15967f) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#4](/bitcoin-bitcoin/4/) __tsan::DoReportRace(__tsan::ThreadState*, __tsan::RawShadow*, __tsan::Shadow, __tsan::Shadow, unsigned long) <null> (bitcoind+0x14926c) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#5](/bitcoin-bitcoin/5/) __tsan::FdClose(__tsan::ThreadState*, unsigned long, int, bool) <null> (bitcoind+0xdd150) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#6](/bitcoin-bitcoin/6/) closedir <null> (bitcoind+0xe6b25) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#7](/bitcoin-bitcoin/7/) leveldb::(anonymous namespace)::PosixEnv::GetChildren(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>>*) src/leveldb/util/env_posix.cc:599:5 (bitcoind+0xae7e28) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#8](/bitcoin-bitcoin/8/) leveldb::DBImpl::DeleteObsoleteFiles() src/leveldb/db/db_impl.cc:237:9 (bitcoind+0xaaef3d) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#9](/bitcoin-bitcoin/9/) leveldb::DBImpl::BackgroundCompaction() src/leveldb/db/db_impl.cc:752:5 (bitcoind+0xab3668) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#10](/bitcoin-bitcoin/10/) leveldb::DBImpl::BackgroundCall() src/leveldb/db/db_impl.cc:687:5 (bitcoind+0xab2d10) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#11](/bitcoin-bitcoin/11/) leveldb::DBImpl::BGWork(void*) src/leveldb/db/db_impl.cc:676:34 (bitcoind+0xab2c4b) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#12](/bitcoin-bitcoin/12/) leveldb::(anonymous namespace)::PosixEnv::BackgroundThreadMain() src/leveldb/util/env_posix.cc:830:5 (bitcoind+0xaebc73) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#13](/bitcoin-bitcoin/13/) leveldb::(anonymous namespace)::PosixEnv::BackgroundThreadEntryPoint(leveldb::(anonymous namespace)::PosixEnv*) src/leveldb/util/env_posix.cc:736:10 (bitcoind+0xaebc73)
    [#14](/bitcoin-bitcoin/14/) decltype(std::declval<void (*)(leveldb::(anonymous namespace)::PosixEnv*)>()(std::declval<leveldb::(anonymous namespace)::PosixEnv*>())) std::__1::__invoke[abi:v15006]<void (*)(leveldb::(anonymous namespace)::PosixEnv*), leveldb::(anonymous namespace)::PosixEnv*>(void (*&&)(leveldb::(anonymous namespace)::PosixEnv*), leveldb::(anonymous namespace)::PosixEnv*&&) /usr/lib/llvm-15/bin/../include/c++/v1/__functional/invoke.h:394:23 (bitcoind+0xaebe48) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#15](/bitcoin-bitcoin/15/) void std::__1::__thread_execute[abi:v15006]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(leveldb::(anonymous namespace)::PosixEnv*), leveldb::(anonymous namespace)::PosixEnv*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(leveldb::(anonymous namespace)::PosixEnv*), leveldb::(anonymous namespace)::PosixEnv*>&, std::__1::__tuple_indices<2ul>) /usr/lib/llvm-15/bin/../include/c++/v1/thread:284:5 (bitcoind+0xaebe48)
    [#16](/bitcoin-bitcoin/16/) void* std::__1::__thread_proxy[abi:v15006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(leveldb::(anonymous namespace)::PosixEnv*), leveldb::(anonymous namespace)::PosixEnv*>>(void*) /usr/lib/llvm-15/bin/../include/c++/v1/thread:295:5 (bitcoind+0xaebe48)
    [#17](/bitcoin-bitcoin/17/) __tsan_thread_start_func <null> (bitcoind+0xe10e6) (BuildId: 7f013e5c75bdabf8cb1b8e9769998ecddb89b677)
    [#18](/bitcoin-bitcoin/18/) <null> <null> (libc.so.6+0x88fd3) (BuildId: a30a1e23fbf2879030fcc0ef73ddf83658a19337)
    [#19](/bitcoin-bitcoin/19/) <null> <null> (libc.so.6+0x10966b) (BuildId: a30a1e23fbf2879030fcc0ef73ddf83658a19337)

maflcko commented at 8:39 AM on December 30, 2022: member

Might be best to revert everything and report the bug, given that it exists in clang-14 and -15.

hebasto commented at 8:41 AM on December 30, 2022: member

Might be best to revert everything and report the bug, given that it exists in clang-14 and -15.

Agree.

sidhujag referenced this in commit 7f425894c3 on Dec 30, 2022

bitcoin locked this on Dec 30, 2023