Skip to content

ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN (libc++) job #30519

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 25, 2024
Merged

ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN (libc++) job #30519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e78616
refactor: fix missing includes
fanquake Jul 18, 2024
e3edacc
ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN job
fanquake Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_native_tsan.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
export PACKAGES="clang-18 llvm-18 libclang-rt-18-dev libc++abi-18-dev libc++-18-dev python3-zmq"
export DEP_OPTS="CC=clang-18 CXX='clang++-18 -stdlib=libc++'"
export GOAL="install"
export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' --with-sanitizers=thread"
export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES' --with-sanitizers=thread"
Copy link
Member

Choose a reason for hiding this comment

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

nit: The same can be achieved, by setting C++23, according to the docs.

This may be beneficial for other reasons as well, even if it is just turning existing compile warnings into errors (#30436 (comment))

So adding -std=c++23 here, and a comment that libc++ is required for the feature seems better. But this is fine either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, had a look at this, and ran into issues with at least leveldb, which we'd first have to fix in the subtree, i.e:

In file included from leveldb/util/comparator.cc:14:
./leveldb/util/no_destructor.h:40:17: error: 'aligned_storage<8, 8>' is deprecated [-Werror,-Wdeprecated-declarations]
   40 |   typename std::aligned_storage<sizeof(InstanceType),
      |                 ^
leveldb/util/comparator.cc:71:47: note: in instantiation of template class 'leveldb::NoDestructor<leveldb::(anonymous namespace)::BytewiseComparatorImpl>' requested here
   71 |   static NoDestructor<BytewiseComparatorImpl> singleton;
      |                                               ^
/usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/aligned_storage.h:90:8: note: 'aligned_storage<8, 8>' has been explicitly marked deprecated here
   90 | struct _LIBCPP_DEPRECATED_IN_CXX23 _LIBCPP_TEMPLATE_VIS aligned_storage {
      |        ^
/usr/lib/llvm-18/bin/../include/c++/v1/__config:1013:41: note: expanded from macro '_LIBCPP_DEPRECATED_IN_CXX23'
 1013 | #    define _LIBCPP_DEPRECATED_IN_CXX23 _LIBCPP_DEPRECATED
      |                                         ^
/usr/lib/llvm-18/bin/../include/c++/v1/__config:974:49: note: expanded from macro '_LIBCPP_DEPRECATED'
  974 | #      define _LIBCPP_DEPRECATED __attribute__((__deprecated__))
      |                                                 ^
1 error generated.

So might followup with this.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is an up-upstream issue / PR for this. google/leveldb#1013 & google/leveldb#896.

5 changes: 3 additions & 2 deletions src/crypto/sha256.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
#include <crypto/sha256.h>
#include <crypto/common.h>

#include <assert.h>
#include <string.h>
#include <algorithm>
#include <cassert>
#include <cstring>

#if !defined(DISABLE_OPTIMIZED_SHA256)
#include <compat/cpuid.h>
Expand Down
4 changes: 1 addition & 3 deletions src/init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#ifndef BITCOIN_INIT_H
#define BITCOIN_INIT_H

#include <any>
#include <memory>
#include <string>
#include <atomic>

//! Default value for -daemon option
static constexpr bool DEFAULT_DAEMON = false;
Expand Down