From f965298154f38bd0f02778bee64932364847b4a3 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Mon, 24 Jul 2023 18:29:30 +0200 Subject: [PATCH 1/8] added reentrancy checking --- .../unordered/detail/foa/concurrent_table.hpp | 16 ++- .../unordered/detail/foa/reentrancy_check.hpp | 134 ++++++++++++++++++ test/Jamfile.v2 | 1 + test/cfoa/reentrancy_check_test.cpp | 68 +++++++++ 4 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 include/boost/unordered/detail/foa/reentrancy_check.hpp create mode 100644 test/cfoa/reentrancy_check_test.cpp diff --git a/include/boost/unordered/detail/foa/concurrent_table.hpp b/include/boost/unordered/detail/foa/concurrent_table.hpp index 0f1f1145..b79f70c6 100644 --- a/include/boost/unordered/detail/foa/concurrent_table.hpp +++ b/include/boost/unordered/detail/foa/concurrent_table.hpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -837,9 +838,10 @@ public: private: using mutex_type=rw_spinlock; using multimutex_type=multimutex; // TODO: adapt 128 to the machine - using shared_lock_guard=shared_lock; - using exclusive_lock_guard=lock_guard; - using exclusive_bilock_guard=scoped_bilock; + using shared_lock_guard=reentrancy_checked>; + using exclusive_lock_guard=reentrancy_checked>; + using exclusive_bilock_guard= + reentrancy_checked>>; using group_shared_lock_guard=typename group_access::shared_lock_guard; using group_exclusive_lock_guard=typename group_access::exclusive_lock_guard; using group_insert_counter_type=typename group_access::insert_counter_type; @@ -859,18 +861,18 @@ private: { thread_local auto id=(++thread_counter)%mutexes.size(); - return shared_lock_guard{mutexes[id]}; + return shared_lock_guard{this,mutexes[id]}; } inline exclusive_lock_guard exclusive_access()const { - return exclusive_lock_guard{mutexes}; + return exclusive_lock_guard{this,mutexes}; } static inline exclusive_bilock_guard exclusive_access( const concurrent_table& x,const concurrent_table& y) { - return {x.mutexes,y.mutexes}; + return {&x,&y,x.mutexes,y.mutexes}; } template @@ -878,7 +880,7 @@ private: const concurrent_table& x, const concurrent_table& y) { - return {x.mutexes,y.mutexes}; + return {&x,&y,x.mutexes,y.mutexes}; } /* Tag-dispatched shared/exclusive group access */ diff --git a/include/boost/unordered/detail/foa/reentrancy_check.hpp b/include/boost/unordered/detail/foa/reentrancy_check.hpp new file mode 100644 index 00000000..289d161c --- /dev/null +++ b/include/boost/unordered/detail/foa/reentrancy_check.hpp @@ -0,0 +1,134 @@ +/* Copyright 2023 Joaquin M Lopez Munoz. + * Distributed under the Boost Software License, Version 1.0. + * (See accompanying file LICENSE_1_0.txt or copy at + * http://www.boost.org/LICENSE_1_0.txt) + * + * See https://www.boost.org/libs/unordered for library home page. + */ + +#ifndef BOOST_UNORDERED_DETAIL_FOA_REENTRANCY_CHECK_HPP +#define BOOST_UNORDERED_DETAIL_FOA_REENTRANCY_CHECK_HPP + +#include +#include + +#if !defined(BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK) +#if defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER)|| \ + (defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_DEBUG_HANDLER)&& \ + !defined(NDEBUG))|| \ + !defined(BOOST_ASSERT_IS_VOID) +#define BOOST_UNORDERED_REENTRANCY_CHECK +#endif +#endif + +#if defined(BOOST_UNORDERED_REENTRANCY_CHECK) +#if defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER)|| \ + defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_DEBUG_HANDLER) + +#include /* BOOST_LIKELY */ + +namespace boost +{ + void boost_unordered_reentrancy_check_failed(); +} + +#define BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG(expr,msg) \ +(BOOST_LIKELY(!!(expr))?((void)0): \ + ::boost::boost_unordered_reentrancy_check_failed()) + +#else + +#define BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG(expr,msg) \ +BOOST_ASSERT_MSG(expr,msg) + +#endif +#endif + +namespace boost{ +namespace unordered{ +namespace detail{ +namespace foa{ + +#if defined(BOOST_UNORDERED_REENTRANCY_CHECK) + +class entry_trace +{ +public: + entry_trace(const void* px_):px{px_} + { + BOOST_ASSERT(px!=nullptr); + BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG( + !find(px),"reentrancy not allowed"); + header()=this; + } + + entry_trace(const entry_trace&)=delete; + entry_trace& operator=(const entry_trace&)=delete; + ~entry_trace(){clear();} + + void clear() + { + if(px){ + header()=next; + px=nullptr; + } + } + +private: + static entry_trace*& header() + { + thread_local entry_trace *pe=nullptr; + return pe; + } + + static bool find(const void* px) + { + for(auto pe=header();pe;pe=pe->next){ + if(pe->px==px)return true; + } + return false; + } + + const void *px; + entry_trace *next=header(); +}; + +template +struct reentrancy_checked +{ + template + reentrancy_checked(const void* px,Args&&... args): + tr{px},lck{std::forward(args)...}{} + + void unlock() + { + lck.unlock(); + tr.clear(); + } + + entry_trace tr; + LockGuard lck; +}; + +#else + +template +struct reentrancy_checked +{ + template + reentrancy_checked(const void*,Args&&... args): + lck{std::forward(args)...}{} + + void unlock(){lck.unlock();} + + LockGuard lck; +}; + +#endif + +} /* namespace foa */ +} /* namespace detail */ +} /* namespace unordered */ +} /* namespace boost */ + +#endif diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index e25f1083..dda83b68 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -202,6 +202,7 @@ local CFOA_TESTS = rw_spinlock_test6 rw_spinlock_test7 rw_spinlock_test8 + reentrancy_check_test ; for local test in $(CFOA_TESTS) diff --git a/test/cfoa/reentrancy_check_test.cpp b/test/cfoa/reentrancy_check_test.cpp new file mode 100644 index 00000000..09377459 --- /dev/null +++ b/test/cfoa/reentrancy_check_test.cpp @@ -0,0 +1,68 @@ +// Copyright 2023 Joaquin M Lopez Munoz +// Distributed under the Boost Software License, Version 1.0. +// https://www.boost.org/LICENSE_1_0.txt + +#define BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER + +static bool reentrancy_detected = false; + +namespace boost { + // Caveat lector: a proper handler should terminate as it may be executed + // within a noexcept function. + + void boost_unordered_reentrancy_check_failed() + { + reentrancy_detected = true; + throw 0; + } +} + +#include +#include + +template +void detect_reentrancy(F f) +{ + reentrancy_detected = false; + try { + f(); + } + catch(int) {} + BOOST_TEST(reentrancy_detected); +} + +int main() +{ + using map = boost::concurrent_flat_map; + using value_type = typename map::value_type; + + map m1, m2; + m1.emplace(0, 0); + m2.emplace(1, 0); + + detect_reentrancy([&] { + m1.visit_all([&](value_type&) { (void)m1.contains(0); }); + }); + + detect_reentrancy([&] { + m1.visit_all([&](value_type&) { m1.rehash(0); }); + }); + + detect_reentrancy([&] { + m1.visit_all([&](value_type&) { + m2.visit_all([&](value_type&) { + m1=m2; + }); + }); + }); + + detect_reentrancy([&] { + m1.visit_all([&](value_type&) { + m2.visit_all([&](value_type&) { + m2=m1; + }); + }); + }); + + return boost::report_errors(); +} From 948151bd7d1caf6de447313637e851920916c1d4 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Mon, 24 Jul 2023 18:43:20 +0200 Subject: [PATCH 2/8] added RVO enabler --- include/boost/unordered/detail/foa/reentrancy_check.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/boost/unordered/detail/foa/reentrancy_check.hpp b/include/boost/unordered/detail/foa/reentrancy_check.hpp index 289d161c..6db39f1a 100644 --- a/include/boost/unordered/detail/foa/reentrancy_check.hpp +++ b/include/boost/unordered/detail/foa/reentrancy_check.hpp @@ -62,8 +62,9 @@ public: header()=this; } - entry_trace(const entry_trace&)=delete; - entry_trace& operator=(const entry_trace&)=delete; + /* not used but VS in pre-C++17 mode needs to see it for RVO */ + entry_trace(const entry_trace&); + ~entry_trace(){clear();} void clear() From a3626b5095ce8bc717dce96bca0736f6d8a9ef92 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Mon, 24 Jul 2023 19:44:09 +0200 Subject: [PATCH 3/8] fixed reentrancy checking for scoped_bilock --- .../unordered/detail/foa/concurrent_table.hpp | 3 +- .../unordered/detail/foa/reentrancy_check.hpp | 39 +++++++++++++++++-- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/include/boost/unordered/detail/foa/concurrent_table.hpp b/include/boost/unordered/detail/foa/concurrent_table.hpp index b79f70c6..7fc397cc 100644 --- a/include/boost/unordered/detail/foa/concurrent_table.hpp +++ b/include/boost/unordered/detail/foa/concurrent_table.hpp @@ -840,8 +840,7 @@ private: using multimutex_type=multimutex; // TODO: adapt 128 to the machine using shared_lock_guard=reentrancy_checked>; using exclusive_lock_guard=reentrancy_checked>; - using exclusive_bilock_guard= - reentrancy_checked>>; + using exclusive_bilock_guard=reentrancy_bichecked>; using group_shared_lock_guard=typename group_access::shared_lock_guard; using group_exclusive_lock_guard=typename group_access::exclusive_lock_guard; using group_insert_counter_type=typename group_access::insert_counter_type; diff --git a/include/boost/unordered/detail/foa/reentrancy_check.hpp b/include/boost/unordered/detail/foa/reentrancy_check.hpp index 6db39f1a..b0e65c64 100644 --- a/include/boost/unordered/detail/foa/reentrancy_check.hpp +++ b/include/boost/unordered/detail/foa/reentrancy_check.hpp @@ -56,10 +56,11 @@ class entry_trace public: entry_trace(const void* px_):px{px_} { - BOOST_ASSERT(px!=nullptr); - BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG( - !find(px),"reentrancy not allowed"); - header()=this; + if(px){ + BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG( + !find(px),"reentrancy not allowed"); + header()=this; + } } /* not used but VS in pre-C++17 mode needs to see it for RVO */ @@ -111,6 +112,24 @@ struct reentrancy_checked LockGuard lck; }; +template +struct reentrancy_bichecked +{ + template + reentrancy_bichecked(const void* px,const void* py,Args&&... args): + tr1{px},tr2{py!=px?py:nullptr},lck{std::forward(args)...}{} + + void unlock() + { + lck.unlock(); + tr2.clear(); + tr1.clear(); + } + + entry_trace tr1,tr2; + LockGuard lck; +}; + #else template @@ -125,6 +144,18 @@ struct reentrancy_checked LockGuard lck; }; +template +struct reentrancy_bichecked +{ + template + reentrancy_bichecked(const void*,const void*,Args&&... args): + lck{std::forward(args)...}{} + + void unlock(){lck.unlock();} + + LockGuard lck; +}; + #endif } /* namespace foa */ From fde765c4941a3020788c10494483bbe01a45d049 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Mon, 24 Jul 2023 20:31:32 +0200 Subject: [PATCH 4/8] added reentrancy check to release notes --- doc/unordered/changes.adoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/unordered/changes.adoc b/doc/unordered/changes.adoc index 5679a9ea..876aeadf 100644 --- a/doc/unordered/changes.adoc +++ b/doc/unordered/changes.adoc @@ -6,6 +6,11 @@ :github-pr-url: https://github.com/boostorg/unordered/pull :cpp: C++ +== Release 1.84.0 + +* Added debug mode mechanisms for detecting illegal reentrancies into +a `boost::concurrent_flat_map` from user code. + == Release 1.83.0 - Major update * Added `boost::concurrent_flat_map`, a fast, thread-safe hashmap based on open addressing. From dd30162c9ee51f40b6b0a20f4cd0e4e97c403461 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Tue, 25 Jul 2023 09:18:53 +0200 Subject: [PATCH 5/8] simplified reentrancy check config --- .../unordered/detail/foa/reentrancy_check.hpp | 32 ++----------------- test/cfoa/reentrancy_check_test.cpp | 23 +++++++++---- 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/include/boost/unordered/detail/foa/reentrancy_check.hpp b/include/boost/unordered/detail/foa/reentrancy_check.hpp index b0e65c64..2855056a 100644 --- a/include/boost/unordered/detail/foa/reentrancy_check.hpp +++ b/include/boost/unordered/detail/foa/reentrancy_check.hpp @@ -12,37 +12,10 @@ #include #include -#if !defined(BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK) -#if defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER)|| \ - (defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_DEBUG_HANDLER)&& \ - !defined(NDEBUG))|| \ +#if !defined(BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK)&& \ !defined(BOOST_ASSERT_IS_VOID) #define BOOST_UNORDERED_REENTRANCY_CHECK #endif -#endif - -#if defined(BOOST_UNORDERED_REENTRANCY_CHECK) -#if defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER)|| \ - defined(BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_DEBUG_HANDLER) - -#include /* BOOST_LIKELY */ - -namespace boost -{ - void boost_unordered_reentrancy_check_failed(); -} - -#define BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG(expr,msg) \ -(BOOST_LIKELY(!!(expr))?((void)0): \ - ::boost::boost_unordered_reentrancy_check_failed()) - -#else - -#define BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG(expr,msg) \ -BOOST_ASSERT_MSG(expr,msg) - -#endif -#endif namespace boost{ namespace unordered{ @@ -57,8 +30,7 @@ public: entry_trace(const void* px_):px{px_} { if(px){ - BOOST_UNORDERED_REENTRANCY_CHECK_ASSERT_MSG( - !find(px),"reentrancy not allowed"); + BOOST_ASSERT_MSG(!find(px),"reentrancy not allowed"); header()=this; } } diff --git a/test/cfoa/reentrancy_check_test.cpp b/test/cfoa/reentrancy_check_test.cpp index 09377459..d07c6cdf 100644 --- a/test/cfoa/reentrancy_check_test.cpp +++ b/test/cfoa/reentrancy_check_test.cpp @@ -2,19 +2,28 @@ // Distributed under the Boost Software License, Version 1.0. // https://www.boost.org/LICENSE_1_0.txt -#define BOOST_UNORDERED_ENABLE_REENTRANCY_CHECK_HANDLER +#include + +#define BOOST_ENABLE_ASSERT_HANDLER static bool reentrancy_detected = false; namespace boost { - // Caveat lector: a proper handler should terminate as it may be executed + // Caveat lector: a proper handler shouldn't throw as it may be executed // within a noexcept function. - void boost_unordered_reentrancy_check_failed() - { - reentrancy_detected = true; - throw 0; - } +void assertion_failed_msg( + char const*, char const*, char const*, char const*, long) +{ + reentrancy_detected = true; + throw 0; +} + +void assertion_failed(char const*, char const*, char const*, long) +{ + std::abort(); +} + } #include From 30187f774354d6cc0a12fcc5fe7c78bd18cebfc5 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Wed, 26 Jul 2023 12:47:31 +0200 Subject: [PATCH 6/8] excluded code from coverage analysis --- test/cfoa/reentrancy_check_test.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/cfoa/reentrancy_check_test.cpp b/test/cfoa/reentrancy_check_test.cpp index d07c6cdf..b359b983 100644 --- a/test/cfoa/reentrancy_check_test.cpp +++ b/test/cfoa/reentrancy_check_test.cpp @@ -19,10 +19,10 @@ void assertion_failed_msg( throw 0; } -void assertion_failed(char const*, char const*, char const*, long) +void assertion_failed(char const*, char const*, char const*, long) // LCOV_EXCL_START { std::abort(); -} +} // LCOV_EXCL_STOP } @@ -51,27 +51,27 @@ int main() detect_reentrancy([&] { m1.visit_all([&](value_type&) { (void)m1.contains(0); }); - }); + }); // LCOV_EXCL_LINE detect_reentrancy([&] { m1.visit_all([&](value_type&) { m1.rehash(0); }); - }); + }); // LCOV_EXCL_LINE detect_reentrancy([&] { m1.visit_all([&](value_type&) { m2.visit_all([&](value_type&) { m1=m2; - }); + }); // LCOV_EXCL_START }); - }); + }); // LCOV_EXCL_STOP detect_reentrancy([&] { m1.visit_all([&](value_type&) { m2.visit_all([&](value_type&) { m2=m1; - }); + }); // LCOV_EXCL_START }); - }); + }); // LCOV_EXCL_STOP return boost::report_errors(); } From f919ce532a0d41403cb05375aace5f9777b722d0 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Wed, 26 Jul 2023 16:50:25 +0200 Subject: [PATCH 7/8] repositioned LCOV annotations --- test/cfoa/reentrancy_check_test.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/cfoa/reentrancy_check_test.cpp b/test/cfoa/reentrancy_check_test.cpp index b359b983..69ef5efc 100644 --- a/test/cfoa/reentrancy_check_test.cpp +++ b/test/cfoa/reentrancy_check_test.cpp @@ -63,7 +63,8 @@ int main() m1=m2; }); // LCOV_EXCL_START }); - }); // LCOV_EXCL_STOP + }); + // LCOV_EXCL_STOP detect_reentrancy([&] { m1.visit_all([&](value_type&) { @@ -71,7 +72,8 @@ int main() m2=m1; }); // LCOV_EXCL_START }); - }); // LCOV_EXCL_STOP + }); + // LCOV_EXCL_STOP return boost::report_errors(); } From 1979ce98a2730bb5b5fc87dd4774a8cb144600e3 Mon Sep 17 00:00:00 2001 From: joaquintides Date: Fri, 28 Jul 2023 18:22:20 +0200 Subject: [PATCH 8/8] documented BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK --- doc/unordered/concurrent_flat_map.adoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/unordered/concurrent_flat_map.adoc b/doc/unordered/concurrent_flat_map.adoc index fcdc8662..3f669e1b 100644 --- a/doc/unordered/concurrent_flat_map.adoc +++ b/doc/unordered/concurrent_flat_map.adoc @@ -364,6 +364,18 @@ if concurrent outstanding operations on `y` do not access `x` directly or indire --- +=== Configuration Macros + +==== `BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK` + +In debug builds (more precisely, when +link:../../../assert/doc/html/assert.html#boost_assert_is_void[`BOOST_ASSERT_IS_VOID`^] +is not defined), __container reentrancies__ (illegaly invoking an operation on `m` from within +a function visiting elements of `m`) are detected and signalled through `BOOST_ASSERT_MSG`. +When run-time speed is a concern, the feature can be disabled by globally defining +this macro. + + === Constructors ==== Default Constructor