Skip to content

Commit bfda905

Browse files
committed
Prefer libc++ when running sanitizers
Ideally we would have matrix with both standard libraries, but it looks like Vagrind does not like implementation of the std::future in libstdc++, while libc++ passes without false positives
1 parent 4dbe09c commit bfda905

10 files changed

+89
-46
lines changed

.github/workflows/sanitizers.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
- name: Install dependencies
3232
run: |
3333
sudo apt-get update -y
34-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
34+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
3535
- uses: actions/checkout@v4
3636
with:
3737
submodules: recursive
@@ -69,7 +69,7 @@ jobs:
6969
- name: Install dependencies
7070
run: |
7171
sudo apt-get update -y
72-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
72+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
7373
- uses: actions/checkout@v4
7474
with:
7575
submodules: recursive
@@ -114,7 +114,7 @@ jobs:
114114
- name: Install dependencies
115115
run: |
116116
sudo apt-get update -y
117-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
117+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
118118
- uses: actions/checkout@v4
119119
with:
120120
submodules: recursive

bin/build-tests

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,38 +50,38 @@ echo "CB_CMAKE_BUILD_TYPE=${CB_CMAKE_BUILD_TYPE}"
5050
echo "CB_CACHE_OPTION=${CB_CACHE_OPTION}"
5151
echo "CB_TEST_SUITE=${CB_TEST_SUITE}"
5252
echo "CB_BORINGSSL=${CB_BORINGSSL}"
53+
echo "CB_SANITIZER=${CB_SANITIZER}"
54+
55+
CB_CMAKE_EXTRAS=()
56+
57+
if [ "x${CB_SANITIZER}" != "x" ]; then
58+
CB_CMAKE_EXTRAS+=(-DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_EXE_LINKER_FLAGS="-stdlib=libc++ -lc++abi")
59+
fi
5360

54-
CB_CMAKE_EXTRAS=
5561
case "${CB_SANITIZER}" in
5662
asan | address)
57-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_ADDRESS=ON"
63+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_ADDRESS=ON)
5864
;;
59-
6065
lsan | leak)
61-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_LEAK=ON"
66+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_LEAK=ON)
6267
;;
63-
6468
ubsan | undefined_behaviour)
65-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_UNDEFINED_BEHAVIOUR=ON"
69+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_UNDEFINED_BEHAVIOUR=ON)
6670
;;
67-
6871
tsan | thread)
69-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_THREAD=ON"
70-
export TSAN_OPTIONS="second_deadlock_stack=1"
72+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_THREAD=ON)
7173
;;
72-
7374
msan | memory)
74-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_MEMORY=ON"
75+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_MEMORY=ON)
7576
;;
76-
7777
valgrind)
78-
CB_CMAKE_EXTRAS="-DCOUCHBASE_CXX_CLIENT_ENABLE_VALGRIND=ON"
78+
CB_CMAKE_EXTRAS+=(-DCOUCHBASE_CXX_CLIENT_ENABLE_VALGRIND=ON)
7979
;;
8080
esac
8181

8282
CB_COLUMNAR=${CB_COLUMNAR:-""}
83-
if [ ! -z "$CB_COLUMNAR" ] ; then
84-
CB_CMAKE_EXTRAS="${CB_CMAKE_EXTRAS} -DCOUCHBASE_CXX_CLIENT_COLUMNAR=ON"
83+
if [ ! -z "${CB_COLUMNAR}" ]; then
84+
CB_CMAKE_EXTRAS+=(-DCOUCHBASE_CXX_CLIENT_COLUMNAR=ON)
8585
fi
8686
echo "CB_COLUMNAR=${CB_COLUMNAR}"
8787

@@ -101,6 +101,8 @@ if [ "x${CB_TEST_SUITE}" != "x" ] ; then
101101
CB_CMAKE_BUILD_EXTRAS="--target build_${CB_TEST_SUITE}_tests"
102102
fi
103103

104+
echo "CB_CMAKE_BUILD_EXTRAS=${CB_CMAKE_BUILD_EXTRAS}"
105+
104106
${CB_CMAKE} \
105107
-DCMAKE_BUILD_TYPE=${CB_CMAKE_BUILD_TYPE} \
106108
-DCMAKE_C_COMPILER="${CB_CC}" \
@@ -110,7 +112,7 @@ ${CB_CMAKE} \
110112
-DCOUCHBASE_CXX_CLIENT_BUILD_SHARED=ON \
111113
-DCOUCHBASE_CXX_CLIENT_STATIC_BORINGSSL=${CB_BORINGSSL} \
112114
-DCACHE_OPTION="${CB_CACHE_OPTION}" \
113-
${CB_CMAKE_EXTRAS} \
115+
"${CB_CMAKE_EXTRAS[@]}" \
114116
-B "${BUILD_DIR}" \
115117
-S "${PROJECT_ROOT}"
116118

bin/run-integration-tests

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
PROJECT_ROOT="$( cd "$(dirname "$0")/.." >/dev/null 2>&1 ; pwd -P )"
1818

19+
[ -f $PROJECT_ROOT/bin/sanitizer-options.sh ] && source $PROJECT_ROOT/bin/sanitizer-options.sh
20+
1921
echo "HOSTNAME=${HOSTNAME}"
2022
echo "NODE_NAME=${NODE_NAME}"
2123
echo "CONTAINER_TAG=${CONTAINER_TAG}"
@@ -40,6 +42,9 @@ if [ "x${CB_SANITIZER}" = "xvalgrind" ]; then
4042
export TEST_USE_WAN_DEVELOPMENT_PROFILE=yes
4143
fi
4244

45+
export ASAN_OPTIONS="$(IFS=:; echo "${asan_opts[*]}")"
46+
export TSAN_OPTIONS="$(IFS=:; echo "${tsan_opts[*]}")"
47+
4348
cd "${BUILD_DIR}"
4449

4550
CB_USE_GOCAVES=${CB_USE_GOCAVES:-""}

bin/run-transaction-tests

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616

1717
PROJECT_ROOT="$( cd "$(dirname "$0"/..)" >/dev/null 2>&1 ; pwd -P )"
1818

19+
[ -f $PROJECT_ROOT/bin/sanitizer-options.sh ] && source $PROJECT_ROOT/bin/sanitizer-options.sh
20+
1921
CB_CTEST=${CB_CTEST:-$(which ctest)}
22+
CB_CTEST_EXTRAS=${CB_CTEST_EXTRAS:-""}
2023
CB_SANITIZER=${CB_SANITIZER:-""}
2124

2225
echo "CB_CTEST=${CB_CTEST}"
@@ -27,6 +30,12 @@ BUILD_DIR="${PROJECT_ROOT}/cmake-build-tests"
2730
if [ "x${CB_SANITIZER}" != "x" ]; then
2831
BUILD_DIR="${BUILD_DIR}-${CB_SANITIZER}"
2932
fi
33+
if [ "x${CB_SANITIZER}" = "xvalgrind" ]; then
34+
CB_CTEST_EXTRAS="--test-action memcheck"
35+
fi
36+
37+
export ASAN_OPTIONS="$(IFS=:; echo "${asan_opts[*]}")"
38+
export TSAN_OPTIONS="$(IFS=:; echo "${tsan_opts[*]}")"
3039

3140
cd "${BUILD_DIR}"
3241

bin/run-unit-tests

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
PROJECT_ROOT="$( cd "$(dirname "$0"/..)" >/dev/null 2>&1 ; pwd -P )"
1818

19+
[ -f $PROJECT_ROOT/bin/sanitizer-options.sh ] && source $PROJECT_ROOT/bin/sanitizer-options.sh
20+
1921
CB_CTEST=${CB_CTEST:-$(which ctest)}
2022
CB_CTEST_EXTRAS=${CB_CTEST_EXTRAS:-""}
2123
CB_SANITIZER=${CB_SANITIZER:-""}
@@ -32,6 +34,9 @@ if [ "x${CB_SANITIZER}" = "xvalgrind" ]; then
3234
CB_CTEST_EXTRAS="--test-action memcheck"
3335
fi
3436

37+
export ASAN_OPTIONS="$(IFS=:; echo "${asan_opts[*]}")"
38+
export TSAN_OPTIONS="$(IFS=:; echo "${tsan_opts[*]}")"
39+
3540
cd "${BUILD_DIR}"
3641

3742
${CB_CTEST} --output-on-failure --label-regex 'unit' --output-junit results.xml

bin/sanitizer-options.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
asan_opts=(
2+
"print_cmdline=1"
3+
"print_stats=1"
4+
"use_odr_indicator=1"
5+
"dump_instruction_bytes=1"
6+
"check_initialization_order=1"
7+
"alloc_dealloc_mismatch=1"
8+
"detect_odr_violation=1"
9+
"detect_stack_use_after_return=1"
10+
)
11+
tsan_opts=(
12+
"print_cmdline=1"
13+
"detect_deadlocks=1"
14+
"second_deadlock_stack=1"
15+
"halt_on_error=1"
16+
)

core/io/http_session_manager.hxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public:
114114
void update_config(topology::configuration config) override
115115
{
116116
{
117-
std::scoped_lock config_lock(config_mutex_, sessions_mutex_, next_index_mutex_);
117+
std::scoped_lock config_lock(sessions_mutex_, config_mutex_, next_index_mutex_);
118118
config_ = std::move(config);
119119
if (!config_.nodes.empty() && next_index_ >= config_.nodes.size()) {
120120
next_index_ = 0;

core/io/mcbp_session.cxx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,16 +1751,20 @@ class mcbp_session_impl
17511751
}
17521752
}
17531753
state_ = diag::endpoint_state::connected;
1754-
const std::scoped_lock lock(pending_buffer_mutex_);
17551754
bootstrapped_ = true;
17561755
bootstrap_handler_->stop();
17571756
handler_ = std::make_shared<message_handler>(shared_from_this());
17581757
handler_->start();
1759-
if (!pending_buffer_.empty()) {
1760-
for (auto& buf : pending_buffer_) {
1758+
1759+
std::vector<std::vector<std::byte>> pending_buffer{};
1760+
{
1761+
const std::scoped_lock lock(pending_buffer_mutex_);
1762+
std::swap(pending_buffer, pending_buffer_);
1763+
}
1764+
if (!pending_buffer.empty()) {
1765+
for (auto& buf : pending_buffer) {
17611766
write(std::move(buf));
17621767
}
1763-
pending_buffer_.clear();
17641768
flush();
17651769
}
17661770
}

test/test_integration_connect.cxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,10 @@ TEST_CASE("integration: destroy cluster without waiting for close completion", "
156156
"SDK. FIXME");
157157
}
158158

159-
asio::io_context io{};
159+
asio::io_context io{ ASIO_CONCURRENCY_HINT_SAFE };
160160

161161
couchbase::core::cluster cluster(io);
162-
auto io_thread = std::thread([&io]() {
162+
auto io_thread = std::thread([&io]() -> void {
163163
io.run();
164164
});
165165

test/utils/integration_shortcuts.cxx

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ namespace test::utils
2727
void
2828
open_cluster(const couchbase::core::cluster& cluster, const couchbase::core::origin& origin)
2929
{
30-
auto barrier = std::make_shared<std::promise<std::error_code>>();
31-
auto f = barrier->get_future();
30+
std::promise<std::error_code> barrier;
31+
auto f = barrier.get_future();
3232
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
33-
cluster.open_in_background(origin, [barrier](std::error_code ec) mutable {
34-
barrier->set_value(ec);
33+
cluster.open_in_background(origin, [barrier = std::move(barrier)](std::error_code ec) mutable {
34+
barrier.set_value(ec);
3535
});
3636
#else
37-
cluster.open(origin, [barrier](std::error_code ec) mutable {
38-
barrier->set_value(ec);
37+
cluster.open(origin, [barrier = std::move(barrier)](std::error_code ec) mutable -> void {
38+
barrier.set_value(ec);
3939
});
4040
#endif
4141
auto rc = f.get();
@@ -49,22 +49,23 @@ open_cluster(const couchbase::core::cluster& cluster, const couchbase::core::ori
4949
void
5050
close_cluster(const couchbase::core::cluster& cluster)
5151
{
52-
auto barrier = std::make_shared<std::promise<void>>();
53-
auto f = barrier->get_future();
54-
cluster.close([barrier]() {
55-
barrier->set_value();
52+
std::promise<void> barrier;
53+
auto f = barrier.get_future();
54+
cluster.close([barrier = std::move(barrier)]() mutable -> void {
55+
barrier.set_value();
5656
});
5757
f.get();
5858
}
5959

6060
void
6161
open_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_name)
6262
{
63-
auto barrier = std::make_shared<std::promise<std::error_code>>();
64-
auto f = barrier->get_future();
65-
cluster.open_bucket(bucket_name, [barrier](std::error_code ec) mutable {
66-
barrier->set_value(ec);
67-
});
63+
std::promise<std::error_code> barrier;
64+
auto f = barrier.get_future();
65+
cluster.open_bucket(bucket_name,
66+
[barrier = std::move(barrier)](std::error_code ec) mutable -> void {
67+
barrier.set_value(ec);
68+
});
6869
auto rc = f.get();
6970
if (rc) {
7071
CB_LOG_CRITICAL("unable to open bucket: {}, name={}", rc.message(), bucket_name);
@@ -75,11 +76,12 @@ open_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_n
7576
void
7677
close_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_name)
7778
{
78-
auto barrier = std::make_shared<std::promise<std::error_code>>();
79-
auto f = barrier->get_future();
80-
cluster.close_bucket(bucket_name, [barrier](std::error_code ec) mutable {
81-
barrier->set_value(ec);
82-
});
79+
std::promise<std::error_code> barrier;
80+
auto f = barrier.get_future();
81+
cluster.close_bucket(bucket_name,
82+
[barrier = std::move(barrier)](std::error_code ec) mutable -> void {
83+
barrier.set_value(ec);
84+
});
8385
auto rc = f.get();
8486
if (rc) {
8587
CB_LOG_CRITICAL("unable to close bucket: {}, name={}", rc.message(), bucket_name);

0 commit comments

Comments
 (0)