From a8b03d90e0afe03fefa16d4a871ece344a5d52ad Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 4 Jun 2020 14:05:55 -0700 Subject: [PATCH] Export of internal Abseil changes -- 5b9d5ce21074c0541432555d383d1b9c5898b553 by Gennadiy Rozental : Convert FlagSaver into public API PiperOrigin-RevId: 314799739 -- 5f796c9862b1177f161f4b10fe1a84698ebbb5cf by Abseil Team : Google-internal changes only. PiperOrigin-RevId: 314786474 -- 67f2ae0ac5ae73bcd9d57a058ac4fea8fc1243ba by Gennadiy Rozental : Makes sure stacktrace_generic is only used with glibc. Fixes #701 PiperOrigin-RevId: 314726944 -- efb1ef3636b0698b79d3ee3590f12c4dff32a3cb by Samuel Benzaquen : Take the bits into account when reserving the space for the operation, not just the exponent. PiperOrigin-RevId: 314622744 GitOrigin-RevId: 5b9d5ce21074c0541432555d383d1b9c5898b553 Change-Id: I080a5e333e2dc1545b5aa0417882f7ac7116a20c --- absl/base/internal/low_level_scheduling.h | 5 +++ absl/debugging/internal/stacktrace_config.h | 2 +- absl/flags/commandlineflag_test.cc | 4 +- absl/flags/flag_test.cc | 2 +- absl/flags/internal/registry.h | 25 ------------ absl/flags/internal/usage_test.cc | 2 +- absl/flags/parse_test.cc | 2 +- absl/flags/reflection.cc | 21 +++------- absl/flags/reflection.h | 38 +++++++++++++++++++ absl/flags/reflection_test.cc | 4 +- .../internal/str_format/convert_test.cc | 6 +++ .../internal/str_format/float_conversion.cc | 12 +++--- absl/synchronization/mutex.cc | 7 ++++ 13 files changed, 75 insertions(+), 55 deletions(-) diff --git a/absl/base/internal/low_level_scheduling.h b/absl/base/internal/low_level_scheduling.h index 961cc981b86..7c52d48c127 100644 --- a/absl/base/internal/low_level_scheduling.h +++ b/absl/base/internal/low_level_scheduling.h @@ -29,6 +29,9 @@ extern "C" void __google_enable_rescheduling(bool disable_result); namespace absl { ABSL_NAMESPACE_BEGIN +class CondVar; +class Mutex; + namespace base_internal { class SchedulingHelper; // To allow use of SchedulingGuard. @@ -77,6 +80,8 @@ class SchedulingGuard { }; // Access to SchedulingGuard is explicitly white-listed. + friend class absl::CondVar; + friend class absl::Mutex; friend class SchedulingHelper; friend class SpinLock; diff --git a/absl/debugging/internal/stacktrace_config.h b/absl/debugging/internal/stacktrace_config.h index 8caa97c0509..d5cc1740139 100644 --- a/absl/debugging/internal/stacktrace_config.h +++ b/absl/debugging/internal/stacktrace_config.h @@ -61,7 +61,7 @@ # elif defined(__aarch64__) #define ABSL_STACKTRACE_INL_HEADER \ "absl/debugging/internal/stacktrace_aarch64-inl.inc" -# elif defined(__arm__) +#elif defined(__arm__) && defined(__GLIBC__) // Note: When using glibc this may require -funwind-tables to function properly. #define ABSL_STACKTRACE_INL_HEADER \ "absl/debugging/internal/stacktrace_generic-inl.inc" diff --git a/absl/flags/commandlineflag_test.cc b/absl/flags/commandlineflag_test.cc index cc8eae91591..c5afff61ec9 100644 --- a/absl/flags/commandlineflag_test.cc +++ b/absl/flags/commandlineflag_test.cc @@ -47,7 +47,7 @@ class CommandLineFlagTest : public testing::Test { absl::SetFlagsUsageConfig(default_config); } - void SetUp() override { flag_saver_ = absl::make_unique(); } + void SetUp() override { flag_saver_ = absl::make_unique(); } void TearDown() override { flag_saver_.reset(); } private: @@ -60,7 +60,7 @@ class CommandLineFlagTest : public testing::Test { return std::string(fname); } - std::unique_ptr flag_saver_; + std::unique_ptr flag_saver_; }; TEST_F(CommandLineFlagTest, TestAttributesAccessMethods) { diff --git a/absl/flags/flag_test.cc b/absl/flags/flag_test.cc index 0c88e24ab56..2eb2ba71d37 100644 --- a/absl/flags/flag_test.cc +++ b/absl/flags/flag_test.cc @@ -81,7 +81,7 @@ class FlagTest : public testing::Test { #endif return std::string(fname); } - flags::FlagSaver flag_saver_; + absl::FlagSaver flag_saver_; }; struct S1 { diff --git a/absl/flags/internal/registry.h b/absl/flags/internal/registry.h index 1e655a3ec8a..c72eebe2fb5 100644 --- a/absl/flags/internal/registry.h +++ b/absl/flags/internal/registry.h @@ -89,31 +89,6 @@ inline bool RetiredFlag(const char* flag_name) { return flags_internal::Retire(flag_name, base_internal::FastTypeId()); } -//----------------------------------------------------------------------------- -// Saves the states (value, default value, whether the user has set -// the flag, registered validators, etc) of all flags, and restores -// them when the FlagSaver is destroyed. -// -// This class is thread-safe. However, its destructor writes to -// exactly the set of flags that have changed value during its -// lifetime, so concurrent _direct_ access to those flags -// (i.e. FLAGS_foo instead of {Get,Set}CommandLineOption()) is unsafe. - -class FlagSaver { - public: - FlagSaver(); - ~FlagSaver(); - - FlagSaver(const FlagSaver&) = delete; - void operator=(const FlagSaver&) = delete; - - // Prevents saver from restoring the saved state of flags. - void Ignore(); - - private: - class FlagSaverImpl* impl_; // we use pimpl here to keep API steady -}; - } // namespace flags_internal ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/flags/internal/usage_test.cc b/absl/flags/internal/usage_test.cc index 46f85b55e40..6e583fbe4b0 100644 --- a/absl/flags/internal/usage_test.cc +++ b/absl/flags/internal/usage_test.cc @@ -89,7 +89,7 @@ class UsageReportingTest : public testing::Test { } private: - flags::FlagSaver flag_saver_; + absl::FlagSaver flag_saver_; }; // -------------------------------------------------------------------- diff --git a/absl/flags/parse_test.cc b/absl/flags/parse_test.cc index 1ab8769066d..67ae8aad30f 100644 --- a/absl/flags/parse_test.cc +++ b/absl/flags/parse_test.cc @@ -208,7 +208,7 @@ using testing::ElementsAreArray; class ParseTest : public testing::Test { private: - flags::FlagSaver flag_saver_; + absl::FlagSaver flag_saver_; }; // -------------------------------------------------------------------- diff --git a/absl/flags/reflection.cc b/absl/flags/reflection.cc index 02bb6c2eaf8..5fc945f2376 100644 --- a/absl/flags/reflection.cc +++ b/absl/flags/reflection.cc @@ -65,7 +65,8 @@ class FlagRegistry { static FlagRegistry& GlobalRegistry(); // returns a singleton registry private: - friend class FlagSaverImpl; // reads all the flags in order to copy them + friend class flags_internal::FlagSaverImpl; // reads all the flags in order + // to copy them friend void ForEachFlagUnlocked( std::function visitor); @@ -249,15 +250,6 @@ bool Retire(const char* name, FlagFastTypeId type_id) { // -------------------------------------------------------------------- -// FlagSaver -// FlagSaverImpl -// This class stores the states of all flags at construct time, -// and restores all flags to that state at destruct time. -// Its major implementation challenge is that it never modifies -// pointers in the 'main' registry, so global FLAG_* vars always -// point to the right place. -// -------------------------------------------------------------------- - class FlagSaverImpl { public: FlagSaverImpl() = default; @@ -288,11 +280,10 @@ class FlagSaverImpl { backup_registry_; }; -FlagSaver::FlagSaver() : impl_(new FlagSaverImpl) { impl_->SaveFromRegistry(); } +} // namespace flags_internal -void FlagSaver::Ignore() { - delete impl_; - impl_ = nullptr; +FlagSaver::FlagSaver() : impl_(new flags_internal::FlagSaverImpl) { + impl_->SaveFromRegistry(); } FlagSaver::~FlagSaver() { @@ -304,8 +295,6 @@ FlagSaver::~FlagSaver() { // -------------------------------------------------------------------- -} // namespace flags_internal - CommandLineFlag* FindCommandLineFlag(absl::string_view name) { if (name.empty()) return nullptr; flags_internal::FlagRegistry& registry = diff --git a/absl/flags/reflection.h b/absl/flags/reflection.h index e8e24f68f9f..045f9784e26 100644 --- a/absl/flags/reflection.h +++ b/absl/flags/reflection.h @@ -31,6 +31,9 @@ namespace absl { ABSL_NAMESPACE_BEGIN +namespace flags_internal { +class FlagSaverImpl; +} // namespace flags_internal // FindCommandLineFlag() // @@ -39,6 +42,41 @@ ABSL_NAMESPACE_BEGIN // 'retired' flag is specified. CommandLineFlag* FindCommandLineFlag(absl::string_view name); +//------------------------------------------------------------------------------ +// FlagSaver +//------------------------------------------------------------------------------ +// +// A FlagSaver object stores the state of flags in the scope where the FlagSaver +// is defined, allowing modification of those flags within that scope and +// automatic restoration of the flags to their previous state upon leaving the +// scope. +// +// A FlagSaver can be used within tests to temporarily change the test +// environment and restore the test case to its previous state. +// +// Example: +// +// void MyFunc() { +// absl::FlagSaver fs; +// ... +// absl::SetFlag(FLAGS_myFlag, otherValue); +// ... +// } // scope of FlagSaver left, flags return to previous state +// +// This class is thread-safe. + +class FlagSaver { + public: + FlagSaver(); + ~FlagSaver(); + + FlagSaver(const FlagSaver&) = delete; + void operator=(const FlagSaver&) = delete; + + private: + flags_internal::FlagSaverImpl* impl_; +}; + //----------------------------------------------------------------------------- ABSL_NAMESPACE_END diff --git a/absl/flags/reflection_test.cc b/absl/flags/reflection_test.cc index 2a137bf716c..9781e597dbd 100644 --- a/absl/flags/reflection_test.cc +++ b/absl/flags/reflection_test.cc @@ -34,11 +34,11 @@ namespace flags = absl::flags_internal; class ReflectionTest : public testing::Test { protected: - void SetUp() override { flag_saver_ = absl::make_unique(); } + void SetUp() override { flag_saver_ = absl::make_unique(); } void TearDown() override { flag_saver_.reset(); } private: - std::unique_ptr flag_saver_; + std::unique_ptr flag_saver_; }; // -------------------------------------------------------------------- diff --git a/absl/strings/internal/str_format/convert_test.cc b/absl/strings/internal/str_format/convert_test.cc index 20c6229fcb3..0e8535c27b7 100644 --- a/absl/strings/internal/str_format/convert_test.cc +++ b/absl/strings/internal/str_format/convert_test.cc @@ -764,6 +764,12 @@ TEST_F(FormatConvertTest, LongDouble) { } } + // Regression tests + // + // Using a string literal because not all platforms support hex literals or it + // might be out of range. + doubles.push_back(std::strtold("-0xf.ffffffb5feafffbp-16324L", nullptr)); + for (const char *fmt : kFormats) { for (char f : {'f', 'F', // 'g', 'G', // diff --git a/absl/strings/internal/str_format/float_conversion.cc b/absl/strings/internal/str_format/float_conversion.cc index a761a5a5f9c..10e4695411b 100644 --- a/absl/strings/internal/str_format/float_conversion.cc +++ b/absl/strings/internal/str_format/float_conversion.cc @@ -224,13 +224,13 @@ class FractionalDigitGenerator { // This function will allocate enough stack space to perform the conversion. static void RunConversion( uint128 v, int exp, absl::FunctionRef f) { + using Limits = std::numeric_limits; assert(-exp < 0); - assert(-exp >= std::numeric_limits::min_exponent - 128); - static_assert( - StackArray::kMaxCapacity >= - (128 - std::numeric_limits::min_exponent + 31) / 32, - ""); - StackArray::RunWithCapacity((exp + 31) / 32, + assert(-exp >= Limits::min_exponent - 128); + static_assert(StackArray::kMaxCapacity >= + (Limits::digits + 128 - Limits::min_exponent + 31) / 32, + ""); + StackArray::RunWithCapacity((Limits::digits + exp + 31) / 32, [=](absl::Span input) { f(FractionalDigitGenerator(input, v, exp)); }); diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 1f8a696e08e..36c93d5f5ba 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -58,6 +58,7 @@ using absl::base_internal::CurrentThreadIdentityIfPresent; using absl::base_internal::PerThreadSynch; +using absl::base_internal::SchedulingGuard; using absl::base_internal::ThreadIdentity; using absl::synchronization_internal::GetOrCreateCurrentThreadIdentity; using absl::synchronization_internal::GraphCycles; @@ -1108,6 +1109,7 @@ void Mutex::TryRemove(PerThreadSynch *s) { // on the mutex queue. In this case, remove "s" from the queue and return // true, otherwise return false. ABSL_XRAY_LOG_ARGS(1) void Mutex::Block(PerThreadSynch *s) { + SchedulingGuard::ScopedDisable disable_rescheduling; while (s->state.load(std::memory_order_acquire) == PerThreadSynch::kQueued) { if (!DecrementSynchSem(this, s, s->waitp->timeout)) { // After a timeout, we go into a spin loop until we remove ourselves @@ -1897,6 +1899,7 @@ static void CheckForMutexCorruption(intptr_t v, const char* label) { } void Mutex::LockSlowLoop(SynchWaitParams *waitp, int flags) { + SchedulingGuard::ScopedDisable disable_rescheduling; int c = 0; intptr_t v = mu_.load(std::memory_order_relaxed); if ((v & kMuEvent) != 0) { @@ -2016,6 +2019,7 @@ void Mutex::LockSlowLoop(SynchWaitParams *waitp, int flags) { // or it is in the process of blocking on a condition variable; it must requeue // itself on the mutex/condvar to wait for its condition to become true. ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams *waitp) { + SchedulingGuard::ScopedDisable disable_rescheduling; intptr_t v = mu_.load(std::memory_order_relaxed); this->AssertReaderHeld(); CheckForMutexCorruption(v, "Unlock"); @@ -2331,6 +2335,7 @@ void Mutex::Trans(MuHow how) { // It will later acquire the mutex with high probability. Otherwise, we // enqueue thread w on this mutex. void Mutex::Fer(PerThreadSynch *w) { + SchedulingGuard::ScopedDisable disable_rescheduling; int c = 0; ABSL_RAW_CHECK(w->waitp->cond == nullptr, "Mutex::Fer while waiting on Condition"); @@ -2429,6 +2434,7 @@ CondVar::~CondVar() { // Remove thread s from the list of waiters on this condition variable. void CondVar::Remove(PerThreadSynch *s) { + SchedulingGuard::ScopedDisable disable_rescheduling; intptr_t v; int c = 0; for (v = cv_.load(std::memory_order_relaxed);; @@ -2589,6 +2595,7 @@ void CondVar::Wakeup(PerThreadSynch *w) { } void CondVar::Signal() { + SchedulingGuard::ScopedDisable disable_rescheduling; ABSL_TSAN_MUTEX_PRE_SIGNAL(nullptr, 0); intptr_t v; int c = 0;