[PATCH] D36767: Implement the standard for timeouts for std::condition_variable
tomcherry abandoned this revision. tomcherry added a comment. Herald added a subscriber: jfb. See https://reviews.llvm.org/D65339 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36767/new/ https://reviews.llvm.org/D36767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36767: Implement the standard for timeouts for std::condition_variable
tomcherry created this revision. The C++ standard calls for wait_for() to use steady clock and wait_until() to use the clock that is provided as an argument. This is not currently done in libc++ and is not possible with the pthreads API, however it is possible with the underlying futex system call. This change re-implements std::condition_variable with a tweaked version of the implementation of pthread_cond_* from Android bionic to support the correct semantics. Bug: 35756266 https://reviews.llvm.org/D36767 Files: include/__mutex_base src/condition_variable.cpp Index: src/condition_variable.cpp === --- src/condition_variable.cpp +++ src/condition_variable.cpp @@ -11,6 +11,10 @@ #ifndef _LIBCPP_HAS_NO_THREADS +#include +#include +#include + #include "condition_variable" #include "thread" #include "system_error" @@ -20,60 +24,95 @@ condition_variable::~condition_variable() { -__libcpp_condvar_destroy(&__cv_); +atomic_store_explicit(&__state, 0xdeadc04d, memory_order_relaxed); +} + +void +condition_variable::__pulse(int thread_count) _NOEXCEPT +{ +_LIBCPP_CONSTEXPR unsigned int kCondCounterStep = 0x0004; +atomic_fetch_add_explicit(&__state, kCondCounterStep, memory_order_relaxed); +__futex(&__state, FUTEX_WAKE_PRIVATE, thread_count, nullptr, 0); } void condition_variable::notify_one() _NOEXCEPT { -__libcpp_condvar_signal(&__cv_); +__pulse(1); } void condition_variable::notify_all() _NOEXCEPT { -__libcpp_condvar_broadcast(&__cv_); +__pulse(numeric_limits::max()); } void condition_variable::wait(unique_lock& lk) _NOEXCEPT { -if (!lk.owns_lock()) -__throw_system_error(EPERM, - "condition_variable::wait: mutex not locked"); -int ec = __libcpp_condvar_wait(&__cv_, lk.mutex()->native_handle()); -if (ec) -__throw_system_error(ec, "condition_variable wait failed"); +__wait(lk, chrono::nanoseconds(0), false, false); } -void -condition_variable::__do_timed_wait(unique_lock& lk, - chrono::time_point tp) _NOEXCEPT +int +condition_variable::__wait(unique_lock& __lk, chrono::nanoseconds __t, + bool __rel_timeout, bool __realtime) _NOEXCEPT { +int op; +int val3; + +if (__t.count() == 0 || __rel_timeout) { +op = FUTEX_WAIT_PRIVATE; +val3 = 0; +} else if (__realtime) { +op = FUTEX_WAIT_BITSET_PRIVATE | FUTEX_CLOCK_REALTIME; +val3 = FUTEX_BITSET_MATCH_ANY; +} else { +op = FUTEX_WAIT_BITSET_PRIVATE; +val3 = FUTEX_BITSET_MATCH_ANY; +} + using namespace chrono; -if (!lk.owns_lock()) -__throw_system_error(EPERM, -"condition_variable::timed wait: mutex not locked"); -nanoseconds d = tp.time_since_epoch(); -if (d > nanoseconds(0x59682F00E941)) -d = nanoseconds(0x59682F00E941); +if (__t > nanoseconds(0x59682F00E941)) +__t = nanoseconds(0x59682F00E941); timespec ts; -seconds s = duration_cast(d); -typedef decltype(ts.tv_sec) ts_sec; -_LIBCPP_CONSTEXPR ts_sec ts_sec_max = numeric_limits::max(); -if (s.count() < ts_sec_max) -{ -ts.tv_sec = static_cast(s.count()); -ts.tv_nsec = static_cast((d - s).count()); +timespec* pts = nullptr; + +if (__t.count() > 0) { +seconds s = duration_cast(__t); +typedef decltype(ts.tv_sec) ts_sec; +_LIBCPP_CONSTEXPR ts_sec ts_sec_max = numeric_limits::max(); +if (s.count() < ts_sec_max) +{ +ts.tv_sec = static_cast(s.count()); +ts.tv_nsec = static_cast((__t - s).count()); +} +else +{ +ts.tv_sec = ts_sec_max; +ts.tv_nsec = giga::num - 1; +} +pts = &ts; } -else -{ -ts.tv_sec = ts_sec_max; -ts.tv_nsec = giga::num - 1; + +unsigned int old_state = atomic_load_explicit(&__state, memory_order_relaxed); +__lk.unlock(); +int ec = __futex(&__state, op, old_state, pts, val3); +__lk.lock(); + +return ec; +} + +int condition_variable::__futex(volatile void* ftx, int op, int value, +const struct timespec* timeout, +int bitset) _NOEXCEPT { +// Our generated syscall assembler sets errno, but our callers don't want to. +int saved_errno = errno; +int result = syscall(__NR_futex, ftx, op, value, timeout, NULL, bitset); +if (result == -1) { +result = -errno; +errno = saved_errno; } -int ec = __libcpp_condvar_timedwait(&__cv_, lk.mutex()->native_handle(), &ts); -if (ec != 0 && ec != ETIMEDOUT) -__throw_system_error(ec, "condition_variable timed_wait failed"); +return result; } void Index: include/__mutex_base ===
[PATCH] D36767: Implement the standard for timeouts for std::condition_variable
tomcherry added a comment. This is mostly an RFC. The underlying issue is that we cannot use timed waits based on CLOCK_REALTIME on Android. In trying to fix that, I figured I might as well try implementing the standard as described, though I understand that there'd be resistance to such a re-implementation. https://reviews.llvm.org/D36767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30764: Disable unsigned integer sanitizer for basic_string::replace()
tomcherry updated this revision to Diff 9. tomcherry added a comment. Moved comments to an appropriate line https://reviews.llvm.org/D30764 Files: include/string Index: include/string === --- include/string +++ include/string @@ -2560,6 +2560,7 @@ template basic_string<_CharT, _Traits, _Allocator>& basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __n1, const value_type* __s, size_type __n2) +_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK { _LIBCPP_ASSERT(__n2 == 0 || __s != nullptr, "string::replace received nullptr"); size_type __sz = size(); @@ -2599,6 +2600,8 @@ } traits_type::move(__p + __pos, __s, __n2); __finish: +// __sz += __n2 - __n1; in this and the below function below can cause unsigned integer overflow, +// but this is a safe operation, so we disable the check. __sz += __n2 - __n1; __set_size(__sz); __invalidate_iterators_past(__sz); @@ -2612,6 +2615,7 @@ template basic_string<_CharT, _Traits, _Allocator>& basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __n1, size_type __n2, value_type __c) +_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK { size_type __sz = size(); if (__pos > __sz) Index: include/string === --- include/string +++ include/string @@ -2560,6 +2560,7 @@ template basic_string<_CharT, _Traits, _Allocator>& basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __n1, const value_type* __s, size_type __n2) +_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK { _LIBCPP_ASSERT(__n2 == 0 || __s != nullptr, "string::replace received nullptr"); size_type __sz = size(); @@ -2599,6 +2600,8 @@ } traits_type::move(__p + __pos, __s, __n2); __finish: +// __sz += __n2 - __n1; in this and the below function below can cause unsigned integer overflow, +// but this is a safe operation, so we disable the check. __sz += __n2 - __n1; __set_size(__sz); __invalidate_iterators_past(__sz); @@ -2612,6 +2615,7 @@ template basic_string<_CharT, _Traits, _Allocator>& basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __n1, size_type __n2, value_type __c) +_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK { size_type __sz = size(); if (__pos > __sz) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30764: Disable unsigned integer sanitizer for basic_string::replace()
tomcherry added a comment. I don't have commit access, so please commit for me. Thank you https://reviews.llvm.org/D30764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits