From 02f0ab209314000bc1c8fdff0d60378995316f9c Mon Sep 17 00:00:00 2001 From: Randolf J <34705014+jun-sheaf@users.noreply.github.com> Date: Fri, 7 Oct 2022 13:02:41 +0200 Subject: Update charconv.cc --- absl/strings/charconv.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/absl/strings/charconv.cc b/absl/strings/charconv.cc index c08623c4..25ac4499 100644 --- a/absl/strings/charconv.cc +++ b/absl/strings/charconv.cc @@ -347,7 +347,8 @@ bool HandleEdgeCase(const strings_internal::ParsedFloat& input, bool negative, // https://bugs.llvm.org/show_bug.cgi?id=37778 // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86113 constexpr ptrdiff_t kNanBufferSize = 128; -#if defined(__GNUC__) || (defined(__clang__) && __clang_major__ < 7) +#if (defined(__GNUC__) && !defined(__clang__)) || \ + (defined(__clang__) && __clang_major__ < 7) volatile char n_char_sequence[kNanBufferSize]; #else char n_char_sequence[kNanBufferSize]; -- cgit v1.2.3 From fd3ac7427a85a51dd8f24e3c95a93eb6e4a870bd Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati Date: Wed, 25 Jan 2023 15:19:20 -0800 Subject: Add mingw as a config_setting --- absl/BUILD.bazel | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/absl/BUILD.bazel b/absl/BUILD.bazel index 29963ccc..69bdbf14 100644 --- a/absl/BUILD.bazel +++ b/absl/BUILD.bazel @@ -35,6 +35,14 @@ config_setting( visibility = [":__subpackages__"], ) +config_setting( + name = "mingw_compiler", + flag_values = { + "@bazel_tools//tools/cpp:compiler": "mingw", + }, + visibility = [":__subpackages__"], +) + config_setting( name = "msvc_compiler", flag_values = { -- cgit v1.2.3 From 3a0f99e84bbc3c1de08f8412ad158020eff8b213 Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati Date: Wed, 25 Jan 2023 15:22:05 -0800 Subject: Pass correct flags for mingw to use bcrypt --- absl/random/internal/BUILD.bazel | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/absl/random/internal/BUILD.bazel b/absl/random/internal/BUILD.bazel index 81ca669b..a51c9375 100644 --- a/absl/random/internal/BUILD.bazel +++ b/absl/random/internal/BUILD.bazel @@ -82,6 +82,10 @@ cc_library( linkopts = ABSL_DEFAULT_LINKOPTS + select({ "//absl:msvc_compiler": ["-DEFAULTLIB:bcrypt.lib"], "//absl:clang-cl_compiler": ["-DEFAULTLIB:bcrypt.lib"], + "//absl:mingw_compiler": [ + "-DEFAULTLIB:bcrypt.lib", + "-lbcrypt", + ], "//conditions:default": [], }), deps = [ -- cgit v1.2.3 From 35e8e3f7a2c6972d4c591448e8bbe4f9ed9f815a Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Wed, 25 Jan 2023 19:57:34 -0800 Subject: Make `SanitizerSafeCopy()` constexpr, and check for constant evaluation PiperOrigin-RevId: 504728034 Change-Id: Ifb338247b7484426e25a58580783a1d70d27e6fd --- absl/strings/internal/cord_internal.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/absl/strings/internal/cord_internal.h b/absl/strings/internal/cord_internal.h index 63a81f4f..e6f0d544 100644 --- a/absl/strings/internal/cord_internal.h +++ b/absl/strings/internal/cord_internal.h @@ -768,18 +768,22 @@ class InlineData { } #ifdef ABSL_INTERNAL_CORD_HAVE_SANITIZER - Rep SanitizerSafeCopy() const { - Rep res; - if (is_tree()) { - res = *this; + constexpr Rep SanitizerSafeCopy() const { + if (!absl::is_constant_evaluated()) { + Rep res; + if (is_tree()) { + res = *this; + } else { + res.set_tag(tag()); + memcpy(res.as_chars(), as_chars(), inline_size()); + } + return res; } else { - res.set_tag(tag()); - memcpy(res.as_chars(), as_chars(), inline_size()); + return *this; } - return res; } #else - const Rep& SanitizerSafeCopy() const { return *this; } + constexpr const Rep& SanitizerSafeCopy() const { return *this; } #endif // If the data has length <= kMaxInline, we store it in `data`, and -- cgit v1.2.3 From b4705164197f8b10ef4c50f517e9434ada7607f8 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Thu, 26 Jan 2023 08:34:37 -0800 Subject: Cord does not need its str_format dependency Fixes #1360 PiperOrigin-RevId: 504850733 Change-Id: Ifa1e83d0c703ed69c969a12076db474caec9d496 --- absl/strings/BUILD.bazel | 1 - absl/strings/cord.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 53c57718..c6f401e4 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -457,7 +457,6 @@ cc_library( ":cordz_update_scope", ":cordz_update_tracker", ":internal", - ":str_format", ":strings", "//absl/base", "//absl/base:config", diff --git a/absl/strings/cord.cc b/absl/strings/cord.cc index 1d33dd83..267163c8 100644 --- a/absl/strings/cord.cc +++ b/absl/strings/cord.cc @@ -48,7 +48,6 @@ #include "absl/strings/internal/cordz_update_tracker.h" #include "absl/strings/internal/resize_uninitialized.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" -- cgit v1.2.3 From c21bd95257bd0978824e12d91a6f8975a506877f Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 26 Jan 2023 09:43:17 -0800 Subject: Add lifetime annotations to AlphaNum. PiperOrigin-RevId: 504866618 Change-Id: I9519299946c693ae1bedd769a25a9cb5137f2f54 --- absl/strings/str_cat.h | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/absl/strings/str_cat.h b/absl/strings/str_cat.h index 730b4d8c..8b68dac0 100644 --- a/absl/strings/str_cat.h +++ b/absl/strings/str_cat.h @@ -94,6 +94,7 @@ #include #include +#include "absl/base/attributes.h" #include "absl/base/port.h" #include "absl/strings/internal/has_absl_stringify.h" #include "absl/strings/internal/stringify_sink.h" @@ -287,23 +288,28 @@ class AlphaNum { template AlphaNum( // NOLINT(runtime/explicit) - const strings_internal::AlphaNumBuffer& buf) + const strings_internal::AlphaNumBuffer& buf + ABSL_ATTRIBUTE_LIFETIME_BOUND) : piece_(&buf.data[0], buf.size) {} - AlphaNum(const char* c_str) // NOLINT(runtime/explicit) - : piece_(NullSafeStringView(c_str)) {} // NOLINT(runtime/explicit) - AlphaNum(absl::string_view pc) : piece_(pc) {} // NOLINT(runtime/explicit) + AlphaNum(const char* c_str // NOLINT(runtime/explicit) + ABSL_ATTRIBUTE_LIFETIME_BOUND) + : piece_(NullSafeStringView(c_str)) {} + AlphaNum(absl::string_view pc // NOLINT(runtime/explicit) + ABSL_ATTRIBUTE_LIFETIME_BOUND) + : piece_(pc) {} template ::value>::type> - AlphaNum( // NOLINT(runtime/explicit) - const T& v, // NOLINT(runtime/explicit) - strings_internal::StringifySink&& sink = {}) // NOLINT(runtime/explicit) + AlphaNum( // NOLINT(runtime/explicit) + const T& v ABSL_ATTRIBUTE_LIFETIME_BOUND, + strings_internal::StringifySink&& sink ABSL_ATTRIBUTE_LIFETIME_BOUND = {}) : piece_(strings_internal::ExtractStringification(sink, v)) {} template AlphaNum( // NOLINT(runtime/explicit) - const std::basic_string, Allocator>& str) + const std::basic_string, Allocator>& str + ABSL_ATTRIBUTE_LIFETIME_BOUND) : piece_(str) {} // Use string literals ":" instead of character literals ':'. -- cgit v1.2.3 From db51f68788bfc505e1d68a731b0175c593cf89cd Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Thu, 26 Jan 2023 14:22:33 -0800 Subject: Introduce Abseil Prefetch API PiperOrigin-RevId: 504941246 Change-Id: I94c1e85afd254e84948477b511d41eeb8285fdae --- CMake/AbseilDll.cmake | 1 + absl/base/BUILD.bazel | 17 ++-- absl/base/CMakeLists.txt | 3 +- absl/base/prefetch.h | 196 +++++++++++++++++++++++++++++++++++++++++++++ absl/base/prefetch_test.cc | 64 +++++++++++++++ 5 files changed, 272 insertions(+), 9 deletions(-) create mode 100644 absl/base/prefetch.h create mode 100644 absl/base/prefetch_test.cc diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index c4a41e6d..52a563cd 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -28,6 +28,7 @@ set(ABSL_INTERNAL_DLL_FILES "base/internal/low_level_scheduling.h" "base/internal/per_thread_tls.h" "base/internal/prefetch.h" + "base/prefetch.h" "base/internal/pretty_function.h" "base/internal/raw_logging.cc" "base/internal/raw_logging.h" diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index ded26d6a..b4d1c218 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -732,21 +732,22 @@ cc_test( cc_library( name = "prefetch", - hdrs = ["internal/prefetch.h"], + hdrs = [ + "internal/prefetch.h", + "prefetch.h", + ], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, - visibility = [ - "//absl:__subpackages__", - ], - deps = [ - ":config", - ], + deps = [":config"], ) cc_test( name = "prefetch_test", size = "small", - srcs = ["internal/prefetch_test.cc"], + srcs = [ + "internal/prefetch_test.cc", + "prefetch_test.cc", + ], copts = ABSL_TEST_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ diff --git a/absl/base/CMakeLists.txt b/absl/base/CMakeLists.txt index 26e2b48a..74495d01 100644 --- a/absl/base/CMakeLists.txt +++ b/absl/base/CMakeLists.txt @@ -645,11 +645,11 @@ absl_cc_test( GTest::gtest_main ) -# Internal-only target, do not depend on directly. absl_cc_library( NAME prefetch HDRS + "prefetch.h" "internal/prefetch.h" COPTS ${ABSL_DEFAULT_COPTS} @@ -663,6 +663,7 @@ absl_cc_test( NAME prefetch_test SRCS + "prefetch_test.cc" "internal/prefetch_test.cc" COPTS ${ABSL_TEST_COPTS} diff --git a/absl/base/prefetch.h b/absl/base/prefetch.h new file mode 100644 index 00000000..4d428462 --- /dev/null +++ b/absl/base/prefetch.h @@ -0,0 +1,196 @@ +// Copyright 2023 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ----------------------------------------------------------------------------- +// File: prefetch.h +// ----------------------------------------------------------------------------- +// +// This header file defines prefetch functions to prefetch memory contents +// into the first level cache (L1) for the current CPU. The prefetch logic +// offered in this header is limited to prefetching first level cachelines +// only, and is aimed at relatively 'simple' prefetching logic. +// +#ifndef ABSL_BASE_PREFETCH_H_ +#define ABSL_BASE_PREFETCH_H_ + +#include "absl/base/config.h" + +#if defined(ABSL_INTERNAL_HAVE_SSE) +#include +#endif + +#if defined(_MSC_VER) && defined(ABSL_INTERNAL_HAVE_SSE) +#include +#pragma intrinsic(_mm_prefetch) +#endif + +namespace absl { +ABSL_NAMESPACE_BEGIN + +// Moves data into the L1 cache before it is read, or "prefetches" it. +// +// The value of `addr` is the address of the memory to prefetch. If +// the target and compiler support it, data prefetch instructions are +// generated. If the prefetch is done some time before the memory is +// read, it may be in the cache by the time the read occurs. +// +// This method prefetches data with the highest degree of temporal locality; +// data is prefetched where possible into all levels of the cache. +// +// Incorrect or gratuitous use of this function can degrade performance. +// Use this function only when representative benchmarks show an improvement. +// +// Example: +// +// // Computes incremental checksum for `data`. +// int ComputeChecksum(int sum, absl::string_view data); +// +// // Computes cumulative checksum for all values in `data` +// int ComputeChecksum(absl::Span data) { +// int sum = 0; +// auto it = data.begin(); +// auto pit = data.begin(); +// auto end = data.end(); +// for (int dist = 8; dist > 0 && pit != data.end(); --dist, ++pit) { +// absl::PrefetchToLocalCache(pit->data()); +// } +// for (; pit != end; ++pit, ++it) { +// sum = ComputeChecksum(sum, *it); +// absl::PrefetchToLocalCache(pit->data()); +// } +// for (; it != end; ++it) { +// sum = ComputeChecksum(sum, *it); +// } +// return sum; +// } +// +void PrefetchToLocalCache(const void* addr); + +// Moves data into the L1 cache before it is read, or "prefetches" it. +// +// This function is identical to `PrefetchToLocalCache()` except that it has +// non-temporal locality: the fetched data should not be left in any of the +// cache tiers. This is useful for cases where the data is used only once / +// short term, for example, invoking a destructor on an object. +// +// Incorrect or gratuitous use of this function can degrade performance. +// Use this function only when representative benchmarks show an improvement. +// +// Example: +// +// template +// void DestroyPointers(Iterator begin, Iterator end) { +// size_t distance = std::min(8U, bars.size()); +// +// int dist = 8; +// auto prefetch_it = begin; +// while (prefetch_it != end && --dist;) { +// absl::PrefetchToLocalCacheNta(*prefetch_it++); +// } +// while (prefetch_it != end) { +// delete *begin++; +// absl::PrefetchToLocalCacheNta(*prefetch_it++); +// } +// while (begin != end) { +// delete *begin++; +// } +// } +// +void PrefetchToLocalCacheNta(const void* addr); + +// Moves data into the L1 cache with the intent to modify it. +// +// This function is similar to `PrefetchToLocalCache()` except that it +// prefetches cachelines with an 'intent to modify' This typically includes +// invalidating cache entries for this address in all other cache tiers, and an +// exclusive access intent. +// +// Incorrect or gratuitous use of this function can degrade performance. As this +// function can invalidate cached cachelines on other caches and computer cores, +// incorrect usage of this function can have an even greater negative impact +// than incorrect regular prefetches. +// Use this function only when representative benchmarks show an improvement. +// +// Example: +// +// void* Arena::Allocate(size_t size) { +// void* ptr = AllocateBlock(size); +// absl::PrefetchToLocalCacheForWrite(p); +// return ptr; +// } +// +void PrefetchToLocalCacheforWrite(const void* addr); + +#if ABSL_HAVE_BUILTIN(__builtin_prefetch) || defined(__GNUC__) + +#define ABSL_HAVE_PREFETCH 1 + +// See __builtin_prefetch: +// https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html. +// +inline void PrefetchToLocalCache(const void* addr) { + __builtin_prefetch(addr, 0, 3); +} + +inline void PrefetchToLocalCacheNta(const void* addr) { + __builtin_prefetch(addr, 0, 0); +} + +inline void PrefetchToLocalCacheForWrite(const void* addr) { + // [x86] gcc/clang don't generate PREFETCHW for __builtin_prefetch(.., 1) + // unless -march=broadwell or newer; this is not generally the default, so we + // manually emit prefetchw. PREFETCHW is recognized as a no-op on older Intel + // processors and has been present on AMD processors since the K6-2. +#if defined(__x86_64__) + asm("prefetchw (%0)" : : "r"(addr)); +#else + __builtin_prefetch(addr, 1, 0); +#endif +} + +#elif defined(ABSL_INTERNAL_HAVE_SSE) + +#define ABSL_HAVE_PREFETCH 1 + +inline void PrefetchToLocalCache(const void* addr) { + _mm_prefetch(reinterpret_cast(addr), _MM_HINT_T0); +} + +inline void PrefetchToLocalCacheNta(const void* addr) { + _mm_prefetch(reinterpret_cast(addr), _MM_HINT_NTA); +} + +inline void PrefetchToLocalCacheForWrite(const void* addr) { +#if defined(_MM_HINT_ET0) + _mm_prefetch(reinterpret_cast(addr), _MM_HINT_ET0); +#elif defined(__x86_64__) + // _MM_HINT_ET0 is not universally supported. As we commented further + // up, PREFETCHW is recognized as a no-op on older Intel processors + // and has been present on AMD processors since the K6-2 + asm("prefetchw (%0)" : : "r"(addr)); +#endif +} + +#else + +inline void PrefetchToLocalCache(const void* addr) {} +inline void PrefetchToLocalCacheNta(const void* addr) {} +inline void PrefetchToLocalCacheForWrite(const void* addr) {} + +#endif + +ABSL_NAMESPACE_END +} // namespace absl + +#endif // ABSL_BASE_PREFETCH_H_ diff --git a/absl/base/prefetch_test.cc b/absl/base/prefetch_test.cc new file mode 100644 index 00000000..ee219897 --- /dev/null +++ b/absl/base/prefetch_test.cc @@ -0,0 +1,64 @@ +// Copyright 2023 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/base/prefetch.h" + +#include + +#include "gtest/gtest.h" + +namespace { + +// Below tests exercise the functions only to guarantee they compile and execute +// correctly. We make no attempt at verifying any prefetch instructions being +// generated and executed: we assume the various implementation in terms of +// __builtin_prefetch() or x86 intrinsics to be correct and well tested. + +TEST(PrefetchTest, PrefetchToLocalCache_StackA) { + char buf[100] = {}; + absl::PrefetchToLocalCache(buf); + absl::PrefetchToLocalCacheNta(buf); + absl::PrefetchToLocalCacheForWrite(buf); +} + +TEST(PrefetchTest, PrefetchToLocalCache_Heap) { + auto memory = std::make_unique(200 << 10); + memset(memory.get(), 0, 200 << 10); + absl::PrefetchToLocalCache(memory.get()); + absl::PrefetchToLocalCacheNta(memory.get()); + absl::PrefetchToLocalCacheForWrite(memory.get()); + absl::PrefetchToLocalCache(memory.get() + (50 << 10)); + absl::PrefetchToLocalCacheNta(memory.get() + (50 << 10)); + absl::PrefetchToLocalCacheForWrite(memory.get() + (50 << 10)); + absl::PrefetchToLocalCache(memory.get() + (100 << 10)); + absl::PrefetchToLocalCacheNta(memory.get() + (100 << 10)); + absl::PrefetchToLocalCacheForWrite(memory.get() + (100 << 10)); + absl::PrefetchToLocalCache(memory.get() + (150 << 10)); + absl::PrefetchToLocalCacheNta(memory.get() + (150 << 10)); + absl::PrefetchToLocalCacheForWrite(memory.get() + (150 << 10)); +} + +TEST(PrefetchTest, PrefetchToLocalCache_Nullptr) { + absl::PrefetchToLocalCache(nullptr); + absl::PrefetchToLocalCacheNta(nullptr); + absl::PrefetchToLocalCacheForWrite(nullptr); +} + +TEST(PrefetchTest, PrefetchToLocalCache_InvalidPtr) { + absl::PrefetchToLocalCache(reinterpret_cast(0x785326532L)); + absl::PrefetchToLocalCacheNta(reinterpret_cast(0x785326532L)); + absl::PrefetchToLocalCacheForWrite(reinterpret_cast(0x78532L)); +} + +} // namespace -- cgit v1.2.3 From 8a0693b2a75f21e508ffcd172efda3bbb638a275 Mon Sep 17 00:00:00 2001 From: Phoebe Liang Date: Fri, 27 Jan 2023 10:54:21 -0800 Subject: Adds `AbslStringify` support to `absl::Hex` and `absl::Dec` `absl::Hex` and `absl::Dec` are now stringifiable through `AbslStringify`. This means that they can be passed to logging, `absl::StrFormat`, `absl::StrCat` and `absl::Substitute`. Note that this change will break unsupported usages of `absl::AlphaNum`. `absl::AlphaNum` is not intended to be instantiated as a stack variable due to lifetime issues. Unsupported usages include: * constructing an `absl::AlphaNum` as a local variable. * For example, `absl::AlphaNum a = absl::Hex(...);` * declaring an `absl::AlphaNum` as a member variable Usage of `absl::AlphaNum` as a function parameter will continue to be supported. PiperOrigin-RevId: 505158534 Change-Id: Idecdf0b3d137482e86b393c7480229343d68eb36 --- absl/strings/str_cat.cc | 49 ----------------------------------------- absl/strings/str_cat.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/absl/strings/str_cat.cc b/absl/strings/str_cat.cc index 114a2ff2..6c198f85 100644 --- a/absl/strings/str_cat.cc +++ b/absl/strings/str_cat.cc @@ -30,55 +30,6 @@ namespace absl { ABSL_NAMESPACE_BEGIN -AlphaNum::AlphaNum(Hex hex) { - static_assert(numbers_internal::kFastToBufferSize >= 32, - "This function only works when output buffer >= 32 bytes long"); - char* const end = &digits_[numbers_internal::kFastToBufferSize]; - auto real_width = - absl::numbers_internal::FastHexToBufferZeroPad16(hex.value, end - 16); - if (real_width >= hex.width) { - piece_ = absl::string_view(end - real_width, real_width); - } else { - // Pad first 16 chars because FastHexToBufferZeroPad16 pads only to 16 and - // max pad width can be up to 20. - std::memset(end - 32, hex.fill, 16); - // Patch up everything else up to the real_width. - std::memset(end - real_width - 16, hex.fill, 16); - piece_ = absl::string_view(end - hex.width, hex.width); - } -} - -AlphaNum::AlphaNum(Dec dec) { - assert(dec.width <= numbers_internal::kFastToBufferSize); - char* const end = &digits_[numbers_internal::kFastToBufferSize]; - char* const minfill = end - dec.width; - char* writer = end; - uint64_t value = dec.value; - bool neg = dec.neg; - while (value > 9) { - *--writer = '0' + (value % 10); - value /= 10; - } - *--writer = '0' + static_cast(value); - if (neg) *--writer = '-'; - - ptrdiff_t fillers = writer - minfill; - if (fillers > 0) { - // Tricky: if the fill character is ' ', then it's <+/-> - // But...: if the fill character is '0', then it's <+/-> - bool add_sign_again = false; - if (neg && dec.fill == '0') { // If filling with '0', - ++writer; // ignore the sign we just added - add_sign_again = true; // and re-add the sign later. - } - writer -= fillers; - std::fill_n(writer, fillers, dec.fill); - if (add_sign_again) *--writer = '-'; - } - - piece_ = absl::string_view(writer, static_cast(end - writer)); -} - // ---------------------------------------------------------------------- // StrCat() // This merges the given strings or integers, with no delimiter. This diff --git a/absl/strings/str_cat.h b/absl/strings/str_cat.h index 8b68dac0..fcd48c4e 100644 --- a/absl/strings/str_cat.h +++ b/absl/strings/str_cat.h @@ -87,8 +87,10 @@ #ifndef ABSL_STRINGS_STR_CAT_H_ #define ABSL_STRINGS_STR_CAT_H_ +#include #include #include +#include #include #include #include @@ -202,6 +204,27 @@ struct Hex { explicit Hex(Pointee* v, PadSpec spec = absl::kNoPad) : Hex(spec, reinterpret_cast(v)) {} + template + friend void AbslStringify(S& sink, Hex hex) { + static_assert( + numbers_internal::kFastToBufferSize >= 32, + "This function only works when output buffer >= 32 bytes long"); + char buffer[numbers_internal::kFastToBufferSize]; + char* const end = &buffer[numbers_internal::kFastToBufferSize]; + auto real_width = + absl::numbers_internal::FastHexToBufferZeroPad16(hex.value, end - 16); + if (real_width >= hex.width) { + sink.Append(absl::string_view(end - real_width, real_width)); + } else { + // Pad first 16 chars because FastHexToBufferZeroPad16 pads only to 16 and + // max pad width can be up to 20. + std::memset(end - 32, hex.fill, 16); + // Patch up everything else up to the real_width. + std::memset(end - real_width - 16, hex.fill, 16); + sink.Append(absl::string_view(end - hex.width, hex.width)); + } + } + private: Hex(PadSpec spec, uint64_t v) : value(v), @@ -236,6 +259,38 @@ struct Dec { : spec - absl::kZeroPad2 + 2), fill(spec >= absl::kSpacePad2 ? ' ' : '0'), neg(v < 0) {} + + template + friend void AbslStringify(S& sink, Dec dec) { + assert(dec.width <= numbers_internal::kFastToBufferSize); + char buffer[numbers_internal::kFastToBufferSize]; + char* const end = &buffer[numbers_internal::kFastToBufferSize]; + char* const minfill = end - dec.width; + char* writer = end; + uint64_t val = dec.value; + while (val > 9) { + *--writer = '0' + (val % 10); + val /= 10; + } + *--writer = '0' + static_cast(val); + if (dec.neg) *--writer = '-'; + + ptrdiff_t fillers = writer - minfill; + if (fillers > 0) { + // Tricky: if the fill character is ' ', then it's <+/-> + // But...: if the fill character is '0', then it's <+/-> + bool add_sign_again = false; + if (dec.neg && dec.fill == '0') { // If filling with '0', + ++writer; // ignore the sign we just added + add_sign_again = true; // and re-add the sign later. + } + writer -= fillers; + std::fill_n(writer, fillers, dec.fill); + if (add_sign_again) *--writer = '-'; + } + + sink.Append(absl::string_view(writer, static_cast(end - writer))); + } }; // ----------------------------------------------------------------------------- @@ -283,9 +338,6 @@ class AlphaNum { AlphaNum(double f) // NOLINT(runtime/explicit) : piece_(digits_, numbers_internal::SixDigitsToBuffer(f, digits_)) {} - AlphaNum(Hex hex); // NOLINT(runtime/explicit) - AlphaNum(Dec dec); // NOLINT(runtime/explicit) - template AlphaNum( // NOLINT(runtime/explicit) const strings_internal::AlphaNumBuffer& buf -- cgit v1.2.3 From 75d2525117c8da93840ab256f07b191086fd7cbb Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Fri, 27 Jan 2023 12:36:55 -0800 Subject: Replace absl::base_internal::Prefetch* calls with absl::Prefetch* calls PiperOrigin-RevId: 505184961 Change-Id: I64482558a76abda6896bec4b2d323833b6cd7edf --- absl/base/BUILD.bazel | 5 +++- absl/base/CMakeLists.txt | 1 + absl/base/internal/prefetch.h | 35 ++++++++++++++-------------- absl/base/prefetch.h | 13 ++++++++--- absl/container/internal/raw_hash_set.h | 22 ++++++++++------- absl/container/internal/raw_hash_set_test.cc | 2 +- absl/crc/internal/crc.cc | 4 ++-- absl/crc/internal/crc_memcpy_x86_64.cc | 8 +++---- absl/crc/internal/crc_x86_arm_combined.cc | 14 +++++------ 9 files changed, 58 insertions(+), 46 deletions(-) diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index b4d1c218..dd29daf6 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -738,7 +738,10 @@ cc_library( ], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, - deps = [":config"], + deps = [ + ":config", + ":core_headers", # TODO(b/265984188): remove + ], ) cc_test( diff --git a/absl/base/CMakeLists.txt b/absl/base/CMakeLists.txt index 74495d01..71b93795 100644 --- a/absl/base/CMakeLists.txt +++ b/absl/base/CMakeLists.txt @@ -657,6 +657,7 @@ absl_cc_library( ${ABSL_DEFAULT_LINKOPTS} DEPS absl::config + absl::core_headers # TODO(b/265984188): remove ) absl_cc_test( diff --git a/absl/base/internal/prefetch.h b/absl/base/internal/prefetch.h index 06419283..aecfd877 100644 --- a/absl/base/internal/prefetch.h +++ b/absl/base/internal/prefetch.h @@ -12,10 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +// TODO(b/265984188): remove all uses and delete this header. + #ifndef ABSL_BASE_INTERNAL_PREFETCH_H_ #define ABSL_BASE_INTERNAL_PREFETCH_H_ +#include "absl/base/attributes.h" #include "absl/base/config.h" +#include "absl/base/prefetch.h" #ifdef __SSE__ #include @@ -72,10 +76,21 @@ namespace absl { ABSL_NAMESPACE_BEGIN namespace base_internal { -void PrefetchT0(const void* addr); +ABSL_DEPRECATED("Use absl::PrefetchToLocalCache() instead") +inline void PrefetchT0(const void* address) { + absl::PrefetchToLocalCache(address); +} + +ABSL_DEPRECATED("Use absl::PrefetchToLocalCache() instead") +inline void PrefetchNta(const void* address) { + absl::PrefetchToLocalCacheNta(address); +} + +ABSL_DEPRECATED("Use __builtin_prefetch() for advanced prefetch logic instead") void PrefetchT1(const void* addr); + +ABSL_DEPRECATED("Use __builtin_prefetch() for advanced prefetch logic instead") void PrefetchT2(const void* addr); -void PrefetchNta(const void* addr); // Implementation details follow. @@ -90,10 +105,6 @@ void PrefetchNta(const void* addr); // safe for all currently supported platforms. However, prefetch for // store may have problems depending on the target platform. // -inline void PrefetchT0(const void* addr) { - // Note: this uses prefetcht0 on Intel. - __builtin_prefetch(addr, 0, 3); -} inline void PrefetchT1(const void* addr) { // Note: this uses prefetcht1 on Intel. __builtin_prefetch(addr, 0, 2); @@ -102,33 +113,21 @@ inline void PrefetchT2(const void* addr) { // Note: this uses prefetcht2 on Intel. __builtin_prefetch(addr, 0, 1); } -inline void PrefetchNta(const void* addr) { - // Note: this uses prefetchtnta on Intel. - __builtin_prefetch(addr, 0, 0); -} #elif defined(ABSL_INTERNAL_HAVE_SSE) #define ABSL_INTERNAL_HAVE_PREFETCH 1 -inline void PrefetchT0(const void* addr) { - _mm_prefetch(reinterpret_cast(addr), _MM_HINT_T0); -} inline void PrefetchT1(const void* addr) { _mm_prefetch(reinterpret_cast(addr), _MM_HINT_T1); } inline void PrefetchT2(const void* addr) { _mm_prefetch(reinterpret_cast(addr), _MM_HINT_T2); } -inline void PrefetchNta(const void* addr) { - _mm_prefetch(reinterpret_cast(addr), _MM_HINT_NTA); -} #else -inline void PrefetchT0(const void*) {} inline void PrefetchT1(const void*) {} inline void PrefetchT2(const void*) {} -inline void PrefetchNta(const void*) {} #endif } // namespace base_internal diff --git a/absl/base/prefetch.h b/absl/base/prefetch.h index 4d428462..6bc98637 100644 --- a/absl/base/prefetch.h +++ b/absl/base/prefetch.h @@ -30,9 +30,11 @@ #include #endif -#if defined(_MSC_VER) && defined(ABSL_INTERNAL_HAVE_SSE) +#if defined(_MSC_VER) && _MSC_VER >= 1900 && \ + (defined(_M_X64) || defined(_M_IX86)) #include #pragma intrinsic(_mm_prefetch) +#pragma intrinsic(_m_prefetchw) #endif namespace absl { @@ -174,10 +176,15 @@ inline void PrefetchToLocalCacheNta(const void* addr) { inline void PrefetchToLocalCacheForWrite(const void* addr) { #if defined(_MM_HINT_ET0) _mm_prefetch(reinterpret_cast(addr), _MM_HINT_ET0); -#elif defined(__x86_64__) +#elif defined(_MSC_VER) && _MSC_VER >= 1900 && \ + (defined(_M_X64) || defined(_M_IX86)) + // MSVC 2015 and up on x86/x64 supports prefetchw (feature listed as 3DNOW) + _m_prefetchw(const_cast(addr)); +#elif !defined(_MSC_VER) && defined(__x86_64__) // _MM_HINT_ET0 is not universally supported. As we commented further // up, PREFETCHW is recognized as a no-op on older Intel processors - // and has been present on AMD processors since the K6-2 + // and has been present on AMD processors since the K6-2. We have this + // disabled for MSVC compilers as this miscompiles on older MSVC compilers. asm("prefetchw (%0)" : : "r"(addr)); #endif } diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 61ef196d..09b55f66 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -185,10 +185,10 @@ #include "absl/base/config.h" #include "absl/base/internal/endian.h" -#include "absl/base/internal/prefetch.h" #include "absl/base/internal/raw_logging.h" #include "absl/base/optimization.h" #include "absl/base/port.h" +#include "absl/base/prefetch.h" #include "absl/container/internal/common.h" #include "absl/container/internal/compressed_tuple.h" #include "absl/container/internal/container_memory.h" @@ -2117,12 +2117,12 @@ class raw_hash_set { void prefetch(const key_arg& key) const { (void)key; // Avoid probing if we won't be able to prefetch the addresses received. -#ifdef ABSL_INTERNAL_HAVE_PREFETCH +#ifdef ABSL_HAVE_PREFETCH prefetch_heap_block(); auto seq = probe(common(), hash_ref()(key)); - base_internal::PrefetchT0(control() + seq.offset()); - base_internal::PrefetchT0(slot_array() + seq.offset()); -#endif // ABSL_INTERNAL_HAVE_PREFETCH + PrefetchToLocalCache(control() + seq.offset()); + PrefetchToLocalCache(slot_array() + seq.offset()); +#endif // ABSL_HAVE_PREFETCH } // The API of find() has two extensions. @@ -2529,10 +2529,14 @@ class raw_hash_set { // See `CapacityToGrowth()`. size_t& growth_left() { return common().growth_left(); } - // Prefetch the heap-allocated memory region to resolve potential TLB misses. - // This is intended to overlap with execution of calculating the hash for a - // key. - void prefetch_heap_block() const { base_internal::PrefetchT2(control()); } + // Prefetch the heap-allocated memory region to resolve potential TLB and + // cache misses. This is intended to overlap with execution of calculating the + // hash for a key. + void prefetch_heap_block() const { +#if ABSL_HAVE_BUILTIN(__builtin_prefetch) || defined(__GNUC__) + __builtin_prefetch(control(), 0, 1); +#endif + } CommonFields& common() { return settings_.template get<0>(); } const CommonFields& common() const { return settings_.template get<0>(); } diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 3d3b089c..bdffb817 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -40,8 +40,8 @@ #include "absl/base/attributes.h" #include "absl/base/config.h" #include "absl/base/internal/cycleclock.h" -#include "absl/base/internal/prefetch.h" #include "absl/base/internal/raw_logging.h" +#include "absl/base/prefetch.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/container/internal/container_memory.h" diff --git a/absl/crc/internal/crc.cc b/absl/crc/internal/crc.cc index bb8936e3..337a173f 100644 --- a/absl/crc/internal/crc.cc +++ b/absl/crc/internal/crc.cc @@ -44,8 +44,8 @@ #include #include "absl/base/internal/endian.h" -#include "absl/base/internal/prefetch.h" #include "absl/base/internal/raw_logging.h" +#include "absl/base/prefetch.h" #include "absl/crc/internal/crc_internal.h" namespace absl { @@ -309,7 +309,7 @@ void CRC32::Extend(uint32_t* crc, const void* bytes, size_t length) const { // Process kStride interleaved swaths through the data in parallel. while ((e - p) > kPrefetchHorizon) { - base_internal::PrefetchNta( + PrefetchToLocalCacheNta( reinterpret_cast(p + kPrefetchHorizon)); // Process 64 bytes at a time step_stride(); diff --git a/absl/crc/internal/crc_memcpy_x86_64.cc b/absl/crc/internal/crc_memcpy_x86_64.cc index 66f784de..0078f0e8 100644 --- a/absl/crc/internal/crc_memcpy_x86_64.cc +++ b/absl/crc/internal/crc_memcpy_x86_64.cc @@ -52,8 +52,8 @@ #include #include "absl/base/dynamic_annotations.h" -#include "absl/base/internal/prefetch.h" #include "absl/base/optimization.h" +#include "absl/base/prefetch.h" #include "absl/crc/crc32c.h" #include "absl/crc/internal/cpu_detect.h" #include "absl/crc/internal/crc_memcpy.h" @@ -242,10 +242,8 @@ crc32c_t AcceleratedCrcMemcpyEngine::Compute( while (copy_rounds > kBlocksPerCacheLine) { // Prefetch kPrefetchAhead bytes ahead of each pointer. for (size_t i = 0; i < kRegions; i++) { - absl::base_internal::PrefetchT0(src_bytes + kPrefetchAhead + - region_size * i); - absl::base_internal::PrefetchT0(dst_bytes + kPrefetchAhead + - region_size * i); + absl::PrefetchToLocalCache(src_bytes + kPrefetchAhead + region_size * i); + absl::PrefetchToLocalCache(dst_bytes + kPrefetchAhead + region_size * i); } // Load and store data, computing CRC on the way. diff --git a/absl/crc/internal/crc_x86_arm_combined.cc b/absl/crc/internal/crc_x86_arm_combined.cc index d71191e3..e482b37a 100644 --- a/absl/crc/internal/crc_x86_arm_combined.cc +++ b/absl/crc/internal/crc_x86_arm_combined.cc @@ -21,7 +21,7 @@ #include "absl/base/config.h" #include "absl/base/dynamic_annotations.h" #include "absl/base/internal/endian.h" -#include "absl/base/internal/prefetch.h" +#include "absl/base/prefetch.h" #include "absl/crc/internal/cpu_detect.h" #include "absl/crc/internal/crc.h" #include "absl/crc/internal/crc32_x86_arm_combined_simd.h" @@ -429,11 +429,11 @@ class CRC32AcceleratedX86ARMCombinedMultipleStreams ABSL_INTERNAL_STEP8BY3(l64, l641, l642, p, p1, p2); ABSL_INTERNAL_STEP8BY3(l64, l641, l642, p, p1, p2); ABSL_INTERNAL_STEP8BY3(l64, l641, l642, p, p1, p2); - base_internal::PrefetchT0( + PrefetchToLocalCache( reinterpret_cast(p + kPrefetchHorizonMedium)); - base_internal::PrefetchT0( + PrefetchToLocalCache( reinterpret_cast(p1 + kPrefetchHorizonMedium)); - base_internal::PrefetchT0( + PrefetchToLocalCache( reinterpret_cast(p2 + kPrefetchHorizonMedium)); } // Don't run crc on last 8 bytes. @@ -517,12 +517,12 @@ class CRC32AcceleratedX86ARMCombinedMultipleStreams for (size_t i = 1; i < bs; i++) { // Prefetch data for next itterations. for (size_t j = 0; j < num_crc_streams; j++) { - base_internal::PrefetchT0( + PrefetchToLocalCache( reinterpret_cast(crc_streams[j] + kPrefetchHorizon)); } for (size_t j = 0; j < num_pclmul_streams; j++) { - base_internal::PrefetchT0(reinterpret_cast( - pclmul_streams[j] + kPrefetchHorizon)); + PrefetchToLocalCache(reinterpret_cast(pclmul_streams[j] + + kPrefetchHorizon)); } // We process each stream in 64 byte blocks. This can be written as -- cgit v1.2.3 From 0c3df2f5a7d3918b9e5762cd8f143a6004b76cda Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Sat, 28 Jan 2023 13:41:47 -0800 Subject: The type of MINSIGSTKSZ is not guaranteed; avoid potential implicit sign conversion. On some glibcs, this is defined as a call to sysconf(), which returns a long. PiperOrigin-RevId: 505380003 Change-Id: I53207846d733d3a529630a6aff9bca425cf90a21 --- absl/debugging/internal/stack_consumption.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/debugging/internal/stack_consumption.cc b/absl/debugging/internal/stack_consumption.cc index 51348649..6d974992 100644 --- a/absl/debugging/internal/stack_consumption.cc +++ b/absl/debugging/internal/stack_consumption.cc @@ -162,7 +162,7 @@ int GetSignalHandlerStackConsumption(void (*signal_handler)(int)) { // versions of musl have a bug that rejects ss_size==0. Work around this by // setting ss_size to MINSIGSTKSZ, which should be ignored by the kernel // when SS_DISABLE is set. - old_sigstk.ss_size = MINSIGSTKSZ; + old_sigstk.ss_size = static_cast(MINSIGSTKSZ); } ABSL_RAW_CHECK(sigaltstack(&old_sigstk, nullptr) == 0, "sigaltstack() failed"); -- cgit v1.2.3 From ed59f62f8bbc5f05bcba2f89ee16f107e03813f2 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 30 Jan 2023 14:59:39 -0800 Subject: In sanitizer mode, detect when references become invalidated by randomly rehashing on insertions when there is no reserved growth. PiperOrigin-RevId: 505807487 Change-Id: I9051a04f6a75e579d16e9ae8defd404bcc377fba --- absl/container/BUILD.bazel | 1 + absl/container/CMakeLists.txt | 1 + absl/container/internal/raw_hash_set.cc | 15 +++++++++++++ absl/container/internal/raw_hash_set.h | 33 ++++++++++++++++++++-------- absl/container/internal/raw_hash_set_test.cc | 31 ++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 7a966d63..92031811 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -623,6 +623,7 @@ cc_library( "//absl/base:endian", "//absl/base:prefetch", "//absl/base:raw_logging_internal", + "//absl/hash", "//absl/memory", "//absl/meta:type_traits", "//absl/numeric:bits", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 416e3e38..2ebfd08e 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -705,6 +705,7 @@ absl_cc_library( absl::container_memory absl::core_headers absl::endian + absl::hash absl::hash_policy_traits absl::hashtable_debug_hooks absl::hashtablez_sampler diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 3677ac59..5dc8b2fa 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -19,6 +19,7 @@ #include #include "absl/base/config.h" +#include "absl/hash/hash.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -51,6 +52,20 @@ inline size_t RandomSeed() { return value ^ static_cast(reinterpret_cast(&counter)); } +bool CommonFieldsGenerationInfoEnabled:: + should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, + size_t capacity) const { + if (reserved_growth_ == kReservedGrowthJustRanOut) return true; + if (reserved_growth_ > 0) return false; + // Note: we can't use the abseil-random library because abseil-random + // depends on swisstable. We want to return true with probability + // `min(1, RehashProbabilityConstant() / capacity())`. In order to do this, + // we probe based on a random hash and see if the offset is less than + // RehashProbabilityConstant(). + return probe(ctrl, capacity, absl::HashOf(RandomSeed())).offset() < + RehashProbabilityConstant(); +} + bool ShouldInsertBackwards(size_t hash, const ctrl_t* ctrl) { // To avoid problems with weak hashes and single bit tests, we use % 13. // TODO(kfm,sbenza): revisit after we do unconditional mixing diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 09b55f66..65dc66c2 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -749,6 +749,15 @@ using Group = GroupAArch64Impl; using Group = GroupPortableImpl; #endif +// When there is an insertion with no reserved growth, we rehash with +// probability `min(1, RehashProbabilityConstant() / capacity())`. Using a +// constant divided by capacity ensures that inserting N elements is still O(N) +// in the average case. Using the constant 16 means that we expect to rehash ~8 +// times more often than when generations are disabled. We are adding expected +// rehash_probability * #insertions/capacity_growth = 16/capacity * ((7/8 - +// 7/16) * capacity)/capacity_growth = ~7 extra rehashes per capacity growth. +inline size_t RehashProbabilityConstant() { return 16; } + class CommonFieldsGenerationInfoEnabled { // A sentinel value for reserved_growth_ indicating that we just ran out of // reserved growth on the last insertion. When reserve is called and then @@ -769,12 +778,10 @@ class CommonFieldsGenerationInfoEnabled { // Whether we should rehash on insert in order to detect bugs of using invalid // references. We rehash on the first insertion after reserved_growth_ reaches - // 0 after a call to reserve. - // TODO(b/254649633): we could potentially do a rehash with low probability + // 0 after a call to reserve. We also do a rehash with low probability // whenever reserved_growth_ is zero. - bool should_rehash_for_bug_detection_on_insert() const { - return reserved_growth_ == kReservedGrowthJustRanOut; - } + bool should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, + size_t capacity) const; void maybe_increment_generation_on_insert() { if (reserved_growth_ == kReservedGrowthJustRanOut) reserved_growth_ = 0; @@ -820,7 +827,9 @@ class CommonFieldsGenerationInfoDisabled { CommonFieldsGenerationInfoDisabled& operator=( CommonFieldsGenerationInfoDisabled&&) = default; - bool should_rehash_for_bug_detection_on_insert() const { return false; } + bool should_rehash_for_bug_detection_on_insert(const ctrl_t*, size_t) const { + return false; + } void maybe_increment_generation_on_insert() {} void reset_reserved_growth(size_t, size_t) {} size_t reserved_growth() const { return 0; } @@ -905,6 +914,10 @@ class CommonFields : public CommonFieldsGenerationInfo { return compressed_tuple_.template get<1>(); } + bool should_rehash_for_bug_detection_on_insert() const { + return CommonFieldsGenerationInfo:: + should_rehash_for_bug_detection_on_insert(control_, capacity_); + } void reset_reserved_growth(size_t reservation) { CommonFieldsGenerationInfo::reset_reserved_growth(reservation, size_); } @@ -1106,11 +1119,13 @@ struct FindInfo { inline bool is_small(size_t capacity) { return capacity < Group::kWidth - 1; } // Begins a probing operation on `common.control`, using `hash`. -inline probe_seq probe(const CommonFields& common, size_t hash) { - const ctrl_t* ctrl = common.control_; - const size_t capacity = common.capacity_; +inline probe_seq probe(const ctrl_t* ctrl, const size_t capacity, + size_t hash) { return probe_seq(H1(hash, ctrl), capacity); } +inline probe_seq probe(const CommonFields& common, size_t hash) { + return probe(common.control_, common.capacity_, hash); +} // Probes an array of control bits using a probe sequence derived from `hash`, // and returns the offset corresponding to the first deleted or empty slot. diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index bdffb817..e33fda20 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -865,6 +865,10 @@ void TestDecompose(bool construct_three) { } TEST(Table, Decompose) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + TestDecompose(false); struct TransparentHashIntOverload { @@ -903,6 +907,10 @@ struct Modulo1000HashTable // Test that rehash with no resize happen in case of many deleted slots. TEST(Table, RehashWithNoResize) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + Modulo1000HashTable t; // Adding the same length (and the same hash) strings // to have at least kMinFullGroups groups @@ -996,6 +1004,10 @@ TEST(Table, EnsureNonQuadraticAsInRust) { } TEST(Table, ClearBug) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + IntTable t; constexpr size_t capacity = container_internal::Group::kWidth - 1; constexpr size_t max_size = capacity / 2 + 1; @@ -2318,6 +2330,25 @@ TEST(Table, ReservedGrowthUpdatesWhenTableDoesntGrow) { EXPECT_EQ(*it, 0); } +TEST(Table, InvalidReferenceUseCrashesWithSanitizers) { + if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; +#ifdef ABSL_HAVE_MEMORY_SANITIZER + GTEST_SKIP() << "MSan fails to detect some of these rehashes."; +#endif + + IntTable t; + t.insert(0); + // Rehashing is guaranteed on every insertion while capacity is less than + // RehashProbabilityConstant(). + int64_t i = 0; + while (t.capacity() <= RehashProbabilityConstant()) { + // ptr will become invalidated on rehash. + const int64_t* ptr = &*t.begin(); + t.insert(++i); + EXPECT_DEATH_IF_SUPPORTED(std::cout << *ptr, "heap-use-after-free") << i; + } +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END -- cgit v1.2.3 From dcddc54407e44b55815e9aa784c84f5c933ca310 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 31 Jan 2023 08:00:06 -0800 Subject: Rollback in sanitizer mode, detect when references become invalidated by randomly rehashing on insertions when there is no reserved growth. Rollback of ed59f62f8bbc5f05bcba2f89ee16f107e03813f2 PiperOrigin-RevId: 506003574 Change-Id: I1766321f279a3226e2821e0390387d5639d28964 --- absl/container/BUILD.bazel | 1 - absl/container/CMakeLists.txt | 1 - absl/container/internal/raw_hash_set.cc | 15 ------------- absl/container/internal/raw_hash_set.h | 33 ++++++++-------------------- absl/container/internal/raw_hash_set_test.cc | 31 -------------------------- 5 files changed, 9 insertions(+), 72 deletions(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 92031811..7a966d63 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -623,7 +623,6 @@ cc_library( "//absl/base:endian", "//absl/base:prefetch", "//absl/base:raw_logging_internal", - "//absl/hash", "//absl/memory", "//absl/meta:type_traits", "//absl/numeric:bits", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 2ebfd08e..416e3e38 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -705,7 +705,6 @@ absl_cc_library( absl::container_memory absl::core_headers absl::endian - absl::hash absl::hash_policy_traits absl::hashtable_debug_hooks absl::hashtablez_sampler diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 5dc8b2fa..3677ac59 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -19,7 +19,6 @@ #include #include "absl/base/config.h" -#include "absl/hash/hash.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -52,20 +51,6 @@ inline size_t RandomSeed() { return value ^ static_cast(reinterpret_cast(&counter)); } -bool CommonFieldsGenerationInfoEnabled:: - should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, - size_t capacity) const { - if (reserved_growth_ == kReservedGrowthJustRanOut) return true; - if (reserved_growth_ > 0) return false; - // Note: we can't use the abseil-random library because abseil-random - // depends on swisstable. We want to return true with probability - // `min(1, RehashProbabilityConstant() / capacity())`. In order to do this, - // we probe based on a random hash and see if the offset is less than - // RehashProbabilityConstant(). - return probe(ctrl, capacity, absl::HashOf(RandomSeed())).offset() < - RehashProbabilityConstant(); -} - bool ShouldInsertBackwards(size_t hash, const ctrl_t* ctrl) { // To avoid problems with weak hashes and single bit tests, we use % 13. // TODO(kfm,sbenza): revisit after we do unconditional mixing diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 65dc66c2..09b55f66 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -749,15 +749,6 @@ using Group = GroupAArch64Impl; using Group = GroupPortableImpl; #endif -// When there is an insertion with no reserved growth, we rehash with -// probability `min(1, RehashProbabilityConstant() / capacity())`. Using a -// constant divided by capacity ensures that inserting N elements is still O(N) -// in the average case. Using the constant 16 means that we expect to rehash ~8 -// times more often than when generations are disabled. We are adding expected -// rehash_probability * #insertions/capacity_growth = 16/capacity * ((7/8 - -// 7/16) * capacity)/capacity_growth = ~7 extra rehashes per capacity growth. -inline size_t RehashProbabilityConstant() { return 16; } - class CommonFieldsGenerationInfoEnabled { // A sentinel value for reserved_growth_ indicating that we just ran out of // reserved growth on the last insertion. When reserve is called and then @@ -778,10 +769,12 @@ class CommonFieldsGenerationInfoEnabled { // Whether we should rehash on insert in order to detect bugs of using invalid // references. We rehash on the first insertion after reserved_growth_ reaches - // 0 after a call to reserve. We also do a rehash with low probability + // 0 after a call to reserve. + // TODO(b/254649633): we could potentially do a rehash with low probability // whenever reserved_growth_ is zero. - bool should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, - size_t capacity) const; + bool should_rehash_for_bug_detection_on_insert() const { + return reserved_growth_ == kReservedGrowthJustRanOut; + } void maybe_increment_generation_on_insert() { if (reserved_growth_ == kReservedGrowthJustRanOut) reserved_growth_ = 0; @@ -827,9 +820,7 @@ class CommonFieldsGenerationInfoDisabled { CommonFieldsGenerationInfoDisabled& operator=( CommonFieldsGenerationInfoDisabled&&) = default; - bool should_rehash_for_bug_detection_on_insert(const ctrl_t*, size_t) const { - return false; - } + bool should_rehash_for_bug_detection_on_insert() const { return false; } void maybe_increment_generation_on_insert() {} void reset_reserved_growth(size_t, size_t) {} size_t reserved_growth() const { return 0; } @@ -914,10 +905,6 @@ class CommonFields : public CommonFieldsGenerationInfo { return compressed_tuple_.template get<1>(); } - bool should_rehash_for_bug_detection_on_insert() const { - return CommonFieldsGenerationInfo:: - should_rehash_for_bug_detection_on_insert(control_, capacity_); - } void reset_reserved_growth(size_t reservation) { CommonFieldsGenerationInfo::reset_reserved_growth(reservation, size_); } @@ -1119,12 +1106,10 @@ struct FindInfo { inline bool is_small(size_t capacity) { return capacity < Group::kWidth - 1; } // Begins a probing operation on `common.control`, using `hash`. -inline probe_seq probe(const ctrl_t* ctrl, const size_t capacity, - size_t hash) { - return probe_seq(H1(hash, ctrl), capacity); -} inline probe_seq probe(const CommonFields& common, size_t hash) { - return probe(common.control_, common.capacity_, hash); + const ctrl_t* ctrl = common.control_; + const size_t capacity = common.capacity_; + return probe_seq(H1(hash, ctrl), capacity); } // Probes an array of control bits using a probe sequence derived from `hash`, diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index e33fda20..bdffb817 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -865,10 +865,6 @@ void TestDecompose(bool construct_three) { } TEST(Table, Decompose) { - if (SwisstableGenerationsEnabled()) { - GTEST_SKIP() << "Generations being enabled causes extra rehashes."; - } - TestDecompose(false); struct TransparentHashIntOverload { @@ -907,10 +903,6 @@ struct Modulo1000HashTable // Test that rehash with no resize happen in case of many deleted slots. TEST(Table, RehashWithNoResize) { - if (SwisstableGenerationsEnabled()) { - GTEST_SKIP() << "Generations being enabled causes extra rehashes."; - } - Modulo1000HashTable t; // Adding the same length (and the same hash) strings // to have at least kMinFullGroups groups @@ -1004,10 +996,6 @@ TEST(Table, EnsureNonQuadraticAsInRust) { } TEST(Table, ClearBug) { - if (SwisstableGenerationsEnabled()) { - GTEST_SKIP() << "Generations being enabled causes extra rehashes."; - } - IntTable t; constexpr size_t capacity = container_internal::Group::kWidth - 1; constexpr size_t max_size = capacity / 2 + 1; @@ -2330,25 +2318,6 @@ TEST(Table, ReservedGrowthUpdatesWhenTableDoesntGrow) { EXPECT_EQ(*it, 0); } -TEST(Table, InvalidReferenceUseCrashesWithSanitizers) { - if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; -#ifdef ABSL_HAVE_MEMORY_SANITIZER - GTEST_SKIP() << "MSan fails to detect some of these rehashes."; -#endif - - IntTable t; - t.insert(0); - // Rehashing is guaranteed on every insertion while capacity is less than - // RehashProbabilityConstant(). - int64_t i = 0; - while (t.capacity() <= RehashProbabilityConstant()) { - // ptr will become invalidated on rehash. - const int64_t* ptr = &*t.begin(); - t.insert(++i); - EXPECT_DEATH_IF_SUPPORTED(std::cout << *ptr, "heap-use-after-free") << i; - } -} - } // namespace } // namespace container_internal ABSL_NAMESPACE_END -- cgit v1.2.3 From 1a38beaaaf4cbdcc61a03e07a1212be17a5cd5c8 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 31 Jan 2023 16:37:12 -0800 Subject: Add const qualifier to c_binary_search Sequence. PiperOrigin-RevId: 506151912 Change-Id: I388884b0bd80ef0f8a0b3d3c7d0e1b404ddfb742 --- absl/algorithm/container.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/absl/algorithm/container.h b/absl/algorithm/container.h index c7782d4f..679e0267 100644 --- a/absl/algorithm/container.h +++ b/absl/algorithm/container.h @@ -1131,7 +1131,7 @@ c_equal_range(Sequence& sequence, const T& value, LessThan&& comp) { // to test if any element in the sorted container contains a value equivalent to // 'value'. template -bool c_binary_search(Sequence&& sequence, const T& value) { +bool c_binary_search(const Sequence& sequence, const T& value) { return std::binary_search(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), value); @@ -1140,7 +1140,8 @@ bool c_binary_search(Sequence&& sequence, const T& value) { // Overload of c_binary_search() for performing a `comp` comparison other than // the default `operator<`. template -bool c_binary_search(Sequence&& sequence, const T& value, LessThan&& comp) { +bool c_binary_search(const Sequence& sequence, const T& value, + LessThan&& comp) { return std::binary_search(container_algorithm_internal::c_begin(sequence), container_algorithm_internal::c_end(sequence), value, std::forward(comp)); -- cgit v1.2.3 From 385ad37dac01bb65a2cef93565fd3731b142b702 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Wed, 1 Feb 2023 07:49:45 -0800 Subject: Rollforward: in sanitizer mode, detect when references become invalidated by randomly rehashing on insertions when there is no reserved growth. Rollforward of ed59f62f8bbc5f05bcba2f89ee16f107e03813f2 PiperOrigin-RevId: 506314970 Change-Id: I7a654aef36bb169da9ea5c618789ee771f05fe28 --- absl/container/BUILD.bazel | 1 + absl/container/CMakeLists.txt | 1 + absl/container/internal/raw_hash_set.cc | 15 +++++++++++++ absl/container/internal/raw_hash_set.h | 33 ++++++++++++++++++++-------- absl/container/internal/raw_hash_set_test.cc | 31 ++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 7a966d63..92031811 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -623,6 +623,7 @@ cc_library( "//absl/base:endian", "//absl/base:prefetch", "//absl/base:raw_logging_internal", + "//absl/hash", "//absl/memory", "//absl/meta:type_traits", "//absl/numeric:bits", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 416e3e38..2ebfd08e 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -705,6 +705,7 @@ absl_cc_library( absl::container_memory absl::core_headers absl::endian + absl::hash absl::hash_policy_traits absl::hashtable_debug_hooks absl::hashtablez_sampler diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 3677ac59..5dc8b2fa 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -19,6 +19,7 @@ #include #include "absl/base/config.h" +#include "absl/hash/hash.h" namespace absl { ABSL_NAMESPACE_BEGIN @@ -51,6 +52,20 @@ inline size_t RandomSeed() { return value ^ static_cast(reinterpret_cast(&counter)); } +bool CommonFieldsGenerationInfoEnabled:: + should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, + size_t capacity) const { + if (reserved_growth_ == kReservedGrowthJustRanOut) return true; + if (reserved_growth_ > 0) return false; + // Note: we can't use the abseil-random library because abseil-random + // depends on swisstable. We want to return true with probability + // `min(1, RehashProbabilityConstant() / capacity())`. In order to do this, + // we probe based on a random hash and see if the offset is less than + // RehashProbabilityConstant(). + return probe(ctrl, capacity, absl::HashOf(RandomSeed())).offset() < + RehashProbabilityConstant(); +} + bool ShouldInsertBackwards(size_t hash, const ctrl_t* ctrl) { // To avoid problems with weak hashes and single bit tests, we use % 13. // TODO(kfm,sbenza): revisit after we do unconditional mixing diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 09b55f66..65dc66c2 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -749,6 +749,15 @@ using Group = GroupAArch64Impl; using Group = GroupPortableImpl; #endif +// When there is an insertion with no reserved growth, we rehash with +// probability `min(1, RehashProbabilityConstant() / capacity())`. Using a +// constant divided by capacity ensures that inserting N elements is still O(N) +// in the average case. Using the constant 16 means that we expect to rehash ~8 +// times more often than when generations are disabled. We are adding expected +// rehash_probability * #insertions/capacity_growth = 16/capacity * ((7/8 - +// 7/16) * capacity)/capacity_growth = ~7 extra rehashes per capacity growth. +inline size_t RehashProbabilityConstant() { return 16; } + class CommonFieldsGenerationInfoEnabled { // A sentinel value for reserved_growth_ indicating that we just ran out of // reserved growth on the last insertion. When reserve is called and then @@ -769,12 +778,10 @@ class CommonFieldsGenerationInfoEnabled { // Whether we should rehash on insert in order to detect bugs of using invalid // references. We rehash on the first insertion after reserved_growth_ reaches - // 0 after a call to reserve. - // TODO(b/254649633): we could potentially do a rehash with low probability + // 0 after a call to reserve. We also do a rehash with low probability // whenever reserved_growth_ is zero. - bool should_rehash_for_bug_detection_on_insert() const { - return reserved_growth_ == kReservedGrowthJustRanOut; - } + bool should_rehash_for_bug_detection_on_insert(const ctrl_t* ctrl, + size_t capacity) const; void maybe_increment_generation_on_insert() { if (reserved_growth_ == kReservedGrowthJustRanOut) reserved_growth_ = 0; @@ -820,7 +827,9 @@ class CommonFieldsGenerationInfoDisabled { CommonFieldsGenerationInfoDisabled& operator=( CommonFieldsGenerationInfoDisabled&&) = default; - bool should_rehash_for_bug_detection_on_insert() const { return false; } + bool should_rehash_for_bug_detection_on_insert(const ctrl_t*, size_t) const { + return false; + } void maybe_increment_generation_on_insert() {} void reset_reserved_growth(size_t, size_t) {} size_t reserved_growth() const { return 0; } @@ -905,6 +914,10 @@ class CommonFields : public CommonFieldsGenerationInfo { return compressed_tuple_.template get<1>(); } + bool should_rehash_for_bug_detection_on_insert() const { + return CommonFieldsGenerationInfo:: + should_rehash_for_bug_detection_on_insert(control_, capacity_); + } void reset_reserved_growth(size_t reservation) { CommonFieldsGenerationInfo::reset_reserved_growth(reservation, size_); } @@ -1106,11 +1119,13 @@ struct FindInfo { inline bool is_small(size_t capacity) { return capacity < Group::kWidth - 1; } // Begins a probing operation on `common.control`, using `hash`. -inline probe_seq probe(const CommonFields& common, size_t hash) { - const ctrl_t* ctrl = common.control_; - const size_t capacity = common.capacity_; +inline probe_seq probe(const ctrl_t* ctrl, const size_t capacity, + size_t hash) { return probe_seq(H1(hash, ctrl), capacity); } +inline probe_seq probe(const CommonFields& common, size_t hash) { + return probe(common.control_, common.capacity_, hash); +} // Probes an array of control bits using a probe sequence derived from `hash`, // and returns the offset corresponding to the first deleted or empty slot. diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index bdffb817..e33fda20 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -865,6 +865,10 @@ void TestDecompose(bool construct_three) { } TEST(Table, Decompose) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + TestDecompose(false); struct TransparentHashIntOverload { @@ -903,6 +907,10 @@ struct Modulo1000HashTable // Test that rehash with no resize happen in case of many deleted slots. TEST(Table, RehashWithNoResize) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + Modulo1000HashTable t; // Adding the same length (and the same hash) strings // to have at least kMinFullGroups groups @@ -996,6 +1004,10 @@ TEST(Table, EnsureNonQuadraticAsInRust) { } TEST(Table, ClearBug) { + if (SwisstableGenerationsEnabled()) { + GTEST_SKIP() << "Generations being enabled causes extra rehashes."; + } + IntTable t; constexpr size_t capacity = container_internal::Group::kWidth - 1; constexpr size_t max_size = capacity / 2 + 1; @@ -2318,6 +2330,25 @@ TEST(Table, ReservedGrowthUpdatesWhenTableDoesntGrow) { EXPECT_EQ(*it, 0); } +TEST(Table, InvalidReferenceUseCrashesWithSanitizers) { + if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; +#ifdef ABSL_HAVE_MEMORY_SANITIZER + GTEST_SKIP() << "MSan fails to detect some of these rehashes."; +#endif + + IntTable t; + t.insert(0); + // Rehashing is guaranteed on every insertion while capacity is less than + // RehashProbabilityConstant(). + int64_t i = 0; + while (t.capacity() <= RehashProbabilityConstant()) { + // ptr will become invalidated on rehash. + const int64_t* ptr = &*t.begin(); + t.insert(++i); + EXPECT_DEATH_IF_SUPPORTED(std::cout << *ptr, "heap-use-after-free") << i; + } +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END -- cgit v1.2.3 From 7c7cafef4ff11979a26702080a2a4bf0dad7d856 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Wed, 1 Feb 2023 08:24:55 -0800 Subject: Delete unused Futex methods PiperOrigin-RevId: 506323250 Change-Id: I0f7d4532c19088b011740ceff87ecec55cc34edb --- absl/synchronization/internal/futex.h | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/absl/synchronization/internal/futex.h b/absl/synchronization/internal/futex.h index cb97da09..9cf9841d 100644 --- a/absl/synchronization/internal/futex.h +++ b/absl/synchronization/internal/futex.h @@ -110,19 +110,6 @@ class FutexImpl { return 0; } - static int WaitBitsetAbsoluteTimeout(std::atomic *v, int32_t val, - int32_t bits, - const struct timespec *abstime) { - // NOLINTNEXTLINE(runtime/int) - long err = syscall(SYS_futex, reinterpret_cast(v), - FUTEX_WAIT_BITSET | FUTEX_PRIVATE_FLAG, val, abstime, - nullptr, bits); - if (ABSL_PREDICT_FALSE(err != 0)) { - return -errno; - } - return 0; - } - static int Wake(std::atomic *v, int32_t count) { // NOLINTNEXTLINE(runtime/int) long err = syscall(SYS_futex, reinterpret_cast(v), @@ -132,18 +119,6 @@ class FutexImpl { } return 0; } - - // FUTEX_WAKE_BITSET - static int WakeBitset(std::atomic *v, int32_t count, int32_t bits) { - // NOLINTNEXTLINE(runtime/int) - long err = syscall(SYS_futex, reinterpret_cast(v), - FUTEX_WAKE_BITSET | FUTEX_PRIVATE_FLAG, count, nullptr, - nullptr, bits); - if (ABSL_PREDICT_FALSE(err < 0)) { - return -errno; - } - return 0; - } }; class Futex : public FutexImpl {}; -- cgit v1.2.3 From 9858e5421b14c9d72f2103af10010c5452c6d55f Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 2 Feb 2023 00:48:53 -0800 Subject: Get rid of tail padding within `absl::Duration`. This reduces memory usage needs when storing duration in containers (e.g. `vector` uses 25% less memory), and allows classes with `absl::Duration` fields to fit other stuff in memory previously used by tail padding (e.g. `std::optional` is now 16 bytes instead of 24). PiperOrigin-RevId: 506543503 Change-Id: Ifeb2397c953a5d3da317a70ab49a3ebb85042344 --- absl/time/duration.cc | 39 ++++++++++++++----------- absl/time/duration_test.cc | 10 ++++++- absl/time/time.h | 73 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 101 insertions(+), 21 deletions(-) diff --git a/absl/time/duration.cc b/absl/time/duration.cc index 911e80f8..b915d749 100644 --- a/absl/time/duration.cc +++ b/absl/time/duration.cc @@ -407,16 +407,18 @@ int64_t IDivDuration(bool satq, const Duration num, const Duration den, Duration& Duration::operator+=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) return *this = rhs; - const int64_t orig_rep_hi = rep_hi_; - rep_hi_ = - DecodeTwosComp(EncodeTwosComp(rep_hi_) + EncodeTwosComp(rhs.rep_hi_)); + const int64_t orig_rep_hi = rep_hi_.Get(); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + + EncodeTwosComp(rhs.rep_hi_.Get())); if (rep_lo_ >= kTicksPerSecond - rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) + 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + 1); rep_lo_ -= kTicksPerSecond; } rep_lo_ += rhs.rep_lo_; - if (rhs.rep_hi_ < 0 ? rep_hi_ > orig_rep_hi : rep_hi_ < orig_rep_hi) { - return *this = rhs.rep_hi_ < 0 ? -InfiniteDuration() : InfiniteDuration(); + if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() > orig_rep_hi + : rep_hi_.Get() < orig_rep_hi) { + return *this = + rhs.rep_hi_.Get() < 0 ? -InfiniteDuration() : InfiniteDuration(); } return *this; } @@ -424,18 +426,21 @@ Duration& Duration::operator+=(Duration rhs) { Duration& Duration::operator-=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) { - return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); + return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() + : InfiniteDuration(); } - const int64_t orig_rep_hi = rep_hi_; - rep_hi_ = - DecodeTwosComp(EncodeTwosComp(rep_hi_) - EncodeTwosComp(rhs.rep_hi_)); + const int64_t orig_rep_hi = rep_hi_.Get(); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - + EncodeTwosComp(rhs.rep_hi_.Get())); if (rep_lo_ < rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) - 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - 1); rep_lo_ += kTicksPerSecond; } rep_lo_ -= rhs.rep_lo_; - if (rhs.rep_hi_ < 0 ? rep_hi_ < orig_rep_hi : rep_hi_ > orig_rep_hi) { - return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); + if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() < orig_rep_hi + : rep_hi_.Get() > orig_rep_hi) { + return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() + : InfiniteDuration(); } return *this; } @@ -446,7 +451,7 @@ Duration& Duration::operator-=(Duration rhs) { Duration& Duration::operator*=(int64_t r) { if (time_internal::IsInfiniteDuration(*this)) { - const bool is_neg = (r < 0) != (rep_hi_ < 0); + const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -454,7 +459,7 @@ Duration& Duration::operator*=(int64_t r) { Duration& Duration::operator*=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsFinite(r)) { - const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); + const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); @@ -462,7 +467,7 @@ Duration& Duration::operator*=(double r) { Duration& Duration::operator/=(int64_t r) { if (time_internal::IsInfiniteDuration(*this) || r == 0) { - const bool is_neg = (r < 0) != (rep_hi_ < 0); + const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -470,7 +475,7 @@ Duration& Duration::operator/=(int64_t r) { Duration& Duration::operator/=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsValidDivisor(r)) { - const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); + const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); diff --git a/absl/time/duration_test.cc b/absl/time/duration_test.cc index b7abf4ba..84224f47 100644 --- a/absl/time/duration_test.cc +++ b/absl/time/duration_test.cc @@ -16,8 +16,9 @@ #include // for timeval #endif -#include // NOLINT(build/c++11) +#include #include +#include // NOLINT(build/c++11) #include #include #include @@ -1853,4 +1854,11 @@ TEST(Duration, FormatParseRoundTrip) { #undef TEST_PARSE_ROUNDTRIP } +TEST(Duration, NoPadding) { + // Should match the size of a struct with uint32_t alignment and no padding. + using NoPadding = std::array; + EXPECT_EQ(sizeof(NoPadding), sizeof(absl::Duration)); + EXPECT_EQ(alignof(NoPadding), alignof(absl::Duration)); +} + } // namespace diff --git a/absl/time/time.h b/absl/time/time.h index cc390082..bf1007c2 100644 --- a/absl/time/time.h +++ b/absl/time/time.h @@ -84,6 +84,7 @@ struct timeval; #include #include +#include "absl/base/config.h" #include "absl/base/macros.h" #include "absl/strings/string_view.h" #include "absl/time/civil_time.h" @@ -214,7 +215,7 @@ class Duration { template friend H AbslHashValue(H h, Duration d) { - return H::combine(std::move(h), d.rep_hi_, d.rep_lo_); + return H::combine(std::move(h), d.rep_hi_.Get(), d.rep_lo_); } private: @@ -223,7 +224,73 @@ class Duration { friend constexpr Duration time_internal::MakeDuration(int64_t hi, uint32_t lo); constexpr Duration(int64_t hi, uint32_t lo) : rep_hi_(hi), rep_lo_(lo) {} - int64_t rep_hi_; + + // We store `hi_rep_` 4-byte rather than 8-byte aligned to avoid 4 bytes of + // tail padding. + class HiRep { + public: + // Default constructor default-initializes `hi_`, which has the same + // semantics as default-initializing an `int64_t` (undetermined value). + HiRep() = default; + + HiRep(const HiRep&) = default; + HiRep& operator=(const HiRep&) = default; + + explicit constexpr HiRep(const int64_t value) + : // C++17 forbids default-initialization in constexpr contexts. We can + // remove this in C++20. + lo_(0), + hi_(0) { + *this = value; + } + + constexpr int64_t Get() const { + const uint64_t unsigned_value = + (static_cast(hi_) << 32) | static_cast(lo_); + // `static_cast(unsigned_value)` is implementation-defined + // before c++20. On all supported platforms the behaviour is that mandated + // by c++20, i.e. "If the destination type is signed, [...] the result is + // the unique value of the destination type equal to the source value + // modulo 2^n, where n is the number of bits used to represent the + // destination type." + static_assert( + (static_cast((std::numeric_limits::max)()) == + int64_t{-1}) && + (static_cast(static_cast( + (std::numeric_limits::max)()) + + 1) == + (std::numeric_limits::min)()), + "static_cast(uint64_t) does not have c++20 semantics"); + return static_cast(unsigned_value); + } + + constexpr HiRep& operator=(const int64_t value) { + // "If the destination type is unsigned, the resulting value is the + // smallest unsigned value equal to the source value modulo 2^n + // where `n` is the number of bits used to represent the destination + // type". + const auto unsigned_value = static_cast(value); + hi_ = static_cast(unsigned_value >> 32); + lo_ = static_cast(unsigned_value); + return *this; + } + + private: + // Notes: + // - Ideally we would use a `char[]` and `std::bitcast`, but the latter + // does not exist (and is not constexpr in `absl`) before c++20. + // - Order is optimized depending on endianness so that the compiler can + // turn `Get()` (resp. `operator=()`) into a single 8-byte load (resp. + // store). +#if defined(ABSL_IS_BIG_ENDIAN) && ABSL_IS_BIG_ENDIAN + uint32_t hi_; + uint32_t lo_; +#else + uint32_t lo_; + uint32_t hi_; +#endif + }; + HiRep rep_hi_; uint32_t rep_lo_; }; @@ -1491,7 +1558,7 @@ ABSL_ATTRIBUTE_CONST_FUNCTION constexpr Duration MakeNormalizedDuration( // Provide access to the Duration representation. ABSL_ATTRIBUTE_CONST_FUNCTION constexpr int64_t GetRepHi(Duration d) { - return d.rep_hi_; + return d.rep_hi_.Get(); } ABSL_ATTRIBUTE_CONST_FUNCTION constexpr uint32_t GetRepLo(Duration d) { return d.rep_lo_; -- cgit v1.2.3 From 7005fede1e309d0a6070be3008b365112492aa80 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 2 Feb 2023 03:05:07 -0800 Subject: Get rid of tail padding within `absl::Duration`. This reduces memory usage needs when storing duration in containers (e.g. `vector` uses 25% less memory), and allows classes with `absl::Duration` fields to fit other stuff in memory previously used by tail padding (e.g. `std::optional` is now 16 bytes instead of 24). PiperOrigin-RevId: 506568782 Change-Id: Ic9e077f02a80da013fb2d312aff77761b970c07a --- absl/time/duration.cc | 39 +++++++++++-------------- absl/time/duration_test.cc | 10 +------ absl/time/time.h | 73 ++-------------------------------------------- 3 files changed, 21 insertions(+), 101 deletions(-) diff --git a/absl/time/duration.cc b/absl/time/duration.cc index b915d749..911e80f8 100644 --- a/absl/time/duration.cc +++ b/absl/time/duration.cc @@ -407,18 +407,16 @@ int64_t IDivDuration(bool satq, const Duration num, const Duration den, Duration& Duration::operator+=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) return *this = rhs; - const int64_t orig_rep_hi = rep_hi_.Get(); - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + - EncodeTwosComp(rhs.rep_hi_.Get())); + const int64_t orig_rep_hi = rep_hi_; + rep_hi_ = + DecodeTwosComp(EncodeTwosComp(rep_hi_) + EncodeTwosComp(rhs.rep_hi_)); if (rep_lo_ >= kTicksPerSecond - rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) + 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) + 1); rep_lo_ -= kTicksPerSecond; } rep_lo_ += rhs.rep_lo_; - if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() > orig_rep_hi - : rep_hi_.Get() < orig_rep_hi) { - return *this = - rhs.rep_hi_.Get() < 0 ? -InfiniteDuration() : InfiniteDuration(); + if (rhs.rep_hi_ < 0 ? rep_hi_ > orig_rep_hi : rep_hi_ < orig_rep_hi) { + return *this = rhs.rep_hi_ < 0 ? -InfiniteDuration() : InfiniteDuration(); } return *this; } @@ -426,21 +424,18 @@ Duration& Duration::operator+=(Duration rhs) { Duration& Duration::operator-=(Duration rhs) { if (time_internal::IsInfiniteDuration(*this)) return *this; if (time_internal::IsInfiniteDuration(rhs)) { - return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() - : InfiniteDuration(); + return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); } - const int64_t orig_rep_hi = rep_hi_.Get(); - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - - EncodeTwosComp(rhs.rep_hi_.Get())); + const int64_t orig_rep_hi = rep_hi_; + rep_hi_ = + DecodeTwosComp(EncodeTwosComp(rep_hi_) - EncodeTwosComp(rhs.rep_hi_)); if (rep_lo_ < rhs.rep_lo_) { - rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_.Get()) - 1); + rep_hi_ = DecodeTwosComp(EncodeTwosComp(rep_hi_) - 1); rep_lo_ += kTicksPerSecond; } rep_lo_ -= rhs.rep_lo_; - if (rhs.rep_hi_.Get() < 0 ? rep_hi_.Get() < orig_rep_hi - : rep_hi_.Get() > orig_rep_hi) { - return *this = rhs.rep_hi_.Get() >= 0 ? -InfiniteDuration() - : InfiniteDuration(); + if (rhs.rep_hi_ < 0 ? rep_hi_ < orig_rep_hi : rep_hi_ > orig_rep_hi) { + return *this = rhs.rep_hi_ >= 0 ? -InfiniteDuration() : InfiniteDuration(); } return *this; } @@ -451,7 +446,7 @@ Duration& Duration::operator-=(Duration rhs) { Duration& Duration::operator*=(int64_t r) { if (time_internal::IsInfiniteDuration(*this)) { - const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); + const bool is_neg = (r < 0) != (rep_hi_ < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -459,7 +454,7 @@ Duration& Duration::operator*=(int64_t r) { Duration& Duration::operator*=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsFinite(r)) { - const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); + const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); @@ -467,7 +462,7 @@ Duration& Duration::operator*=(double r) { Duration& Duration::operator/=(int64_t r) { if (time_internal::IsInfiniteDuration(*this) || r == 0) { - const bool is_neg = (r < 0) != (rep_hi_.Get() < 0); + const bool is_neg = (r < 0) != (rep_hi_ < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleFixed(*this, r); @@ -475,7 +470,7 @@ Duration& Duration::operator/=(int64_t r) { Duration& Duration::operator/=(double r) { if (time_internal::IsInfiniteDuration(*this) || !IsValidDivisor(r)) { - const bool is_neg = std::signbit(r) != (rep_hi_.Get() < 0); + const bool is_neg = (std::signbit(r) != 0) != (rep_hi_ < 0); return *this = is_neg ? -InfiniteDuration() : InfiniteDuration(); } return *this = ScaleDouble(*this, r); diff --git a/absl/time/duration_test.cc b/absl/time/duration_test.cc index 84224f47..b7abf4ba 100644 --- a/absl/time/duration_test.cc +++ b/absl/time/duration_test.cc @@ -16,9 +16,8 @@ #include // for timeval #endif -#include -#include #include // NOLINT(build/c++11) +#include #include #include #include @@ -1854,11 +1853,4 @@ TEST(Duration, FormatParseRoundTrip) { #undef TEST_PARSE_ROUNDTRIP } -TEST(Duration, NoPadding) { - // Should match the size of a struct with uint32_t alignment and no padding. - using NoPadding = std::array; - EXPECT_EQ(sizeof(NoPadding), sizeof(absl::Duration)); - EXPECT_EQ(alignof(NoPadding), alignof(absl::Duration)); -} - } // namespace diff --git a/absl/time/time.h b/absl/time/time.h index bf1007c2..cc390082 100644 --- a/absl/time/time.h +++ b/absl/time/time.h @@ -84,7 +84,6 @@ struct timeval; #include #include -#include "absl/base/config.h" #include "absl/base/macros.h" #include "absl/strings/string_view.h" #include "absl/time/civil_time.h" @@ -215,7 +214,7 @@ class Duration { template friend H AbslHashValue(H h, Duration d) { - return H::combine(std::move(h), d.rep_hi_.Get(), d.rep_lo_); + return H::combine(std::move(h), d.rep_hi_, d.rep_lo_); } private: @@ -224,73 +223,7 @@ class Duration { friend constexpr Duration time_internal::MakeDuration(int64_t hi, uint32_t lo); constexpr Duration(int64_t hi, uint32_t lo) : rep_hi_(hi), rep_lo_(lo) {} - - // We store `hi_rep_` 4-byte rather than 8-byte aligned to avoid 4 bytes of - // tail padding. - class HiRep { - public: - // Default constructor default-initializes `hi_`, which has the same - // semantics as default-initializing an `int64_t` (undetermined value). - HiRep() = default; - - HiRep(const HiRep&) = default; - HiRep& operator=(const HiRep&) = default; - - explicit constexpr HiRep(const int64_t value) - : // C++17 forbids default-initialization in constexpr contexts. We can - // remove this in C++20. - lo_(0), - hi_(0) { - *this = value; - } - - constexpr int64_t Get() const { - const uint64_t unsigned_value = - (static_cast(hi_) << 32) | static_cast(lo_); - // `static_cast(unsigned_value)` is implementation-defined - // before c++20. On all supported platforms the behaviour is that mandated - // by c++20, i.e. "If the destination type is signed, [...] the result is - // the unique value of the destination type equal to the source value - // modulo 2^n, where n is the number of bits used to represent the - // destination type." - static_assert( - (static_cast((std::numeric_limits::max)()) == - int64_t{-1}) && - (static_cast(static_cast( - (std::numeric_limits::max)()) + - 1) == - (std::numeric_limits::min)()), - "static_cast(uint64_t) does not have c++20 semantics"); - return static_cast(unsigned_value); - } - - constexpr HiRep& operator=(const int64_t value) { - // "If the destination type is unsigned, the resulting value is the - // smallest unsigned value equal to the source value modulo 2^n - // where `n` is the number of bits used to represent the destination - // type". - const auto unsigned_value = static_cast(value); - hi_ = static_cast(unsigned_value >> 32); - lo_ = static_cast(unsigned_value); - return *this; - } - - private: - // Notes: - // - Ideally we would use a `char[]` and `std::bitcast`, but the latter - // does not exist (and is not constexpr in `absl`) before c++20. - // - Order is optimized depending on endianness so that the compiler can - // turn `Get()` (resp. `operator=()`) into a single 8-byte load (resp. - // store). -#if defined(ABSL_IS_BIG_ENDIAN) && ABSL_IS_BIG_ENDIAN - uint32_t hi_; - uint32_t lo_; -#else - uint32_t lo_; - uint32_t hi_; -#endif - }; - HiRep rep_hi_; + int64_t rep_hi_; uint32_t rep_lo_; }; @@ -1558,7 +1491,7 @@ ABSL_ATTRIBUTE_CONST_FUNCTION constexpr Duration MakeNormalizedDuration( // Provide access to the Duration representation. ABSL_ATTRIBUTE_CONST_FUNCTION constexpr int64_t GetRepHi(Duration d) { - return d.rep_hi_.Get(); + return d.rep_hi_; } ABSL_ATTRIBUTE_CONST_FUNCTION constexpr uint32_t GetRepLo(Duration d) { return d.rep_lo_; -- cgit v1.2.3 From 115aac772c23907df48e298ebee04154bb2805c5 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Thu, 2 Feb 2023 08:04:36 -0800 Subject: Fix missing includes/dependencies PiperOrigin-RevId: 506622658 Change-Id: I17ae2d97a6cadb7bdd8ebd0ec0dd3976568cb7e1 --- absl/container/BUILD.bazel | 2 ++ absl/container/CMakeLists.txt | 2 ++ absl/container/internal/hashtablez_sampler.cc | 2 ++ absl/flags/BUILD.bazel | 1 + absl/flags/CMakeLists.txt | 1 + absl/flags/usage.cc | 1 + absl/strings/BUILD.bazel | 1 + absl/strings/CMakeLists.txt | 1 + absl/strings/internal/cordz_info.cc | 1 + 9 files changed, 12 insertions(+) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 92031811..96963c4b 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -534,11 +534,13 @@ cc_library( "//absl/base", "//absl/base:config", "//absl/base:core_headers", + "//absl/base:raw_logging_internal", "//absl/debugging:stacktrace", "//absl/memory", "//absl/profiling:exponential_biased", "//absl/profiling:sample_recorder", "//absl/synchronization", + "//absl/time", "//absl/utility", ], ) diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 2ebfd08e..9ba95b32 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -592,8 +592,10 @@ absl_cc_library( absl::base absl::config absl::exponential_biased + absl::raw_logging_internal absl::sample_recorder absl::synchronization + absl::time ) absl_cc_test( diff --git a/absl/container/internal/hashtablez_sampler.cc b/absl/container/internal/hashtablez_sampler.cc index 6b6d3491..79a0973a 100644 --- a/absl/container/internal/hashtablez_sampler.cc +++ b/absl/container/internal/hashtablez_sampler.cc @@ -23,11 +23,13 @@ #include "absl/base/attributes.h" #include "absl/base/config.h" +#include "absl/base/internal/raw_logging.h" #include "absl/debugging/stacktrace.h" #include "absl/memory/memory.h" #include "absl/profiling/internal/exponential_biased.h" #include "absl/profiling/internal/sample_recorder.h" #include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" #include "absl/utility/utility.h" namespace absl { diff --git a/absl/flags/BUILD.bazel b/absl/flags/BUILD.bazel index f5b3e033..570d280c 100644 --- a/absl/flags/BUILD.bazel +++ b/absl/flags/BUILD.bazel @@ -284,6 +284,7 @@ cc_library( ":usage_internal", "//absl/base:config", "//absl/base:core_headers", + "//absl/base:raw_logging_internal", "//absl/strings", "//absl/synchronization", ], diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt index 2f234aa4..af5e0254 100644 --- a/absl/flags/CMakeLists.txt +++ b/absl/flags/CMakeLists.txt @@ -262,6 +262,7 @@ absl_cc_library( absl::config absl::core_headers absl::flags_usage_internal + absl::raw_logging_internal absl::strings absl::synchronization ) diff --git a/absl/flags/usage.cc b/absl/flags/usage.cc index 452f6675..267a5039 100644 --- a/absl/flags/usage.cc +++ b/absl/flags/usage.cc @@ -21,6 +21,7 @@ #include "absl/base/attributes.h" #include "absl/base/config.h" #include "absl/base/const_init.h" +#include "absl/base/internal/raw_logging.h" #include "absl/base/thread_annotations.h" #include "absl/flags/internal/usage.h" #include "absl/strings/string_view.h" diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index c6f401e4..2989a47c 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -513,6 +513,7 @@ cc_library( "//absl/container:inlined_vector", "//absl/debugging:stacktrace", "//absl/synchronization", + "//absl/time", "//absl/types:span", ], ) diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index a0f7cc54..d2928bd7 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -745,6 +745,7 @@ absl_cc_library( absl::raw_logging_internal absl::stacktrace absl::synchronization + absl::time ) absl_cc_test( diff --git a/absl/strings/internal/cordz_info.cc b/absl/strings/internal/cordz_info.cc index 530f33be..0bea9c46 100644 --- a/absl/strings/internal/cordz_info.cc +++ b/absl/strings/internal/cordz_info.cc @@ -26,6 +26,7 @@ #include "absl/strings/internal/cordz_statistics.h" #include "absl/strings/internal/cordz_update_tracker.h" #include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" #include "absl/types/span.h" namespace absl { -- cgit v1.2.3 From 3020b58f0d987073b8adab204426f82c3f60b283 Mon Sep 17 00:00:00 2001 From: Laramie Leavitt Date: Fri, 3 Feb 2023 10:55:01 -0800 Subject: Add ABSL_ATTRIBUTE_LIFETIME_BOUND to absl::StatusOr::emplace() PiperOrigin-RevId: 506944023 Change-Id: I04ff13dcec89c57ba03707f5698c4198434216b6 --- absl/status/statusor.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/absl/status/statusor.h b/absl/status/statusor.h index a76e7201..beedd795 100644 --- a/absl/status/statusor.h +++ b/absl/status/statusor.h @@ -584,7 +584,7 @@ class StatusOr : private internal_statusor::StatusOrData, // Reconstructs the inner value T in-place using the provided args, using the // T(args...) constructor. Returns reference to the reconstructed `T`. template - T& emplace(Args&&... args) { + T& emplace(Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND { if (ok()) { this->Clear(); this->MakeValue(std::forward(args)...); @@ -600,7 +600,8 @@ class StatusOr : private internal_statusor::StatusOrData, absl::enable_if_t< std::is_constructible&, Args&&...>::value, int> = 0> - T& emplace(std::initializer_list ilist, Args&&... args) { + T& emplace(std::initializer_list ilist, + Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND { if (ok()) { this->Clear(); this->MakeValue(ilist, std::forward(args)...); -- cgit v1.2.3 From cdad8cd96ee9bfe11056997dc960eb2e52c6b00e Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 3 Feb 2023 16:00:19 -0800 Subject: `PrefetchToLocalCacheForWrite` should use `__builtin_prefetch(, 1, 3)` not `__builtin_prefetch, 1, 0)` PiperOrigin-RevId: 507015724 Change-Id: I99f44fe819c27e6dcc0bb7b36f1a37015c6f1987 --- absl/base/prefetch.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/absl/base/prefetch.h b/absl/base/prefetch.h index 6bc98637..a0b1efe6 100644 --- a/absl/base/prefetch.h +++ b/absl/base/prefetch.h @@ -132,7 +132,7 @@ void PrefetchToLocalCacheNta(const void* addr); // return ptr; // } // -void PrefetchToLocalCacheforWrite(const void* addr); +void PrefetchToLocalCacheForWrite(const void* addr); #if ABSL_HAVE_BUILTIN(__builtin_prefetch) || defined(__GNUC__) @@ -157,7 +157,7 @@ inline void PrefetchToLocalCacheForWrite(const void* addr) { #if defined(__x86_64__) asm("prefetchw (%0)" : : "r"(addr)); #else - __builtin_prefetch(addr, 1, 0); + __builtin_prefetch(addr, 1, 3); #endif } -- cgit v1.2.3 From 8e3a3e5f56407f3d675ac9041a20f7455c2ef01a Mon Sep 17 00:00:00 2001 From: Trish Lam Date: Mon, 6 Feb 2023 12:11:17 -0800 Subject: Solving issue with implied SSE when running ARM64EC --- absl/base/config.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/absl/base/config.h b/absl/base/config.h index c2a973af..73d6213d 100644 --- a/absl/base/config.h +++ b/absl/base/config.h @@ -889,7 +889,7 @@ static_assert(ABSL_INTERNAL_INLINE_NAMESPACE_STR[0] != 'h' || #error ABSL_INTERNAL_HAVE_SSE cannot be directly set #elif defined(__SSE__) #define ABSL_INTERNAL_HAVE_SSE 1 -#elif defined(_M_X64) || (defined(_M_IX86_FP) && _M_IX86_FP >= 1) +#elif (defined(_M_X64) || (defined(_M_IX86_FP) && _M_IX86_FP >= 1)) && !defined(_M_ARM64EC) // MSVC only defines _M_IX86_FP for x86 32-bit code, and _M_IX86_FP >= 1 // indicates that at least SSE was targeted with the /arch:SSE option. // All x86-64 processors support SSE, so support can be assumed. @@ -904,7 +904,7 @@ static_assert(ABSL_INTERNAL_INLINE_NAMESPACE_STR[0] != 'h' || #error ABSL_INTERNAL_HAVE_SSE2 cannot be directly set #elif defined(__SSE2__) #define ABSL_INTERNAL_HAVE_SSE2 1 -#elif defined(_M_X64) || (defined(_M_IX86_FP) && _M_IX86_FP >= 2) +#elif (defined(_M_X64) || (defined(_M_IX86_FP) && _M_IX86_FP >= 2)) && !defined(_M_ARM64EC) // MSVC only defines _M_IX86_FP for x86 32-bit code, and _M_IX86_FP >= 2 // indicates that at least SSE2 was targeted with the /arch:SSE2 option. // All x86-64 processors support SSE2, so support can be assumed. -- cgit v1.2.3 From 92fc445f7c0d8158c5f26abab9049fcfbcd0ccff Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 6 Feb 2023 15:23:40 -0800 Subject: Fix a discrepancy between absl::Hash and absl::HashOf for some negative signed integral types and improve the performance of absl::Hash. PiperOrigin-RevId: 507598042 Change-Id: I96a7bd6b9c360f435f216b2671ae84d9768a46e8 --- absl/hash/hash_test.cc | 15 +++++++++++++-- absl/hash/internal/hash.h | 3 ++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/absl/hash/hash_test.cc b/absl/hash/hash_test.cc index 5b556180..4ed6e50a 100644 --- a/absl/hash/hash_test.cc +++ b/absl/hash/hash_test.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -1241,14 +1242,24 @@ TEST(HashTest, DoesNotUseImplicitConversionsToBool) { TEST(HashOf, MatchesHashForSingleArgument) { std::string s = "forty two"; - int i = 42; double d = 42.0; std::tuple t{4, 2}; + int i = 42; + int neg_i = -42; + int16_t i16 = 42; + int16_t neg_i16 = -42; + int8_t i8 = 42; + int8_t neg_i8 = -42; EXPECT_EQ(absl::HashOf(s), absl::Hash{}(s)); - EXPECT_EQ(absl::HashOf(i), absl::Hash{}(i)); EXPECT_EQ(absl::HashOf(d), absl::Hash{}(d)); EXPECT_EQ(absl::HashOf(t), (absl::Hash>{}(t))); + EXPECT_EQ(absl::HashOf(i), absl::Hash{}(i)); + EXPECT_EQ(absl::HashOf(neg_i), absl::Hash{}(neg_i)); + EXPECT_EQ(absl::HashOf(i16), absl::Hash{}(i16)); + EXPECT_EQ(absl::HashOf(neg_i16), absl::Hash{}(neg_i16)); + EXPECT_EQ(absl::HashOf(i8), absl::Hash{}(i8)); + EXPECT_EQ(absl::HashOf(neg_i8), absl::Hash{}(neg_i8)); } TEST(HashOf, MatchesHashOfTupleForMultipleArguments) { diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index ccf4cc1a..eb50e2c7 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -969,7 +969,8 @@ class ABSL_DLL MixingHashState : public HashStateBase { // The result should be the same as running the whole algorithm, but faster. template ::value, int> = 0> static size_t hash(T value) { - return static_cast(Mix(Seed(), static_cast(value))); + return static_cast( + Mix(Seed(), static_cast>(value))); } // Overload of MixingHashState::hash() -- cgit v1.2.3 From f8fa267ec230b8577507b3798ac234999b72f348 Mon Sep 17 00:00:00 2001 From: Saran Tunyasuvunakool Date: Tue, 7 Feb 2023 08:35:38 -0800 Subject: Don't assume that AVX implies PCLMULQDQ when using LLVM on Windows. PiperOrigin-RevId: 507790741 Change-Id: I347357f9a2d698510f29b7d1b065ef73f9289292 --- absl/crc/internal/crc32_x86_arm_combined_simd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/crc/internal/crc32_x86_arm_combined_simd.h b/absl/crc/internal/crc32_x86_arm_combined_simd.h index fb643986..39e53dd0 100644 --- a/absl/crc/internal/crc32_x86_arm_combined_simd.h +++ b/absl/crc/internal/crc32_x86_arm_combined_simd.h @@ -33,7 +33,7 @@ #include #define ABSL_CRC_INTERNAL_HAVE_X86_SIMD -#elif defined(_MSC_VER) && defined(__AVX__) +#elif defined(_MSC_VER) && !defined(__clang__) && defined(__AVX__) // MSVC AVX (/arch:AVX) implies SSE 4.2 and PCLMULQDQ. #include -- cgit v1.2.3 From 2de126cc5826a8d464270ead65a7a9a7b012b741 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 8 Feb 2023 03:13:31 -0800 Subject: Change implementation of OnlyLiteralZero to only fail if the second overload is chosen, not in overload resolution. PiperOrigin-RevId: 508031321 Change-Id: I6981371fbc6498047babe3468f3343090f8bda47 --- absl/types/compare.h | 119 +++++++++++++++++++++++++-------------------------- 1 file changed, 59 insertions(+), 60 deletions(-) diff --git a/absl/types/compare.h b/absl/types/compare.h index 78aa26de..1a965e97 100644 --- a/absl/types/compare.h +++ b/absl/types/compare.h @@ -44,29 +44,28 @@ namespace compare_internal { using value_type = int8_t; -template -struct Fail { - static_assert(sizeof(T) < 0, "Only literal `0` is allowed."); -}; +class OnlyLiteralZero { + // A private type which cannot be named to explicitly cast to it. + struct MatchLiteralZero; -// We need the NullPtrT template to avoid triggering the modernize-use-nullptr -// ClangTidy warning in user code. -template -struct OnlyLiteralZero { - constexpr OnlyLiteralZero(NullPtrT) noexcept {} // NOLINT + public: + // Accept only literal zero since it can be implicitly converted to a pointer + // type. nullptr constants will be caught by the other constructor which + // accepts a nullptr_t. + constexpr OnlyLiteralZero(MatchLiteralZero *) noexcept {} // NOLINT // Fails compilation when `nullptr` or integral type arguments other than // `int` are passed. This constructor doesn't accept `int` because literal `0` // has type `int`. Literal `0` arguments will be implicitly converted to // `std::nullptr_t` and accepted by the above constructor, while other `int` // arguments will fail to be converted and cause compilation failure. - template < - typename T, - typename = typename std::enable_if< - std::is_same::value || - (std::is_integral::value && !std::is_same::value)>::type, - typename = typename Fail::type> - OnlyLiteralZero(T); // NOLINT + template ::value || + (std::is_integral::value && + !std::is_same::value)>::type> + OnlyLiteralZero(T) { // NOLINT + static_assert(sizeof(T) < 0, "Only literal `0` is allowed."); + } }; enum class eq : value_type { @@ -163,18 +162,18 @@ class weak_equality // Comparisons friend constexpr bool operator==( - weak_equality v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_equality v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ == 0; } friend constexpr bool operator!=( - weak_equality v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_equality v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ != 0; } - friend constexpr bool operator==(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator==(compare_internal::OnlyLiteralZero, weak_equality v) noexcept { return 0 == v.value_; } - friend constexpr bool operator!=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator!=(compare_internal::OnlyLiteralZero, weak_equality v) noexcept { return 0 != v.value_; } @@ -214,18 +213,18 @@ class strong_equality } // Comparisons friend constexpr bool operator==( - strong_equality v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_equality v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ == 0; } friend constexpr bool operator!=( - strong_equality v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_equality v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ != 0; } - friend constexpr bool operator==(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator==(compare_internal::OnlyLiteralZero, strong_equality v) noexcept { return 0 == v.value_; } - friend constexpr bool operator!=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator!=(compare_internal::OnlyLiteralZero, strong_equality v) noexcept { return 0 != v.value_; } @@ -277,50 +276,50 @@ class partial_ordering } // Comparisons friend constexpr bool operator==( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.is_ordered() && v.value_ == 0; } friend constexpr bool operator!=( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return !v.is_ordered() || v.value_ != 0; } friend constexpr bool operator<( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.is_ordered() && v.value_ < 0; } friend constexpr bool operator<=( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.is_ordered() && v.value_ <= 0; } friend constexpr bool operator>( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.is_ordered() && v.value_ > 0; } friend constexpr bool operator>=( - partial_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + partial_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.is_ordered() && v.value_ >= 0; } - friend constexpr bool operator==(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator==(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return v.is_ordered() && 0 == v.value_; } - friend constexpr bool operator!=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator!=(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return !v.is_ordered() || 0 != v.value_; } - friend constexpr bool operator<(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return v.is_ordered() && 0 < v.value_; } - friend constexpr bool operator<=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<=(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return v.is_ordered() && 0 <= v.value_; } - friend constexpr bool operator>(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return v.is_ordered() && 0 > v.value_; } - friend constexpr bool operator>=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>=(compare_internal::OnlyLiteralZero, partial_ordering v) noexcept { return v.is_ordered() && 0 >= v.value_; } @@ -369,50 +368,50 @@ class weak_ordering } // Comparisons friend constexpr bool operator==( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ == 0; } friend constexpr bool operator!=( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ != 0; } friend constexpr bool operator<( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ < 0; } friend constexpr bool operator<=( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ <= 0; } friend constexpr bool operator>( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ > 0; } friend constexpr bool operator>=( - weak_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + weak_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ >= 0; } - friend constexpr bool operator==(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator==(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 == v.value_; } - friend constexpr bool operator!=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator!=(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 != v.value_; } - friend constexpr bool operator<(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 < v.value_; } - friend constexpr bool operator<=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<=(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 <= v.value_; } - friend constexpr bool operator>(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 > v.value_; } - friend constexpr bool operator>=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>=(compare_internal::OnlyLiteralZero, weak_ordering v) noexcept { return 0 >= v.value_; } @@ -468,50 +467,50 @@ class strong_ordering } // Comparisons friend constexpr bool operator==( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ == 0; } friend constexpr bool operator!=( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ != 0; } friend constexpr bool operator<( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ < 0; } friend constexpr bool operator<=( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ <= 0; } friend constexpr bool operator>( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ > 0; } friend constexpr bool operator>=( - strong_ordering v, compare_internal::OnlyLiteralZero<>) noexcept { + strong_ordering v, compare_internal::OnlyLiteralZero) noexcept { return v.value_ >= 0; } - friend constexpr bool operator==(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator==(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 == v.value_; } - friend constexpr bool operator!=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator!=(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 != v.value_; } - friend constexpr bool operator<(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 < v.value_; } - friend constexpr bool operator<=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator<=(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 <= v.value_; } - friend constexpr bool operator>(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 > v.value_; } - friend constexpr bool operator>=(compare_internal::OnlyLiteralZero<>, + friend constexpr bool operator>=(compare_internal::OnlyLiteralZero, strong_ordering v) noexcept { return 0 >= v.value_; } -- cgit v1.2.3 From dcaed1a05a813e95b65219e0d1f6a2a684e13028 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 8 Feb 2023 10:49:52 -0800 Subject: Add overrides to other functions which call Waiter::GetWaiter PiperOrigin-RevId: 508124592 Change-Id: Ib183e6e241c81b2760e7f849f8af8e7e2c30ea42 --- .../internal/create_thread_identity.cc | 4 ++++ absl/synchronization/internal/per_thread_sem.cc | 20 ++++++++++++-------- absl/synchronization/internal/per_thread_sem.h | 11 ++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/absl/synchronization/internal/create_thread_identity.cc b/absl/synchronization/internal/create_thread_identity.cc index 44e6129b..f1ddbbb9 100644 --- a/absl/synchronization/internal/create_thread_identity.cc +++ b/absl/synchronization/internal/create_thread_identity.cc @@ -17,6 +17,7 @@ // This file is a no-op if the required LowLevelAlloc support is missing. #include "absl/base/internal/low_level_alloc.h" +#include "absl/synchronization/internal/waiter.h" #ifndef ABSL_LOW_LEVEL_ALLOC_MISSING #include @@ -71,6 +72,9 @@ static intptr_t RoundUp(intptr_t addr, intptr_t align) { void OneTimeInitThreadIdentity(base_internal::ThreadIdentity* identity) { PerThreadSem::Init(identity); + identity->ticker.store(0, std::memory_order_relaxed); + identity->wait_start.store(0, std::memory_order_relaxed); + identity->is_idle.store(false, std::memory_order_relaxed); } static void ResetThreadIdentityBetweenReuse( diff --git a/absl/synchronization/internal/per_thread_sem.cc b/absl/synchronization/internal/per_thread_sem.cc index 469e8f32..c9b8dc1e 100644 --- a/absl/synchronization/internal/per_thread_sem.cc +++ b/absl/synchronization/internal/per_thread_sem.cc @@ -40,13 +40,6 @@ std::atomic *PerThreadSem::GetThreadBlockedCounter() { return identity->blocked_count_ptr; } -void PerThreadSem::Init(base_internal::ThreadIdentity *identity) { - new (Waiter::GetWaiter(identity)) Waiter(); - identity->ticker.store(0, std::memory_order_relaxed); - identity->wait_start.store(0, std::memory_order_relaxed); - identity->is_idle.store(false, std::memory_order_relaxed); -} - void PerThreadSem::Tick(base_internal::ThreadIdentity *identity) { const int ticker = identity->ticker.fetch_add(1, std::memory_order_relaxed) + 1; @@ -54,7 +47,7 @@ void PerThreadSem::Tick(base_internal::ThreadIdentity *identity) { const bool is_idle = identity->is_idle.load(std::memory_order_relaxed); if (wait_start && (ticker - wait_start > Waiter::kIdlePeriods) && !is_idle) { // Wakeup the waiting thread since it is time for it to become idle. - Waiter::GetWaiter(identity)->Poke(); + ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPoke)(identity); } } @@ -64,11 +57,22 @@ ABSL_NAMESPACE_END extern "C" { +ABSL_ATTRIBUTE_WEAK void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemInit)( + absl::base_internal::ThreadIdentity *identity) { + new (absl::synchronization_internal::Waiter::GetWaiter(identity)) + absl::synchronization_internal::Waiter(); +} + ABSL_ATTRIBUTE_WEAK void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPost)( absl::base_internal::ThreadIdentity *identity) { absl::synchronization_internal::Waiter::GetWaiter(identity)->Post(); } +ABSL_ATTRIBUTE_WEAK void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPoke)( + absl::base_internal::ThreadIdentity *identity) { + absl::synchronization_internal::Waiter::GetWaiter(identity)->Poke(); +} + ABSL_ATTRIBUTE_WEAK bool ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemWait)( absl::synchronization_internal::KernelTimeout t) { bool timeout = false; diff --git a/absl/synchronization/internal/per_thread_sem.h b/absl/synchronization/internal/per_thread_sem.h index 90a88809..144ab3cd 100644 --- a/absl/synchronization/internal/per_thread_sem.h +++ b/absl/synchronization/internal/per_thread_sem.h @@ -64,7 +64,7 @@ class PerThreadSem { private: // Create the PerThreadSem associated with "identity". Initializes count=0. // REQUIRES: May only be called by ThreadIdentity. - static void Init(base_internal::ThreadIdentity* identity); + static inline void Init(base_internal::ThreadIdentity* identity); // Increments "identity"'s count. static inline void Post(base_internal::ThreadIdentity* identity); @@ -91,12 +91,21 @@ ABSL_NAMESPACE_END // By changing our extension points to be extern "C", we dodge this // check. extern "C" { +void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemInit)( + absl::base_internal::ThreadIdentity* identity); void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPost)( absl::base_internal::ThreadIdentity* identity); bool ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemWait)( absl::synchronization_internal::KernelTimeout t); +void ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPoke)( + absl::base_internal::ThreadIdentity* identity); } // extern "C" +void absl::synchronization_internal::PerThreadSem::Init( + absl::base_internal::ThreadIdentity* identity) { + ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemInit)(identity); +} + void absl::synchronization_internal::PerThreadSem::Post( absl::base_internal::ThreadIdentity* identity) { ABSL_INTERNAL_C_SYMBOL(AbslInternalPerThreadSemPost)(identity); -- cgit v1.2.3 From 823b837839db2a754af37095c72f97ce6733d32b Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Thu, 9 Feb 2023 14:12:52 -0800 Subject: In sanitizer mode, detect when end iterators from different swisstables are compared. We change AssertSameContainer to be after AssertIsValidForComparison calls so that we can have more specific failure messages. PiperOrigin-RevId: 508472485 Change-Id: Iff2f7512086948a4aca7fd403596e8d4fde53b2a --- absl/container/internal/raw_hash_set.h | 37 ++++++++++++++--- absl/container/internal/raw_hash_set_test.cc | 62 ++++++++++++++++++++-------- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 65dc66c2..a2ada0cb 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -853,6 +853,9 @@ class HashSetIteratorGenerationInfoEnabled { void set_generation_ptr(const GenerationType* ptr) { generation_ptr_ = ptr; } private: + // TODO(b/254649633): use a different static generation for default + // constructed iterators so that we can detect when default constructed + // iterators are compared to iterators from empty tables. const GenerationType* generation_ptr_ = EmptyGeneration(); GenerationType generation_ = *generation_ptr_; }; @@ -1087,12 +1090,33 @@ inline bool AreItersFromSameContainer(const ctrl_t* ctrl_a, // Asserts that two iterators come from the same container. // Note: we take slots by reference so that it's not UB if they're uninitialized // as long as we don't read them (when ctrl is null). -// TODO(b/254649633): when generations are enabled, we can detect more cases of -// different containers by comparing the pointers to the generations - this -// can cover cases of end iterators that we would otherwise miss. inline void AssertSameContainer(const ctrl_t* ctrl_a, const ctrl_t* ctrl_b, const void* const& slot_a, - const void* const& slot_b) { + const void* const& slot_b, + const GenerationType* generation_ptr_a, + const GenerationType* generation_ptr_b) { + if (SwisstableGenerationsEnabled() && generation_ptr_a != generation_ptr_b) { + // TODO(b/254649633): use a different static generation for default + // constructed iterators so that we can split the empty/default cases. + const bool a_is_empty_or_default = generation_ptr_a == EmptyGeneration(); + const bool b_is_empty_or_default = generation_ptr_b == EmptyGeneration(); + if (a_is_empty_or_default != b_is_empty_or_default) { + ABSL_INTERNAL_LOG(FATAL, + "Invalid iterator comparison. Comparing iterator from " + "a non-empty hashtable with an iterator from an empty " + "hashtable or a default-constructed iterator."); + } + const bool a_is_end = ctrl_a == nullptr; + const bool b_is_end = ctrl_b == nullptr; + if (a_is_end || b_is_end) { + ABSL_INTERNAL_LOG(FATAL, + "Invalid iterator comparison. Comparing iterator with " + "an end() iterator from a different hashtable."); + } + ABSL_INTERNAL_LOG(FATAL, + "Invalid iterator comparison. Comparing non-end() " + "iterators from different hashtables."); + } ABSL_HARDENING_ASSERT( AreItersFromSameContainer(ctrl_a, ctrl_b, slot_a, slot_b) && "Invalid iterator comparison. The iterators may be from different " @@ -1463,9 +1487,10 @@ class raw_hash_set { } friend bool operator==(const iterator& a, const iterator& b) { - AssertSameContainer(a.ctrl_, b.ctrl_, a.slot_, b.slot_); AssertIsValidForComparison(a.ctrl_, a.generation(), a.generation_ptr()); AssertIsValidForComparison(b.ctrl_, b.generation(), b.generation_ptr()); + AssertSameContainer(a.ctrl_, b.ctrl_, a.slot_, b.slot_, + a.generation_ptr(), b.generation_ptr()); return a.ctrl_ == b.ctrl_; } friend bool operator!=(const iterator& a, const iterator& b) { @@ -1499,6 +1524,8 @@ class raw_hash_set { if (ABSL_PREDICT_FALSE(*ctrl_ == ctrl_t::kSentinel)) ctrl_ = nullptr; } + // TODO(b/254649633): use non-null control for default-constructed iterators + // so that we can distinguish between them and end iterators in debug mode. ctrl_t* ctrl_ = nullptr; // To avoid uninitialized member warnings, put slot_ in an anonymous union. // The member is not initialized on singleton and end iterators. diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index e33fda20..f7ec1456 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -2078,6 +2078,19 @@ TEST(TableDeathTest, InvalidIteratorAsserts) { "erased or .*the table might have rehashed."); } +// Invalid iterator use can trigger heap-use-after-free in asan, +// use-of-uninitialized-value in msan, or invalidated iterator assertions. +constexpr const char* kInvalidIteratorDeathMessage = + "heap-use-after-free|use-of-uninitialized-value|invalidated " + "iterator|Invalid iterator"; + +// MSVC doesn't support | in regex. +#if defined(_MSC_VER) +constexpr bool kMsvc = true; +#else +constexpr bool kMsvc = false; +#endif + TEST(TableDeathTest, IteratorInvalidAssertsEqualityOperator) { if (!IsAssertEnabled()) GTEST_SKIP() << "Assertions not enabled."; @@ -2105,15 +2118,18 @@ TEST(TableDeathTest, IteratorInvalidAssertsEqualityOperator) { iter1 = t1.begin(); iter2 = t2.begin(); const char* const kContainerDiffDeathMessage = - "Invalid iterator comparison. The iterators may be from different " - ".*containers or the container might have rehashed."; + SwisstableGenerationsEnabled() + ? "Invalid iterator comparison.*non-end" + : "Invalid iterator comparison. The iterators may be from different " + ".*containers or the container might have rehashed."; EXPECT_DEATH_IF_SUPPORTED(void(iter1 == iter2), kContainerDiffDeathMessage); EXPECT_DEATH_IF_SUPPORTED(void(iter2 == iter1), kContainerDiffDeathMessage); for (int i = 0; i < 10; ++i) t1.insert(i); // There should have been a rehash in t1. + if (kMsvc) return; // MSVC doesn't support | in regex. EXPECT_DEATH_IF_SUPPORTED(void(iter1 == t1.begin()), - kContainerDiffDeathMessage); + kInvalidIteratorDeathMessage); } #if defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) @@ -2258,21 +2274,9 @@ TEST(Table, AlignOne) { } } -// Invalid iterator use can trigger heap-use-after-free in asan, -// use-of-uninitialized-value in msan, or invalidated iterator assertions. -constexpr const char* kInvalidIteratorDeathMessage = - "heap-use-after-free|use-of-uninitialized-value|invalidated " - "iterator|Invalid iterator"; - -#if defined(__clang__) && defined(_MSC_VER) -constexpr bool kLexan = true; -#else -constexpr bool kLexan = false; -#endif - TEST(Iterator, InvalidUseCrashesWithSanitizers) { if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; - if (kLexan) GTEST_SKIP() << "Lexan doesn't support | in regexp."; + if (kMsvc) GTEST_SKIP() << "MSVC doesn't support | in regexp."; IntTable t; // Start with 1 element so that `it` is never an end iterator. @@ -2288,7 +2292,7 @@ TEST(Iterator, InvalidUseCrashesWithSanitizers) { TEST(Iterator, InvalidUseWithReserveCrashesWithSanitizers) { if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; - if (kLexan) GTEST_SKIP() << "Lexan doesn't support | in regexp."; + if (kMsvc) GTEST_SKIP() << "MSVC doesn't support | in regexp."; IntTable t; t.reserve(10); @@ -2349,6 +2353,30 @@ TEST(Table, InvalidReferenceUseCrashesWithSanitizers) { } } +TEST(Iterator, InvalidComparisonDifferentTables) { + if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled."; + + IntTable t1, t2; + IntTable::iterator default_constructed_iter; + // TODO(b/254649633): Currently, we can't detect when end iterators from + // different empty tables are compared. If we allocate generations separately + // from control bytes, then we could do so. + // EXPECT_DEATH_IF_SUPPORTED(void(t1.end() == t2.end()), + // "Invalid iterator comparison.*empty hashtable"); + // EXPECT_DEATH_IF_SUPPORTED(void(t1.end() == default_constructed_iter), + // "Invalid iterator comparison.*default-const..."); + t1.insert(0); + EXPECT_DEATH_IF_SUPPORTED(void(t1.begin() == t2.end()), + "Invalid iterator comparison.*empty hashtable"); + EXPECT_DEATH_IF_SUPPORTED(void(t1.begin() == default_constructed_iter), + "Invalid iterator comparison.*default-constructed"); + t2.insert(0); + EXPECT_DEATH_IF_SUPPORTED(void(t1.begin() == t2.end()), + "Invalid iterator comparison.*end.. iterator"); + EXPECT_DEATH_IF_SUPPORTED(void(t1.begin() == t2.begin()), + "Invalid iterator comparison.*non-end"); +} + } // namespace } // namespace container_internal ABSL_NAMESPACE_END -- cgit v1.2.3 From fa4855403cdfb91b9c16f792041258ed819594c1 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Thu, 9 Feb 2023 19:45:05 -0800 Subject: Rewrite KernelTimeout to support both absolute and relative timeouts APIs that take KernelTimeout as a parameter can now query if an absolute or relative timeout was requested. If the underlying API can only use one type of timeout, the code will do a reasonable conversion. The goal is to eventually enable the possibility of using wait times that are based on monotonic clocks that are safe against system clock steps. PiperOrigin-RevId: 508541507 Change-Id: Id08bf13515f3e1bfd78d88393cde98a6fd3ef72c --- CMake/AbseilDll.cmake | 1 + absl/synchronization/BUILD.bazel | 16 +- absl/synchronization/CMakeLists.txt | 18 +- absl/synchronization/internal/kernel_timeout.cc | 168 +++++++++++++ absl/synchronization/internal/kernel_timeout.h | 180 +++++-------- .../internal/kernel_timeout_test.cc | 278 +++++++++++++++++++++ 6 files changed, 546 insertions(+), 115 deletions(-) create mode 100644 absl/synchronization/internal/kernel_timeout.cc create mode 100644 absl/synchronization/internal/kernel_timeout_test.cc diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index 52a563cd..3c0f6901 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -365,6 +365,7 @@ set(ABSL_INTERNAL_DLL_FILES "synchronization/internal/graphcycles.cc" "synchronization/internal/graphcycles.h" "synchronization/internal/kernel_timeout.h" + "synchronization/internal/kernel_timeout.cc" "synchronization/internal/per_thread_sem.cc" "synchronization/internal/per_thread_sem.h" "synchronization/internal/thread_pool.h" diff --git a/absl/synchronization/BUILD.bazel b/absl/synchronization/BUILD.bazel index ccaee796..9c89ac49 100644 --- a/absl/synchronization/BUILD.bazel +++ b/absl/synchronization/BUILD.bazel @@ -53,6 +53,7 @@ cc_library( cc_library( name = "kernel_timeout_internal", + srcs = ["internal/kernel_timeout.cc"], hdrs = ["internal/kernel_timeout.h"], copts = ABSL_DEFAULT_COPTS, linkopts = ABSL_DEFAULT_LINKOPTS, @@ -60,12 +61,25 @@ cc_library( "//absl/synchronization:__pkg__", ], deps = [ - "//absl/base:core_headers", + "//absl/base:config", "//absl/base:raw_logging_internal", "//absl/time", ], ) +cc_test( + name = "kernel_timeout_internal_test", + srcs = ["internal/kernel_timeout_test.cc"], + copts = ABSL_TEST_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + deps = [ + ":kernel_timeout_internal", + "//absl/base:config", + "//absl/time", + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "synchronization", srcs = [ diff --git a/absl/synchronization/CMakeLists.txt b/absl/synchronization/CMakeLists.txt index f64653bb..5eb9b566 100644 --- a/absl/synchronization/CMakeLists.txt +++ b/absl/synchronization/CMakeLists.txt @@ -39,14 +39,30 @@ absl_cc_library( kernel_timeout_internal HDRS "internal/kernel_timeout.h" + SRCS + "internal/kernel_timeout.cc" COPTS ${ABSL_DEFAULT_COPTS} DEPS - absl::core_headers + absl::config absl::raw_logging_internal absl::time ) +absl_cc_test( + NAME + kernel_timeout_internal_test + SRCS + "internal/kernel_timeout_test.cc" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::kernel_timeout_internal + absl::config + absl::time + GTest::gmock_main +) + absl_cc_library( NAME synchronization diff --git a/absl/synchronization/internal/kernel_timeout.cc b/absl/synchronization/internal/kernel_timeout.cc new file mode 100644 index 00000000..4015bd0c --- /dev/null +++ b/absl/synchronization/internal/kernel_timeout.cc @@ -0,0 +1,168 @@ +// Copyright 2023 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/synchronization/internal/kernel_timeout.h" + +#include +#include +#include +#include + +#include "absl/base/config.h" +#include "absl/time/time.h" + +namespace absl { +ABSL_NAMESPACE_BEGIN +namespace synchronization_internal { + +#ifdef ABSL_INTERNAL_NEED_REDUNDANT_CONSTEXPR_DECL +constexpr uint64_t KernelTimeout::kNoTimeout; +constexpr int64_t KernelTimeout::kMaxNanos; +#endif + +KernelTimeout::KernelTimeout(absl::Time t) { + // `absl::InfiniteFuture()` is a common "no timeout" value and cheaper to + // compare than convert. + if (t == absl::InfiniteFuture()) { + rep_ = kNoTimeout; + return; + } + + int64_t unix_nanos = absl::ToUnixNanos(t); + + // A timeout that lands before the unix epoch is converted to 0. + // In theory implementations should expire these timeouts immediately. + if (unix_nanos < 0) { + unix_nanos = 0; + } + + // Values greater than or equal to kMaxNanos are converted to infinite. + if (unix_nanos >= kMaxNanos) { + rep_ = kNoTimeout; + return; + } + + rep_ = static_cast(unix_nanos) << 1; +} + +KernelTimeout::KernelTimeout(absl::Duration d) { + // `absl::InfiniteDuration()` is a common "no timeout" value and cheaper to + // compare than convert. + if (d == absl::InfiniteDuration()) { + rep_ = kNoTimeout; + return; + } + + int64_t nanos = absl::ToInt64Nanoseconds(d); + + // Negative durations are normalized to 0. + // In theory implementations should expire these timeouts immediately. + if (nanos < 0) { + nanos = 0; + } + + // Values greater than or equal to kMaxNanos are converted to infinite. + if (nanos >= kMaxNanos) { + rep_ = kNoTimeout; + return; + } + + rep_ = (static_cast(nanos) << 1) | uint64_t{1}; +} + +int64_t KernelTimeout::MakeAbsNanos() const { + if (!has_timeout()) { + return kMaxNanos; + } + + int64_t nanos = RawNanos(); + + if (is_relative_timeout()) { + int64_t now = ToUnixNanos(absl::Now()); + if (nanos > kMaxNanos - now) { + // Overflow. + nanos = kMaxNanos; + } else { + nanos += now; + } + } else if (nanos == 0) { + // Some callers have assumed that 0 means no timeout, so instead we return a + // time of 1 nanosecond after the epoch. + nanos = 1; + } + + return nanos; +} + +struct timespec KernelTimeout::MakeAbsTimespec() const { + return absl::ToTimespec(absl::Nanoseconds(MakeAbsNanos())); +} + +struct timespec KernelTimeout::MakeRelativeTimespec() const { + if (!has_timeout()) { + return absl::ToTimespec(absl::Nanoseconds(kMaxNanos)); + } + if (is_relative_timeout()) { + return absl::ToTimespec(absl::Nanoseconds(RawNanos())); + } + + int64_t nanos = RawNanos(); + int64_t now = ToUnixNanos(absl::Now()); + if (now > nanos) { + // Convert past values to 0 to be safe. + nanos = 0; + } else { + nanos -= now; + } + return absl::ToTimespec(absl::Nanoseconds(nanos)); +} + +KernelTimeout::DWord KernelTimeout::InMillisecondsFromNow() const { + constexpr DWord kInfinite = std::numeric_limits::max(); + + if (!has_timeout()) { + return kInfinite; + } + + const int64_t nanos = RawNanos(); + constexpr uint64_t kNanosInMillis = uint64_t{1000000}; + + if (is_relative_timeout()) { + uint64_t ms = static_cast(nanos) / kNanosInMillis; + if (ms > static_cast(kInfinite)) { + ms = static_cast(kInfinite); + } + return static_cast(ms); + } + + int64_t now = ToUnixNanos(absl::Now()); + if (nanos >= now) { + // Round up so that now + ms_from_now >= nanos. + constexpr uint64_t kMaxValueNanos = + std::numeric_limits::max() - kNanosInMillis + 1; + uint64_t ms_from_now = + (std::min(kMaxValueNanos, static_cast(nanos - now)) + + kNanosInMillis - 1) / + kNanosInMillis; + if (ms_from_now > kInfinite) { + return kInfinite; + } + return static_cast(ms_from_now); + } + return DWord{0}; +} + +} // namespace synchronization_internal +ABSL_NAMESPACE_END +} // namespace absl diff --git a/absl/synchronization/internal/kernel_timeout.h b/absl/synchronization/internal/kernel_timeout.h index f5c2c0ef..1f4d82cd 100644 --- a/absl/synchronization/internal/kernel_timeout.h +++ b/absl/synchronization/internal/kernel_timeout.h @@ -11,26 +11,16 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -// - -// An optional absolute timeout, with nanosecond granularity, -// compatible with absl::Time. Suitable for in-register -// parameter-passing (e.g. syscalls.) -// Constructible from a absl::Time (for a timeout to be respected) or {} -// (for "no timeout".) -// This is a private low-level API for use by a handful of low-level -// components. Higher-level components should build APIs based on -// absl::Time and absl::Duration. #ifndef ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ #define ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ -#include - #include #include +#include #include +#include "absl/base/config.h" #include "absl/base/internal/raw_logging.h" #include "absl/time/clock.h" #include "absl/time/time.h" @@ -41,56 +31,59 @@ namespace synchronization_internal { class Waiter; +// An optional timeout, with nanosecond granularity. +// +// This is a private low-level API for use by a handful of low-level +// components. Higher-level components should build APIs based on +// absl::Time and absl::Duration. class KernelTimeout { public: - // A timeout that should expire at . Any value, in the full - // InfinitePast() to InfiniteFuture() range, is valid here and will be - // respected. - explicit KernelTimeout(absl::Time t) : ns_(MakeNs(t)) {} - // No timeout. - KernelTimeout() : ns_(0) {} + // Construct an absolute timeout that should expire at `t`. + explicit KernelTimeout(absl::Time t); + + // Construct a relative timeout that should expire after `d`. + explicit KernelTimeout(absl::Duration d); + + // Infinite timeout. + constexpr KernelTimeout() : rep_(kNoTimeout) {} + + // A more explicit factory for those who prefer it. + // Equivalent to `KernelTimeout()`. + static constexpr KernelTimeout Never() { return KernelTimeout(); } - // A more explicit factory for those who prefer it. Equivalent to {}. - static KernelTimeout Never() { return {}; } + // Returns true if there is a timeout that will eventually expire. + // Returns false if the timeout is infinite. + bool has_timeout() const { return rep_ != kNoTimeout; } - // We explicitly do not support other custom formats: timespec, int64_t nanos. - // Unify on this and absl::Time, please. + // If `has_timeout()` is true, returns true if the timeout was provided as an + // `absl::Time`. The return value is undefined if `has_timeout()` is false + // because all indefinite timeouts are equivalent. + bool is_absolute_timeout() const { return (rep_ & 1) == 0; } - bool has_timeout() const { return ns_ != 0; } + // If `has_timeout()` is true, returns true if the timeout was provided as an + // `absl::Duration`. The return value is undefined if `has_timeout()` is false + // because all indefinite timeouts are equivalent. + bool is_relative_timeout() const { return (rep_ & 1) == 1; } - // Convert to parameter for sem_timedwait/futex/similar. Only for approved - // users. Do not call if !has_timeout. + // Convert to `struct timespec` for interfaces that expect an absolute + // timeout. If !has_timeout() or is_relative_timeout(), attempts to convert to + // a reasonable absolute timeout, but callers should to test has_timeout() and + // is_relative_timeout() and prefer to use a more appropriate interface. struct timespec MakeAbsTimespec() const; - // Convert to unix epoch nanos. Do not call if !has_timeout. + // Convert to `struct timespec` for interfaces that expect a relative + // timeout. If !has_timeout() or is_absolute_timeout(), attempts to convert to + // a reasonable relative timeout, but callers should to test has_timeout() and + // is_absolute_timeout() and prefer to use a more appropriate interface. + struct timespec MakeRelativeTimespec() const; + + // Convert to unix epoch nanos for interfaces that expect an absolute timeout + // in nanoseconds. If !has_timeout() or is_relative_timeout(), attempts to + // convert to a reasonable absolute timeout, but callers should to test + // has_timeout() and is_relative_timeout() and prefer to use a more + // appropriate interface. int64_t MakeAbsNanos() const; - private: - // internal rep, not user visible: ns after unix epoch. - // zero = no timeout. - // Negative we treat as an unlikely (and certainly expired!) but valid - // timeout. - int64_t ns_; - - static int64_t MakeNs(absl::Time t) { - // optimization--InfiniteFuture is common "no timeout" value - // and cheaper to compare than convert. - if (t == absl::InfiniteFuture()) return 0; - int64_t x = ToUnixNanos(t); - - // A timeout that lands exactly on the epoch (x=0) needs to be respected, - // so we alter it unnoticably to 1. Negative timeouts are in - // theory supported, but handled poorly by the kernel (long - // delays) so push them forward too; since all such times have - // already passed, it's indistinguishable. - if (x <= 0) x = 1; - // A time larger than what can be represented to the kernel is treated - // as no timeout. - if (x == (std::numeric_limits::max)()) x = 0; - return x; - } - -#ifdef _WIN32 // Converts to milliseconds from now, or INFINITE when // !has_timeout(). For use by SleepConditionVariableSRW on // Windows. Callers should recognize that the return value is a @@ -100,68 +93,29 @@ class KernelTimeout { // so we define our own DWORD and INFINITE instead of getting them from // and . typedef unsigned long DWord; // NOLINT - DWord InMillisecondsFromNow() const { - constexpr DWord kInfinite = (std::numeric_limits::max)(); - if (!has_timeout()) { - return kInfinite; - } - // The use of absl::Now() to convert from absolute time to - // relative time means that absl::Now() cannot use anything that - // depends on KernelTimeout (for example, Mutex) on Windows. - int64_t now = ToUnixNanos(absl::Now()); - if (ns_ >= now) { - // Round up so that Now() + ms_from_now >= ns_. - constexpr uint64_t max_nanos = - (std::numeric_limits::max)() - 999999u; - uint64_t ms_from_now = - ((std::min)(max_nanos, static_cast(ns_ - now)) + 999999u) / - 1000000u; - if (ms_from_now > kInfinite) { - return kInfinite; - } - return static_cast(ms_from_now); - } - return 0; - } - - friend class Waiter; -#endif -}; + DWord InMillisecondsFromNow() const; -inline struct timespec KernelTimeout::MakeAbsTimespec() const { - int64_t n = ns_; - static const int64_t kNanosPerSecond = 1000 * 1000 * 1000; - if (n == 0) { - ABSL_RAW_LOG( - ERROR, "Tried to create a timespec from a non-timeout; never do this."); - // But we'll try to continue sanely. no-timeout ~= saturated timeout. - n = (std::numeric_limits::max)(); - } - - // Kernel APIs validate timespecs as being at or after the epoch, - // despite the kernel time type being signed. However, no one can - // tell the difference between a timeout at or before the epoch (since - // all such timeouts have expired!) - if (n < 0) n = 0; - - struct timespec abstime; - int64_t seconds = (std::min)(n / kNanosPerSecond, - int64_t{(std::numeric_limits::max)()}); - abstime.tv_sec = static_cast(seconds); - abstime.tv_nsec = static_cast(n % kNanosPerSecond); - return abstime; -} - -inline int64_t KernelTimeout::MakeAbsNanos() const { - if (ns_ == 0) { - ABSL_RAW_LOG( - ERROR, "Tried to create a timeout from a non-timeout; never do this."); - // But we'll try to continue sanely. no-timeout ~= saturated timeout. - return (std::numeric_limits::max)(); - } - - return ns_; -} + private: + // Internal representation. + // - If the value is kNoTimeout, then the timeout is infinite, and + // has_timeout() will return true. + // - If the low bit is 0, then the high 63 bits is number of nanoseconds + // after the unix epoch. + // - If the low bit is 1, then the high 63 bits is a relative duration in + // nanoseconds. + uint64_t rep_; + + // Returns the number of nanoseconds stored in the internal representation. + // Together with is_absolute_timeout() and is_relative_timeout(), the return + // value is used to compute when the timeout should occur. + int64_t RawNanos() const { return static_cast(rep_ >> 1); } + + // A value that represents no timeout (or an infinite timeout). + static constexpr uint64_t kNoTimeout = (std::numeric_limits::max)(); + + // The maximum value that can be stored in the high 63 bits. + static constexpr int64_t kMaxNanos = (std::numeric_limits::max)(); +}; } // namespace synchronization_internal ABSL_NAMESPACE_END diff --git a/absl/synchronization/internal/kernel_timeout_test.cc b/absl/synchronization/internal/kernel_timeout_test.cc new file mode 100644 index 00000000..a89ae220 --- /dev/null +++ b/absl/synchronization/internal/kernel_timeout_test.cc @@ -0,0 +1,278 @@ +// Copyright 2023 The Abseil Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/synchronization/internal/kernel_timeout.h" + +#include + +#include "gtest/gtest.h" +#include "absl/base/config.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" + +namespace { + +#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \ + defined(ABSL_HAVE_MEMORY_SANITIZER) || \ + defined(ABSL_HAVE_THREAD_SANITIZER) || defined(__ANDROID__) +constexpr absl::Duration kTimingBound = absl::Milliseconds(5); +#else +constexpr absl::Duration kTimingBound = absl::Microseconds(250); +#endif + +using absl::synchronization_internal::KernelTimeout; + +TEST(KernelTimeout, FiniteTimes) { + constexpr absl::Duration kDurationsToTest[] = { + absl::ZeroDuration(), + absl::Nanoseconds(1), + absl::Microseconds(1), + absl::Milliseconds(1), + absl::Seconds(1), + absl::Minutes(1), + absl::Hours(1), + absl::Hours(1000), + -absl::Nanoseconds(1), + -absl::Microseconds(1), + -absl::Milliseconds(1), + -absl::Seconds(1), + -absl::Minutes(1), + -absl::Hours(1), + -absl::Hours(1000), + }; + + for (auto duration : kDurationsToTest) { + const absl::Time now = absl::Now(); + const absl::Time when = now + duration; + SCOPED_TRACE(duration); + KernelTimeout t(when); + EXPECT_TRUE(t.has_timeout()); + EXPECT_TRUE(t.is_absolute_timeout()); + EXPECT_FALSE(t.is_relative_timeout()); + EXPECT_EQ(absl::TimeFromTimespec(t.MakeAbsTimespec()), when); + // MakeRelativeTimespec() doesn't quite round trip when using an absolute + // time, but it should get pretty close. Past times are converted to zero + // durations. + EXPECT_LE( + absl::AbsDuration(absl::DurationFromTimespec(t.MakeRelativeTimespec()) - + std::max(duration, absl::ZeroDuration())), + kTimingBound); + EXPECT_EQ(absl::FromUnixNanos(t.MakeAbsNanos()), when); + EXPECT_LE(absl::AbsDuration(absl::Milliseconds(t.InMillisecondsFromNow()) - + std::max(duration, absl::ZeroDuration())), + absl::Milliseconds(5)); + } +} + +TEST(KernelTimeout, InfiniteFuture) { + KernelTimeout t(absl::InfiniteFuture()); + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, DefaultConstructor) { + // The default constructor is equivalent to absl::InfiniteFuture(). + KernelTimeout t; + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, TimeMaxNanos) { + // Time >= kMaxNanos should behave as no timeout. + KernelTimeout t(absl::FromUnixNanos(std::numeric_limits::max())); + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, Never) { + // KernelTimeout::Never() is equivalent to absl::InfiniteFuture(). + KernelTimeout t = KernelTimeout::Never(); + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, InfinitePast) { + KernelTimeout t(absl::InfinitePast()); + EXPECT_TRUE(t.has_timeout()); + EXPECT_TRUE(t.is_absolute_timeout()); + EXPECT_FALSE(t.is_relative_timeout()); + EXPECT_LE(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::FromUnixNanos(1)); + EXPECT_EQ(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::ZeroDuration()); + EXPECT_LE(absl::FromUnixNanos(t.MakeAbsNanos()), absl::FromUnixNanos(1)); + EXPECT_EQ(t.InMillisecondsFromNow(), KernelTimeout::DWord{0}); +} + +TEST(KernelTimeout, FiniteDurations) { + constexpr absl::Duration kDurationsToTest[] = { + absl::ZeroDuration(), + absl::Nanoseconds(1), + absl::Microseconds(1), + absl::Milliseconds(1), + absl::Seconds(1), + absl::Minutes(1), + absl::Hours(1), + absl::Hours(1000), + }; + + for (auto duration : kDurationsToTest) { + SCOPED_TRACE(duration); + KernelTimeout t(duration); + EXPECT_TRUE(t.has_timeout()); + EXPECT_FALSE(t.is_absolute_timeout()); + EXPECT_TRUE(t.is_relative_timeout()); + EXPECT_LE(absl::AbsDuration(absl::Now() + duration - + absl::TimeFromTimespec(t.MakeAbsTimespec())), + absl::Milliseconds(5)); + EXPECT_EQ(absl::DurationFromTimespec(t.MakeRelativeTimespec()), duration); + EXPECT_LE(absl::AbsDuration(absl::Now() + duration - + absl::FromUnixNanos(t.MakeAbsNanos())), + absl::Milliseconds(5)); + EXPECT_LE(absl::Milliseconds(t.InMillisecondsFromNow()) - duration, + absl::Milliseconds(5)); + } +} + +TEST(KernelTimeout, NegativeDurations) { + constexpr absl::Duration kDurationsToTest[] = { + -absl::ZeroDuration(), + -absl::Nanoseconds(1), + -absl::Microseconds(1), + -absl::Milliseconds(1), + -absl::Seconds(1), + -absl::Minutes(1), + -absl::Hours(1), + -absl::Hours(1000), + -absl::InfiniteDuration(), + }; + + for (auto duration : kDurationsToTest) { + // Negative durations should all be converted to zero durations or "now". + SCOPED_TRACE(duration); + KernelTimeout t(duration); + EXPECT_TRUE(t.has_timeout()); + EXPECT_FALSE(t.is_absolute_timeout()); + EXPECT_TRUE(t.is_relative_timeout()); + EXPECT_LE(absl::AbsDuration(absl::Now() - + absl::TimeFromTimespec(t.MakeAbsTimespec())), + absl::Milliseconds(5)); + EXPECT_EQ(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::ZeroDuration()); + EXPECT_LE( + absl::AbsDuration(absl::Now() - absl::FromUnixNanos(t.MakeAbsNanos())), + absl::Milliseconds(5)); + EXPECT_EQ(t.InMillisecondsFromNow(), KernelTimeout::DWord{0}); + } +} + +TEST(KernelTimeout, InfiniteDuration) { + KernelTimeout t(absl::InfiniteDuration()); + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, DurationMaxNanos) { + // Duration >= kMaxNanos should behave as no timeout. + KernelTimeout t(absl::Nanoseconds(std::numeric_limits::max())); + EXPECT_FALSE(t.has_timeout()); + // Callers are expected to check has_timeout() instead of using the methods + // below, but we do try to do something reasonable if they don't. We may not + // be able to round-trip back to absl::InfiniteDuration() or + // absl::InfiniteFuture(), but we should return a very large value. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_EQ(t.InMillisecondsFromNow(), + std::numeric_limits::max()); +} + +TEST(KernelTimeout, OverflowNanos) { + // Test what happens when KernelTimeout is constructed with an absl::Duration + // that would overflow now_nanos + duration. + int64_t now_nanos = absl::ToUnixNanos(absl::Now()); + int64_t limit = std::numeric_limits::max() - now_nanos; + absl::Duration duration = absl::Nanoseconds(limit) + absl::Seconds(1); + KernelTimeout t(duration); + EXPECT_TRUE(t.has_timeout()); + // Timeouts should still be far in the future. + EXPECT_GT(absl::TimeFromTimespec(t.MakeAbsTimespec()), + absl::Now() + absl::Hours(100000)); + EXPECT_GT(absl::DurationFromTimespec(t.MakeRelativeTimespec()), + absl::Hours(100000)); + EXPECT_GT(absl::FromUnixNanos(t.MakeAbsNanos()), + absl::Now() + absl::Hours(100000)); + EXPECT_LE(absl::Milliseconds(t.InMillisecondsFromNow()) - duration, + absl::Milliseconds(5)); +} + +} // namespace -- cgit v1.2.3 From cde2f0eaaed3fb8581511cb5719d39172a5a2d81 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 10 Feb 2023 06:31:03 -0800 Subject: Workaround MSan false positive. On Linux Kernels >= 5.4 MSan reports a false positive when accessing thread local storage data from loaded libraries. This was reported on Chromium (which on some build configurations uses absl as a dynamic library). More info here: crbug.com/1414573. PiperOrigin-RevId: 508645053 Change-Id: I5d5a97e1ee7230cc23f3934a4ec5594b883918b4 --- absl/container/BUILD.bazel | 1 + absl/container/CMakeLists.txt | 1 + absl/container/internal/raw_hash_set.cc | 6 ++++++ 3 files changed, 8 insertions(+) diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 96963c4b..038e5234 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel @@ -622,6 +622,7 @@ cc_library( ":hashtablez_sampler", "//absl/base:config", "//absl/base:core_headers", + "//absl/base:dynamic_annotations", "//absl/base:endian", "//absl/base:prefetch", "//absl/base:raw_logging_internal", diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index 9ba95b32..43d60b9d 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt @@ -706,6 +706,7 @@ absl_cc_library( absl::container_common absl::container_memory absl::core_headers + absl::dynamic_annotations absl::endian absl::hash absl::hash_policy_traits diff --git a/absl/container/internal/raw_hash_set.cc b/absl/container/internal/raw_hash_set.cc index 5dc8b2fa..a6d9b7c0 100644 --- a/absl/container/internal/raw_hash_set.cc +++ b/absl/container/internal/raw_hash_set.cc @@ -19,6 +19,7 @@ #include #include "absl/base/config.h" +#include "absl/base/dynamic_annotations.h" #include "absl/hash/hash.h" namespace absl { @@ -44,6 +45,11 @@ constexpr size_t Group::kWidth; inline size_t RandomSeed() { #ifdef ABSL_HAVE_THREAD_LOCAL static thread_local size_t counter = 0; + // On Linux kernels >= 5.4 the MSAN runtime has a false-positive when + // accessing thread local storage data from loaded libraries + // (https://github.com/google/sanitizers/issues/1265), for this reason counter + // needs to be annotated as initialized. + ABSL_ANNOTATE_MEMORY_IS_INITIALIZED(&counter, sizeof(size_t)); size_t value = ++counter; #else // ABSL_HAVE_THREAD_LOCAL static std::atomic counter(0); -- cgit v1.2.3 From 2312dbb66cbcb9ffab2fdbc1672211091d2b5286 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 13 Feb 2023 08:34:13 -0800 Subject: Fix missing constexpr on GetReferenceableValue overload PiperOrigin-RevId: 509236105 Change-Id: I635f521aff106875f06bb93c332d3e437d0ad409 --- absl/log/internal/check_op.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/absl/log/internal/check_op.h b/absl/log/internal/check_op.h index 4907b89b..20b01b5e 100644 --- a/absl/log/internal/check_op.h +++ b/absl/log/internal/check_op.h @@ -371,7 +371,9 @@ inline constexpr unsigned short GetReferenceableValue( // NOLINT return t; } inline constexpr int GetReferenceableValue(int t) { return t; } -inline unsigned int GetReferenceableValue(unsigned int t) { return t; } +inline constexpr unsigned int GetReferenceableValue(unsigned int t) { + return t; +} inline constexpr long GetReferenceableValue(long t) { return t; } // NOLINT inline constexpr unsigned long GetReferenceableValue( // NOLINT unsigned long t) { // NOLINT -- cgit v1.2.3 From 19a44466c19f15b1e9134b10430a2530688b9a2e Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 13 Feb 2023 12:19:15 -0800 Subject: Fix absl/log:stripping_test when ABSL_MIN_LOG_LEVEL is defined The test was getting tripped by EXPECT_DEATH_IF_SUPPORTED keeping a literal copy of the test statement. PiperOrigin-RevId: 509299343 Change-Id: I63da29d844cc630d750535fd4bb13a2895d32541 --- absl/log/BUILD.bazel | 1 + absl/log/CMakeLists.txt | 1 + absl/log/stripping_test.cc | 10 +++++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/absl/log/BUILD.bazel b/absl/log/BUILD.bazel index e9e411ff..378e1f3b 100644 --- a/absl/log/BUILD.bazel +++ b/absl/log/BUILD.bazel @@ -546,6 +546,7 @@ cc_test( deps = [ ":check", ":log", + "//absl/base:log_severity", "//absl/base:strerror", "//absl/flags:program_name", "//absl/log/internal:test_helpers", diff --git a/absl/log/CMakeLists.txt b/absl/log/CMakeLists.txt index fb1b59f5..78adbf1d 100644 --- a/absl/log/CMakeLists.txt +++ b/absl/log/CMakeLists.txt @@ -1014,6 +1014,7 @@ absl_cc_test( absl::flags_program_name absl::log absl::log_internal_test_helpers + absl::log_severity absl::strerror absl::strings absl::str_format diff --git a/absl/log/stripping_test.cc b/absl/log/stripping_test.cc index d6a6606e..8b711ec7 100644 --- a/absl/log/stripping_test.cc +++ b/absl/log/stripping_test.cc @@ -49,6 +49,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/base/internal/strerror.h" +#include "absl/base/log_severity.h" #include "absl/flags/internal/program_name.h" #include "absl/log/check.h" #include "absl/log/internal/test_helpers.h" @@ -57,6 +58,10 @@ #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" +// Set a flag that controls whether we actually execute fatal statements, but +// prevent the compiler from optimizing it out. +static volatile bool kReallyDie = false; + namespace { using ::testing::_; using ::testing::Eq; @@ -304,7 +309,10 @@ TEST_F(StrippingTest, Fatal) { // as would happen if we used a literal. We might (or might not) leave it // lying around later; that's what the tests are for! const std::string needle = absl::Base64Escape("StrippingTest.Fatal"); - EXPECT_DEATH_IF_SUPPORTED(LOG(FATAL) << "U3RyaXBwaW5nVGVzdC5GYXRhbA==", ""); + // We don't care if the LOG statement is actually executed, we're just + // checking that it's stripped. + if (kReallyDie) LOG(FATAL) << "U3RyaXBwaW5nVGVzdC5GYXRhbA=="; + std::unique_ptr> exe = OpenTestExecutable(); ASSERT_THAT(exe, NotNull()); if (absl::LogSeverity::kFatal >= kAbslMinLogLevel) { -- cgit v1.2.3 From b7a8491fbd90b7de010b5e7f74e09996520f3e61 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Fri, 10 Feb 2023 13:19:30 -0500 Subject: Remove Workarounds for Old Clang Bug https://bugs.llvm.org/show_bug.cgi?id=38289 has been addressed since 2019. We just have to check to see if the clang being used to compile with is any version before major version 9. --- absl/numeric/int128.cc | 2 +- absl/strings/internal/str_format/float_conversion.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/absl/numeric/int128.cc b/absl/numeric/int128.cc index e5526c6f..e0e3bce8 100644 --- a/absl/numeric/int128.cc +++ b/absl/numeric/int128.cc @@ -111,7 +111,7 @@ uint128 MakeUint128FromFloat(T v) { return MakeUint128(0, static_cast(v)); } -#if defined(__clang__) && !defined(__SSE3__) +#if defined(__clang__) && (__clang_major__ < 9) && !defined(__SSE3__) // Workaround for clang bug: https://bugs.llvm.org/show_bug.cgi?id=38289 // Casting from long double to uint64_t is miscompiled and drops bits. // It is more work, so only use when we need the workaround. diff --git a/absl/strings/internal/str_format/float_conversion.cc b/absl/strings/internal/str_format/float_conversion.cc index 8e497852..fe0ad4bd 100644 --- a/absl/strings/internal/str_format/float_conversion.cc +++ b/absl/strings/internal/str_format/float_conversion.cc @@ -1102,7 +1102,7 @@ void PrintExponent(int exp, char e, Buffer *out) { template constexpr bool CanFitMantissa() { return -#if defined(__clang__) && !defined(__SSE3__) +#if defined(__clang__) && (__clang_major__ < 9) && !defined(__SSE3__) // Workaround for clang bug: https://bugs.llvm.org/show_bug.cgi?id=38289 // Casting from long double to uint64_t is miscompiled and drops bits. (!std::is_same::value || -- cgit v1.2.3 From 15957950d997cd9e09c61add5200361d5d1e7e11 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Tue, 14 Feb 2023 12:30:41 -0800 Subject: Make default-constructed swisstable iterators use EmptyGroup() for ctrl_ so that we can distinguish between end() iterators and default-constructed iterators in debug mode. PiperOrigin-RevId: 509606271 Change-Id: I77b68590b3904a4cf7809b75d814d74cb89603b6 --- absl/container/internal/raw_hash_set.h | 53 +++++++++++++++++----------- absl/container/internal/raw_hash_set_test.cc | 21 +++++------ 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index a2ada0cb..33e05736 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -816,6 +816,10 @@ class CommonFieldsGenerationInfoEnabled { // the code more complicated, and there's a benefit in having the sizes of // raw_hash_set in sanitizer mode and non-sanitizer mode a bit more different, // which is that tests are less likely to rely on the size remaining the same. + // TODO(b/254649633): Currently, we can't detect when end iterators from + // different empty tables are compared. If we allocate generations separately + // from control bytes, then we could do so. Another option would be to have N + // empty generations and use a random one for empty hashtables. GenerationType* generation_ = EmptyGeneration(); }; @@ -853,9 +857,6 @@ class HashSetIteratorGenerationInfoEnabled { void set_generation_ptr(const GenerationType* ptr) { generation_ptr_ = ptr; } private: - // TODO(b/254649633): use a different static generation for default - // constructed iterators so that we can detect when default constructed - // iterators are compared to iterators from empty tables. const GenerationType* generation_ptr_ = EmptyGeneration(); GenerationType generation_ = *generation_ptr_; }; @@ -1040,10 +1041,10 @@ size_t SelectBucketCountForIterRange(InputIter first, InputIter last, #define ABSL_INTERNAL_ASSERT_IS_FULL(ctrl, generation, generation_ptr, \ operation) \ do { \ - ABSL_HARDENING_ASSERT( \ - (ctrl != nullptr) && operation \ - " called on invalid iterator. The iterator might be an end() " \ - "iterator or may have been default constructed."); \ + ABSL_HARDENING_ASSERT((ctrl != nullptr) && operation \ + " called on end() iterator."); \ + ABSL_HARDENING_ASSERT((ctrl != EmptyGroup()) && operation \ + " called on default-constructed iterator."); \ if (SwisstableGenerationsEnabled() && generation != *generation_ptr) \ ABSL_INTERNAL_LOG(FATAL, operation \ " called on invalidated iterator. The table could " \ @@ -1058,9 +1059,10 @@ size_t SelectBucketCountForIterRange(InputIter first, InputIter last, inline void AssertIsValidForComparison(const ctrl_t* ctrl, GenerationType generation, const GenerationType* generation_ptr) { - ABSL_HARDENING_ASSERT((ctrl == nullptr || IsFull(*ctrl)) && - "Invalid iterator comparison. The element might have " - "been erased or the table might have rehashed."); + ABSL_HARDENING_ASSERT( + (ctrl == nullptr || ctrl == EmptyGroup() || IsFull(*ctrl)) && + "Invalid iterator comparison. The element might have " + "been erased or the table might have rehashed."); if (SwisstableGenerationsEnabled() && generation != *generation_ptr) { ABSL_INTERNAL_LOG(FATAL, "Invalid iterator comparison. The table could have " @@ -1095,16 +1097,27 @@ inline void AssertSameContainer(const ctrl_t* ctrl_a, const ctrl_t* ctrl_b, const void* const& slot_b, const GenerationType* generation_ptr_a, const GenerationType* generation_ptr_b) { +#if defined(ABSL_SWISSTABLE_ENABLE_GENERATIONS) || \ + ABSL_OPTION_HARDENED == 1 || !defined(NDEBUG) + const bool a_is_default = ctrl_a == EmptyGroup(); + const bool b_is_default = ctrl_b == EmptyGroup(); + if (a_is_default != b_is_default) { + ABSL_INTERNAL_LOG( + FATAL, + "Invalid iterator comparison. Comparing default-constructed iterator " + "with non-default-constructed iterator."); + } + if (a_is_default && b_is_default) return; +#endif + if (SwisstableGenerationsEnabled() && generation_ptr_a != generation_ptr_b) { - // TODO(b/254649633): use a different static generation for default - // constructed iterators so that we can split the empty/default cases. - const bool a_is_empty_or_default = generation_ptr_a == EmptyGeneration(); - const bool b_is_empty_or_default = generation_ptr_b == EmptyGeneration(); - if (a_is_empty_or_default != b_is_empty_or_default) { + const bool a_is_empty = generation_ptr_a == EmptyGeneration(); + const bool b_is_empty = generation_ptr_b == EmptyGeneration(); + if (a_is_empty != b_is_empty) { ABSL_INTERNAL_LOG(FATAL, "Invalid iterator comparison. Comparing iterator from " "a non-empty hashtable with an iterator from an empty " - "hashtable or a default-constructed iterator."); + "hashtable."); } const bool a_is_end = ctrl_a == nullptr; const bool b_is_end = ctrl_b == nullptr; @@ -1509,7 +1522,7 @@ class raw_hash_set { } // For end() iterators. explicit iterator(const GenerationType* generation_ptr) - : HashSetIteratorGenerationInfo(generation_ptr) {} + : HashSetIteratorGenerationInfo(generation_ptr), ctrl_(nullptr) {} // Fixes up `ctrl_` to point to a full by advancing it and `slot_` until // they reach one. @@ -1524,9 +1537,9 @@ class raw_hash_set { if (ABSL_PREDICT_FALSE(*ctrl_ == ctrl_t::kSentinel)) ctrl_ = nullptr; } - // TODO(b/254649633): use non-null control for default-constructed iterators - // so that we can distinguish between them and end iterators in debug mode. - ctrl_t* ctrl_ = nullptr; + // We use EmptyGroup() for default-constructed iterators so that they can + // be distinguished from end iterators, which have nullptr ctrl_. + ctrl_t* ctrl_ = EmptyGroup(); // To avoid uninitialized member warnings, put slot_ in an anonymous union. // The member is not initialized on singleton and end iterators. union { diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index f7ec1456..4f2d006d 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -2060,22 +2060,17 @@ TEST(TableDeathTest, InvalidIteratorAsserts) { IntTable t; // Extra simple "regexp" as regexp support is highly varied across platforms. - EXPECT_DEATH_IF_SUPPORTED( - t.erase(t.end()), - "erase.* called on invalid iterator. The iterator might be an " - "end.*iterator or may have been default constructed."); + EXPECT_DEATH_IF_SUPPORTED(t.erase(t.end()), + "erase.* called on end.. iterator."); typename IntTable::iterator iter; EXPECT_DEATH_IF_SUPPORTED( - ++iter, - "operator.* called on invalid iterator. The iterator might be an " - "end.*iterator or may have been default constructed."); + ++iter, "operator.* called on default-constructed iterator."); t.insert(0); iter = t.begin(); t.erase(iter); - EXPECT_DEATH_IF_SUPPORTED( - ++iter, - "operator.* called on invalid iterator. The element might have been " - "erased or .*the table might have rehashed."); + EXPECT_DEATH_IF_SUPPORTED(++iter, + "operator.* called on invalid iterator. The " + "element might have been erased"); } // Invalid iterator use can trigger heap-use-after-free in asan, @@ -2363,8 +2358,8 @@ TEST(Iterator, InvalidComparisonDifferentTables) { // from control bytes, then we could do so. // EXPECT_DEATH_IF_SUPPORTED(void(t1.end() == t2.end()), // "Invalid iterator comparison.*empty hashtable"); - // EXPECT_DEATH_IF_SUPPORTED(void(t1.end() == default_constructed_iter), - // "Invalid iterator comparison.*default-const..."); + EXPECT_DEATH_IF_SUPPORTED(void(t1.end() == default_constructed_iter), + "Invalid iterator comparison.*default-constructed"); t1.insert(0); EXPECT_DEATH_IF_SUPPORTED(void(t1.begin() == t2.end()), "Invalid iterator comparison.*empty hashtable"); -- cgit v1.2.3 From 8aa88cd11caa8dbfd5966e5e2f391b892cfd1c35 Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Tue, 14 Feb 2023 14:06:10 -0800 Subject: Remove _m_prefetchw() in favor of supporting only _mm_prefetch() or __builtin_prefetch() Supporting _m_prefetchw() (officially part of 3DNOW) across various compilers and platforms turns out to be difficult. This change removes the explicit _m_prefetchw call aimed at MSVC compilers given that it causes issues in clang / chromium compilations. PiperOrigin-RevId: 509632497 Change-Id: Ib1b6b2cf667cbc1af5ed6651cd9aa0294a9265b6 --- absl/base/prefetch.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/absl/base/prefetch.h b/absl/base/prefetch.h index a0b1efe6..de7a180d 100644 --- a/absl/base/prefetch.h +++ b/absl/base/prefetch.h @@ -34,7 +34,6 @@ (defined(_M_X64) || defined(_M_IX86)) #include #pragma intrinsic(_mm_prefetch) -#pragma intrinsic(_m_prefetchw) #endif namespace absl { @@ -176,10 +175,6 @@ inline void PrefetchToLocalCacheNta(const void* addr) { inline void PrefetchToLocalCacheForWrite(const void* addr) { #if defined(_MM_HINT_ET0) _mm_prefetch(reinterpret_cast(addr), _MM_HINT_ET0); -#elif defined(_MSC_VER) && _MSC_VER >= 1900 && \ - (defined(_M_X64) || defined(_M_IX86)) - // MSVC 2015 and up on x86/x64 supports prefetchw (feature listed as 3DNOW) - _m_prefetchw(const_cast(addr)); #elif !defined(_MSC_VER) && defined(__x86_64__) // _MM_HINT_ET0 is not universally supported. As we commented further // up, PREFETCHW is recognized as a no-op on older Intel processors -- cgit v1.2.3 From b54044578175b6949fb151b4d8be9153d7093c31 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Wed, 15 Feb 2023 07:40:18 -0800 Subject: KernelTimeout optimization: Use absl::GetCurrentTimeNanos() instead of absl::ToUnixNanos(absl::Now()); PiperOrigin-RevId: 509829866 Change-Id: Ib34362762304ad6eb7980a1227d717069b84f656 --- absl/synchronization/internal/kernel_timeout.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/absl/synchronization/internal/kernel_timeout.cc b/absl/synchronization/internal/kernel_timeout.cc index 4015bd0c..8d9e7d74 100644 --- a/absl/synchronization/internal/kernel_timeout.cc +++ b/absl/synchronization/internal/kernel_timeout.cc @@ -89,7 +89,7 @@ int64_t KernelTimeout::MakeAbsNanos() const { int64_t nanos = RawNanos(); if (is_relative_timeout()) { - int64_t now = ToUnixNanos(absl::Now()); + int64_t now = absl::GetCurrentTimeNanos(); if (nanos > kMaxNanos - now) { // Overflow. nanos = kMaxNanos; @@ -118,7 +118,7 @@ struct timespec KernelTimeout::MakeRelativeTimespec() const { } int64_t nanos = RawNanos(); - int64_t now = ToUnixNanos(absl::Now()); + int64_t now = absl::GetCurrentTimeNanos(); if (now > nanos) { // Convert past values to 0 to be safe. nanos = 0; @@ -146,7 +146,7 @@ KernelTimeout::DWord KernelTimeout::InMillisecondsFromNow() const { return static_cast(ms); } - int64_t now = ToUnixNanos(absl::Now()); + int64_t now = absl::GetCurrentTimeNanos(); if (nanos >= now) { // Round up so that now + ms_from_now >= nanos. constexpr uint64_t kMaxValueNanos = -- cgit v1.2.3 From 32794f0a4272bd4cc7ee5306999e9ff4180278d9 Mon Sep 17 00:00:00 2001 From: Milad Fa Date: Wed, 15 Feb 2023 19:05:44 +0000 Subject: Fix Read1To3 on big Endian --- absl/hash/internal/hash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index eb50e2c7..b1452c4c 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -1083,7 +1083,7 @@ class ABSL_DLL MixingHashState : public HashStateBase { unsigned char significant0 = mem0; #else unsigned char significant2 = mem0; - unsigned char significant1 = mem1; + unsigned char significant1 = len == 2 ? mem0 : mem1; unsigned char significant0 = mem2; #endif return static_cast(significant0 | // -- cgit v1.2.3 From 2d4c6872da56fa0d8156b01ab5658348d9c5de4d Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Thu, 16 Feb 2023 06:23:35 -0800 Subject: std::shared_ptr::unique() is deprecated in C++17 and removed in C++20. Change to checking for use_count() >/== 1. PiperOrigin-RevId: 510125744 Change-Id: I572cca18c3f827f5d3eefb2ec19a1a014c0090ae --- absl/functional/any_invocable_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/absl/functional/any_invocable_test.cc b/absl/functional/any_invocable_test.cc index 1ed85407..10a4dee5 100644 --- a/absl/functional/any_invocable_test.cc +++ b/absl/functional/any_invocable_test.cc @@ -1431,14 +1431,14 @@ TYPED_TEST_P(AnyInvTestRvalue, QualifierIndependentObjectLifetime) { auto refs = std::make_shared(); { AnyInvType fun([refs](auto&&...) noexcept { return 0; }); - EXPECT_FALSE(refs.unique()); + EXPECT_GT(refs.use_count(), 1); std::move(fun)(7, 8, 9); // Ensure destructor hasn't run even if rref-qualified - EXPECT_FALSE(refs.unique()); + EXPECT_GT(refs.use_count(), 1); } - EXPECT_TRUE(refs.unique()); + EXPECT_EQ(refs.use_count(), 1); } // NOTE: This test suite originally attempted to enumerate all possible -- cgit v1.2.3 From 6089a04018dc912e11e11f8b0bd7d2118921e6af Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Thu, 16 Feb 2023 11:45:51 -0500 Subject: Prefer emplace back over push_back where emplace_back is more appropriate This also helps a lot with dealing with conversions and data structure creation under the hood. --- absl/crc/internal/crc_cord_state.cc | 2 +- absl/flags/internal/usage.cc | 2 +- absl/flags/parse.cc | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/absl/crc/internal/crc_cord_state.cc b/absl/crc/internal/crc_cord_state.cc index d0be0ddd..28d04dc4 100644 --- a/absl/crc/internal/crc_cord_state.cc +++ b/absl/crc/internal/crc_cord_state.cc @@ -121,7 +121,7 @@ void CrcCordState::Poison() { } } else { // Add a fake corrupt chunk. - rep->prefix_crc.push_back(PrefixCrc(0, crc32c_t{1})); + rep->prefix_crc.emplace_back(0, crc32c_t{1}); } } diff --git a/absl/flags/internal/usage.cc b/absl/flags/internal/usage.cc index 5efc7b07..e9481488 100644 --- a/absl/flags/internal/usage.cc +++ b/absl/flags/internal/usage.cc @@ -130,7 +130,7 @@ class FlagHelpPrettyPrinter { for (auto line : absl::StrSplit(str, absl::ByAnyChar("\n\r"))) { if (!tokens.empty()) { // Keep line separators in the input string. - tokens.push_back("\n"); + tokens.emplace_back("\n"); } for (auto token : absl::StrSplit(line, absl::ByAnyChar(" \t"), absl::SkipEmpty())) { diff --git a/absl/flags/parse.cc b/absl/flags/parse.cc index fa953f55..368248e3 100644 --- a/absl/flags/parse.cc +++ b/absl/flags/parse.cc @@ -190,7 +190,7 @@ bool ArgsList::ReadFromFlagfile(const std::string& flag_file_name) { // This argument represents fake argv[0], which should be present in all arg // lists. - args_.push_back(""); + args_.emplace_back(""); std::string line; bool success = true; @@ -212,7 +212,7 @@ bool ArgsList::ReadFromFlagfile(const std::string& flag_file_name) { break; } - args_.push_back(std::string(stripped)); + args_.emplace_back(stripped); continue; } @@ -367,7 +367,7 @@ bool ReadFlagsFromEnv(const std::vector& flag_names, // This argument represents fake argv[0], which should be present in all arg // lists. - args.push_back(""); + args.emplace_back(""); for (const auto& flag_name : flag_names) { // Avoid infinite recursion. @@ -680,7 +680,7 @@ std::vector ParseCommandLineImpl(int argc, char* argv[], std::vector flagfile_value; std::vector input_args; - input_args.push_back(ArgsList(argc, argv)); + input_args.emplace_back(argc, argv); std::vector output_args; std::vector positional_args; -- cgit v1.2.3 From fd02f6a4bdbdb5b824da5c80a2832be5549391ca Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Thu, 16 Feb 2023 12:31:07 -0500 Subject: Prefer C++ notation over C nullptr instead of 0 and nothing instead of void for function arguments is preferred. --- absl/base/internal/low_level_alloc.cc | 2 +- absl/base/internal/sysinfo.cc | 2 +- absl/debugging/internal/symbolize.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/absl/base/internal/low_level_alloc.cc b/absl/base/internal/low_level_alloc.cc index 662167b0..3f888f2f 100644 --- a/absl/base/internal/low_level_alloc.cc +++ b/absl/base/internal/low_level_alloc.cc @@ -550,7 +550,7 @@ static void *DoAllocWithArena(size_t request, LowLevelAlloc::Arena *arena) { size_t new_pages_size = RoundUp(req_rnd, arena->pagesize * 16); void *new_pages; #ifdef _WIN32 - new_pages = VirtualAlloc(0, new_pages_size, + new_pages = VirtualAlloc(nullptr, new_pages_size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); ABSL_RAW_CHECK(new_pages != nullptr, "VirtualAlloc failed"); #else diff --git a/absl/base/internal/sysinfo.cc b/absl/base/internal/sysinfo.cc index da499d3a..8429fb90 100644 --- a/absl/base/internal/sysinfo.cc +++ b/absl/base/internal/sysinfo.cc @@ -159,7 +159,7 @@ static double GetNominalCPUFrequency() { DWORD type = 0; DWORD data = 0; DWORD data_size = sizeof(data); - auto result = RegQueryValueExA(key, "~MHz", 0, &type, + auto result = RegQueryValueExA(key, "~MHz", nullptr, &type, reinterpret_cast(&data), &data_size); RegCloseKey(key); if (result == ERROR_SUCCESS && type == REG_DWORD && diff --git a/absl/debugging/internal/symbolize.h b/absl/debugging/internal/symbolize.h index 27d5e652..5593fde6 100644 --- a/absl/debugging/internal/symbolize.h +++ b/absl/debugging/internal/symbolize.h @@ -115,7 +115,7 @@ bool RemoveSymbolDecorator(int ticket); // Remove all installed decorators. Returns true if successful, false if // symbolization is currently in progress. -bool RemoveAllSymbolDecorators(void); +bool RemoveAllSymbolDecorators(); // Registers an address range to a file mapping. // -- cgit v1.2.3 From 0372af19f2f4f588351ca8f71b3f528cbe467a4d Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Thu, 16 Feb 2023 11:52:26 -0800 Subject: Add KernelTimeout methods that convert the timeout to the std::chrono methods used by std::condition_variable. A followup change will add an implemention of synchronization_internal::Waiter that can use std:mutex/std::condition_variable to implement the per-thread semaphore that absl::Mutex waits on. This implementation may at some point become the default on platforms such as Windows where there doesn't seem to be an easy way of supporting real absolute timeouts. In this case we can defer to their standard library to implement correct support. PiperOrigin-RevId: 510204786 Change-Id: Icf4d695013fd060abbd53dae23e71ea36f731565 --- absl/synchronization/internal/kernel_timeout.cc | 41 ++++++++++++++++++++++ absl/synchronization/internal/kernel_timeout.h | 15 ++++++++ .../internal/kernel_timeout_test.cc | 38 ++++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/absl/synchronization/internal/kernel_timeout.cc b/absl/synchronization/internal/kernel_timeout.cc index 8d9e7d74..548a8fc6 100644 --- a/absl/synchronization/internal/kernel_timeout.cc +++ b/absl/synchronization/internal/kernel_timeout.cc @@ -15,6 +15,7 @@ #include "absl/synchronization/internal/kernel_timeout.h" #include +#include // NOLINT(build/c++11) #include #include #include @@ -163,6 +164,46 @@ KernelTimeout::DWord KernelTimeout::InMillisecondsFromNow() const { return DWord{0}; } +std::chrono::time_point +KernelTimeout::ToChronoTimePoint() const { + if (!has_timeout()) { + return std::chrono::time_point::max(); + } + + // The cast to std::microseconds is because (on some platforms) the + // std::ratio used by std::chrono::steady_clock doesn't convert to + // std::nanoseconds, so it doesn't compile. + auto micros = std::chrono::duration_cast( + std::chrono::nanoseconds(RawNanos())); + if (is_relative_timeout()) { + auto now = std::chrono::system_clock::now(); + if (micros > + std::chrono::time_point::max() - now) { + // Overflow. + return std::chrono::time_point::max(); + } + return now + micros; + } + return std::chrono::system_clock::from_time_t(0) + micros; +} + +std::chrono::nanoseconds KernelTimeout::ToChronoDuration() const { + if (!has_timeout()) { + return std::chrono::nanoseconds::max(); + } + if (is_absolute_timeout()) { + auto d = std::chrono::duration_cast( + std::chrono::nanoseconds(RawNanos()) - + (std::chrono::system_clock::now() - + std::chrono::system_clock::from_time_t(0))); + if (d < std::chrono::nanoseconds(0)) { + d = std::chrono::nanoseconds(0); + } + return d; + } + return std::chrono::nanoseconds(RawNanos()); +} + } // namespace synchronization_internal ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/synchronization/internal/kernel_timeout.h b/absl/synchronization/internal/kernel_timeout.h index 1f4d82cd..f7c40337 100644 --- a/absl/synchronization/internal/kernel_timeout.h +++ b/absl/synchronization/internal/kernel_timeout.h @@ -16,6 +16,7 @@ #define ABSL_SYNCHRONIZATION_INTERNAL_KERNEL_TIMEOUT_H_ #include +#include // NOLINT(build/c++11) #include #include #include @@ -95,6 +96,20 @@ class KernelTimeout { typedef unsigned long DWord; // NOLINT DWord InMillisecondsFromNow() const; + // Convert to std::chrono::time_point for interfaces that expect an absolute + // timeout, like std::condition_variable::wait_until(). If !has_timeout() or + // is_relative_timeout(), attempts to convert to a reasonable absolute + // timeout, but callers should test has_timeout() and is_relative_timeout() + // and prefer to use a more appropriate interface. + std::chrono::time_point ToChronoTimePoint() const; + + // Convert to std::chrono::time_point for interfaces that expect a relative + // timeout, like std::condition_variable::wait_for(). If !has_timeout() or + // is_absolute_timeout(), attempts to convert to a reasonable relative + // timeout, but callers should test has_timeout() and is_absolute_timeout() + // and prefer to use a more appropriate interface. + std::chrono::nanoseconds ToChronoDuration() const; + private: // Internal representation. // - If the value is kNoTimeout, then the timeout is infinite, and diff --git a/absl/synchronization/internal/kernel_timeout_test.cc b/absl/synchronization/internal/kernel_timeout_test.cc index a89ae220..431ffcf4 100644 --- a/absl/synchronization/internal/kernel_timeout_test.cc +++ b/absl/synchronization/internal/kernel_timeout_test.cc @@ -14,6 +14,7 @@ #include "absl/synchronization/internal/kernel_timeout.h" +#include // NOLINT(build/c++11) #include #include "gtest/gtest.h" @@ -72,6 +73,11 @@ TEST(KernelTimeout, FiniteTimes) { EXPECT_LE(absl::AbsDuration(absl::Milliseconds(t.InMillisecondsFromNow()) - std::max(duration, absl::ZeroDuration())), absl::Milliseconds(5)); + EXPECT_LE(absl::AbsDuration(absl::FromChrono(t.ToChronoTimePoint()) - when), + absl::Microseconds(1)); + EXPECT_LE(absl::AbsDuration(absl::FromChrono(t.ToChronoDuration()) - + std::max(duration, absl::ZeroDuration())), + kTimingBound); } } @@ -90,6 +96,9 @@ TEST(KernelTimeout, InfiniteFuture) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, DefaultConstructor) { @@ -108,6 +117,9 @@ TEST(KernelTimeout, DefaultConstructor) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, TimeMaxNanos) { @@ -126,6 +138,9 @@ TEST(KernelTimeout, TimeMaxNanos) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, Never) { @@ -144,6 +159,9 @@ TEST(KernelTimeout, Never) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, InfinitePast) { @@ -157,6 +175,9 @@ TEST(KernelTimeout, InfinitePast) { absl::ZeroDuration()); EXPECT_LE(absl::FromUnixNanos(t.MakeAbsNanos()), absl::FromUnixNanos(1)); EXPECT_EQ(t.InMillisecondsFromNow(), KernelTimeout::DWord{0}); + EXPECT_LT(t.ToChronoTimePoint(), std::chrono::system_clock::from_time_t(0) + + std::chrono::seconds(1)); + EXPECT_EQ(t.ToChronoDuration(), std::chrono::nanoseconds(0)); } TEST(KernelTimeout, FiniteDurations) { @@ -186,6 +207,10 @@ TEST(KernelTimeout, FiniteDurations) { absl::Milliseconds(5)); EXPECT_LE(absl::Milliseconds(t.InMillisecondsFromNow()) - duration, absl::Milliseconds(5)); + EXPECT_LE(absl::AbsDuration(absl::Now() + duration - + absl::FromChrono(t.ToChronoTimePoint())), + kTimingBound); + EXPECT_EQ(absl::FromChrono(t.ToChronoDuration()), duration); } } @@ -218,6 +243,10 @@ TEST(KernelTimeout, NegativeDurations) { absl::AbsDuration(absl::Now() - absl::FromUnixNanos(t.MakeAbsNanos())), absl::Milliseconds(5)); EXPECT_EQ(t.InMillisecondsFromNow(), KernelTimeout::DWord{0}); + EXPECT_LE(absl::AbsDuration(absl::Now() - + absl::FromChrono(t.ToChronoTimePoint())), + absl::Milliseconds(5)); + EXPECT_EQ(t.ToChronoDuration(), std::chrono::nanoseconds(0)); } } @@ -236,6 +265,9 @@ TEST(KernelTimeout, InfiniteDuration) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, DurationMaxNanos) { @@ -254,6 +286,9 @@ TEST(KernelTimeout, DurationMaxNanos) { absl::Now() + absl::Hours(100000)); EXPECT_EQ(t.InMillisecondsFromNow(), std::numeric_limits::max()); + EXPECT_EQ(t.ToChronoTimePoint(), + std::chrono::time_point::max()); + EXPECT_GE(t.ToChronoDuration(), std::chrono::nanoseconds::max()); } TEST(KernelTimeout, OverflowNanos) { @@ -273,6 +308,9 @@ TEST(KernelTimeout, OverflowNanos) { absl::Now() + absl::Hours(100000)); EXPECT_LE(absl::Milliseconds(t.InMillisecondsFromNow()) - duration, absl::Milliseconds(5)); + EXPECT_GT(t.ToChronoTimePoint(), + std::chrono::system_clock::now() + std::chrono::hours(100000)); + EXPECT_GT(t.ToChronoDuration(), std::chrono::hours(100000)); } } // namespace -- cgit v1.2.3 From ed37a45a374dce32f78ca65d873902364963dfc2 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Fri, 17 Feb 2023 05:17:10 -0800 Subject: Synchronization: Add support for true relative timeouts using monotonic clocks on Linux when the implementation uses futexes After this change, when synchronization methods that wait are passed an absl::Duration to limit the wait time, these methods will wait for that interval, even if the system clock is changed (subject to any limitations with how CLOCK_MONOTONIC keeps track of time). In other words, an observer measuring the time with a stop watch will now see the correct interval, even if the system clock is changed. Previously, the duration was added to the current time, and methods would wait until that time was reached on the possibly changed realtime system clock. The behavior of the synchronization methods that take an absl::Time is unchanged. These methods always wait until the absolute point in time is reached and respect changes to the system clock. In other words, an observer will always see the timeout occur when a wall clock reaches that time, even if the clock is manipulated externally. Note: ABSL_PREDICT_FALSE was removed from the error case in Futex as timeouts are handled by this case, and timeouts are part of normal operation. PiperOrigin-RevId: 510405347 Change-Id: I0b3ea390de97014cfa353079ae2e0c1c637aca69 --- absl/synchronization/internal/futex.h | 70 ++++++++++++++------- absl/synchronization/internal/waiter.cc | 106 ++++++++++++++++++++++++++++++-- absl/synchronization/internal/waiter.h | 3 + absl/synchronization/mutex.cc | 45 ++++++++------ 4 files changed, 177 insertions(+), 47 deletions(-) diff --git a/absl/synchronization/internal/futex.h b/absl/synchronization/internal/futex.h index 9cf9841d..62bb40f7 100644 --- a/absl/synchronization/internal/futex.h +++ b/absl/synchronization/internal/futex.h @@ -16,9 +16,7 @@ #include "absl/base/config.h" -#ifdef _WIN32 -#include -#else +#ifndef _WIN32 #include #include #endif @@ -85,34 +83,60 @@ namespace synchronization_internal { class FutexImpl { public: - static int WaitUntil(std::atomic *v, int32_t val, + // Atomically check that `*v == val`, and if it is, then sleep until the + // timeout `t` has been reached, or until woken by `Wake()`. + static int WaitUntil(std::atomic* v, int32_t val, KernelTimeout t) { - long err = 0; // NOLINT(runtime/int) - if (t.has_timeout()) { - // https://locklessinc.com/articles/futex_cheat_sheet/ - // Unlike FUTEX_WAIT, FUTEX_WAIT_BITSET uses absolute time. - struct timespec abs_timeout = t.MakeAbsTimespec(); - // Atomically check that the futex value is still 0, and if it - // is, sleep until abs_timeout or until woken by FUTEX_WAKE. - err = syscall( - SYS_futex, reinterpret_cast(v), - FUTEX_WAIT_BITSET | FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME, val, - &abs_timeout, nullptr, FUTEX_BITSET_MATCH_ANY); + if (!t.has_timeout()) { + return Wait(v, val); + } else if (t.is_absolute_timeout()) { + auto abs_timespec = t.MakeAbsTimespec(); + return WaitAbsoluteTimeout(v, val, &abs_timespec); } else { - // Atomically check that the futex value is still 0, and if it - // is, sleep until woken by FUTEX_WAKE. - err = syscall(SYS_futex, reinterpret_cast(v), - FUTEX_WAIT | FUTEX_PRIVATE_FLAG, val, nullptr); + auto rel_timespec = t.MakeRelativeTimespec(); + return WaitRelativeTimeout(v, val, &rel_timespec); } - if (ABSL_PREDICT_FALSE(err != 0)) { + } + + // Atomically check that `*v == val`, and if it is, then sleep until the until + // woken by `Wake()`. + static int Wait(std::atomic* v, int32_t val) { + return WaitAbsoluteTimeout(v, val, nullptr); + } + + // Atomically check that `*v == val`, and if it is, then sleep until + // CLOCK_REALTIME reaches `*abs_timeout`, or until woken by `Wake()`. + static int WaitAbsoluteTimeout(std::atomic* v, int32_t val, + const struct timespec* abs_timeout) { + // https://locklessinc.com/articles/futex_cheat_sheet/ + // Unlike FUTEX_WAIT, FUTEX_WAIT_BITSET uses absolute time. + auto err = + syscall(SYS_futex, reinterpret_cast(v), + FUTEX_WAIT_BITSET | FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME, + val, abs_timeout, nullptr, FUTEX_BITSET_MATCH_ANY); + if (err != 0) { + return -errno; + } + return 0; + } + + // Atomically check that `*v == val`, and if it is, then sleep until + // `*rel_timeout` has elapsed, or until woken by `Wake()`. + static int WaitRelativeTimeout(std::atomic* v, int32_t val, + const struct timespec* rel_timeout) { + // Atomically check that the futex value is still 0, and if it + // is, sleep until abs_timeout or until woken by FUTEX_WAKE. + auto err = syscall(SYS_futex, reinterpret_cast(v), + FUTEX_PRIVATE_FLAG, val, rel_timeout); + if (err != 0) { return -errno; } return 0; } - static int Wake(std::atomic *v, int32_t count) { - // NOLINTNEXTLINE(runtime/int) - long err = syscall(SYS_futex, reinterpret_cast(v), + // Wakes at most `count` waiters that have entered the sleep state on `v`. + static int Wake(std::atomic* v, int32_t count) { + auto err = syscall(SYS_futex, reinterpret_cast(v), FUTEX_WAKE | FUTEX_PRIVATE_FLAG, count); if (ABSL_PREDICT_FALSE(err < 0)) { return -errno; diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc index f2051d67..4eee1298 100644 --- a/absl/synchronization/internal/waiter.cc +++ b/absl/synchronization/internal/waiter.cc @@ -67,11 +67,9 @@ static void MaybeBecomeIdle() { #if ABSL_WAITER_MODE == ABSL_WAITER_MODE_FUTEX -Waiter::Waiter() { - futex_.store(0, std::memory_order_relaxed); -} +Waiter::Waiter() : futex_(0) {} -bool Waiter::Wait(KernelTimeout t) { +bool Waiter::WaitAbsoluteTimeout(KernelTimeout t) { // Loop until we can atomically decrement futex from a positive // value, waiting on a futex while we believe it is zero. // Note that, since the thread ticker is just reset, we don't need to check @@ -90,7 +88,88 @@ bool Waiter::Wait(KernelTimeout t) { } if (!first_pass) MaybeBecomeIdle(); - const int err = Futex::WaitUntil(&futex_, 0, t); + auto abs_timeout = t.MakeAbsTimespec(); + const int err = Futex::WaitAbsoluteTimeout(&futex_, 0, &abs_timeout); + if (err != 0) { + if (err == -EINTR || err == -EWOULDBLOCK) { + // Do nothing, the loop will retry. + } else if (err == -ETIMEDOUT) { + return false; + } else { + ABSL_RAW_LOG(FATAL, "Futex operation failed with error %d\n", err); + } + } + first_pass = false; + } +} + +#ifdef CLOCK_MONOTONIC + +// Subtracts the timespec `sub` from `in` if the result would not be negative, +// and returns true. Returns false if the result would be negative, and leaves +// `in` unchanged. +static bool TimespecSubtract(struct timespec& in, const struct timespec& sub) { + if (in.tv_sec < sub.tv_sec) { + return false; + } + if (in.tv_nsec < sub.tv_nsec) { + if (in.tv_sec == sub.tv_sec) { + return false; + } + // Borrow from tv_sec. + in.tv_sec -= 1; + in.tv_nsec += 1'000'000'000; + } + in.tv_sec -= sub.tv_sec; + in.tv_nsec -= sub.tv_nsec; + return true; +} + +// On some platforms a background thread periodically calls `Poke()` to briefly +// wake waiter threads so that they may call `MaybeBecomeIdle()`. This means +// that `WaitRelativeTimeout()` differs slightly from `WaitAbsoluteTimeout()` +// because it must adjust the timeout by the amount of time that it has already +// slept. +bool Waiter::WaitRelativeTimeout(KernelTimeout t) { + struct timespec start; + ABSL_RAW_CHECK(clock_gettime(CLOCK_MONOTONIC, &start) == 0, + "clock_gettime() failed"); + + // Loop until we can atomically decrement futex from a positive + // value, waiting on a futex while we believe it is zero. + // Note that, since the thread ticker is just reset, we don't need to check + // whether the thread is idle on the very first pass of the loop. + bool first_pass = true; + + while (true) { + int32_t x = futex_.load(std::memory_order_relaxed); + while (x != 0) { + if (!futex_.compare_exchange_weak(x, x - 1, + std::memory_order_acquire, + std::memory_order_relaxed)) { + continue; // Raced with someone, retry. + } + return true; // Consumed a wakeup, we are done. + } + + auto relative_timeout = t.MakeRelativeTimespec(); + if (!first_pass) { + MaybeBecomeIdle(); + + // Adjust relative_timeout for `Poke()`s. + struct timespec now; + ABSL_RAW_CHECK(clock_gettime(CLOCK_MONOTONIC, &now) == 0, + "clock_gettime() failed"); + // If TimespecSubstract(now, start) returns false, then the clock isn't + // truly monotonic. + if (TimespecSubtract(now, start)) { + if (!TimespecSubtract(relative_timeout, now)) { + return false; // Timeout. + } + } + } + + const int err = Futex::WaitRelativeTimeout(&futex_, 0, &relative_timeout); if (err != 0) { if (err == -EINTR || err == -EWOULDBLOCK) { // Do nothing, the loop will retry. @@ -104,6 +183,23 @@ bool Waiter::Wait(KernelTimeout t) { } } +#else // CLOCK_MONOTONIC + +// No support for CLOCK_MONOTONIC. +// KernelTimeout will automatically convert to an absolute timeout. +bool Waiter::WaitRelativeTimeout(KernelTimeout t) { + return WaitAbsoluteTimeout(t); +} + +#endif // CLOCK_MONOTONIC + +bool Waiter::Wait(KernelTimeout t) { + if (t.is_absolute_timeout()) { + return WaitAbsoluteTimeout(t); + } + return WaitRelativeTimeout(t); +} + void Waiter::Post() { if (futex_.fetch_add(1, std::memory_order_release) == 0) { // We incremented from 0, need to wake a potential waiter. diff --git a/absl/synchronization/internal/waiter.h b/absl/synchronization/internal/waiter.h index b8adfeb5..c206cc3f 100644 --- a/absl/synchronization/internal/waiter.h +++ b/absl/synchronization/internal/waiter.h @@ -110,6 +110,9 @@ class Waiter { ~Waiter() = delete; #if ABSL_WAITER_MODE == ABSL_WAITER_MODE_FUTEX + bool WaitAbsoluteTimeout(KernelTimeout t); + bool WaitRelativeTimeout(KernelTimeout t); + // Futexes are defined by specification to be 32-bits. // Thus std::atomic must be just an int32_t with lockfree methods. std::atomic futex_; diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 064ccb74..f6a8506c 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -635,21 +635,6 @@ void Mutex::InternalAttemptToUseMutexInFatalSignalHandler() { std::memory_order_release); } -// --------------------------time support - -// Return the current time plus the timeout. Use the same clock as -// PerThreadSem::Wait() for consistency. Unfortunately, we don't have -// such a choice when a deadline is given directly. -static absl::Time DeadlineFromTimeout(absl::Duration timeout) { -#ifndef _WIN32 - struct timeval tv; - gettimeofday(&tv, nullptr); - return absl::TimeFromTimeval(tv) + timeout; -#else - return absl::Now() + timeout; -#endif -} - // --------------------------Mutexes // In the layout below, the msb of the bottom byte is currently unused. Also, @@ -1546,7 +1531,13 @@ void Mutex::LockWhen(const Condition &cond) { } bool Mutex::LockWhenWithTimeout(const Condition &cond, absl::Duration timeout) { - return LockWhenWithDeadline(cond, DeadlineFromTimeout(timeout)); + ABSL_TSAN_MUTEX_PRE_LOCK(this, 0); + GraphId id = DebugOnlyDeadlockCheck(this); + bool res = LockSlowWithDeadline(kExclusive, &cond, + KernelTimeout(timeout), 0); + DebugOnlyLockEnter(this, id); + ABSL_TSAN_MUTEX_POST_LOCK(this, 0, 0); + return res; } bool Mutex::LockWhenWithDeadline(const Condition &cond, absl::Time deadline) { @@ -1569,7 +1560,12 @@ void Mutex::ReaderLockWhen(const Condition &cond) { bool Mutex::ReaderLockWhenWithTimeout(const Condition &cond, absl::Duration timeout) { - return ReaderLockWhenWithDeadline(cond, DeadlineFromTimeout(timeout)); + ABSL_TSAN_MUTEX_PRE_LOCK(this, __tsan_mutex_read_lock); + GraphId id = DebugOnlyDeadlockCheck(this); + bool res = LockSlowWithDeadline(kShared, &cond, KernelTimeout(timeout), 0); + DebugOnlyLockEnter(this, id); + ABSL_TSAN_MUTEX_POST_LOCK(this, __tsan_mutex_read_lock, 0); + return res; } bool Mutex::ReaderLockWhenWithDeadline(const Condition &cond, @@ -1594,7 +1590,18 @@ void Mutex::Await(const Condition &cond) { } bool Mutex::AwaitWithTimeout(const Condition &cond, absl::Duration timeout) { - return AwaitWithDeadline(cond, DeadlineFromTimeout(timeout)); + if (cond.Eval()) { // condition already true; nothing to do + if (kDebugMode) { + this->AssertReaderHeld(); + } + return true; + } + + KernelTimeout t{timeout}; + bool res = this->AwaitCommon(cond, t); + ABSL_RAW_CHECK(res || t.has_timeout(), + "condition untrue on return from Await"); + return res; } bool Mutex::AwaitWithDeadline(const Condition &cond, absl::Time deadline) { @@ -2660,7 +2667,7 @@ bool CondVar::WaitCommon(Mutex *mutex, KernelTimeout t) { } bool CondVar::WaitWithTimeout(Mutex *mu, absl::Duration timeout) { - return WaitWithDeadline(mu, DeadlineFromTimeout(timeout)); + return WaitCommon(mu, KernelTimeout(timeout)); } bool CondVar::WaitWithDeadline(Mutex *mu, absl::Time deadline) { -- cgit v1.2.3 From ab92654a37348637c134c8d24264b8cbf56d9ff8 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Thu, 16 Feb 2023 12:20:06 -0500 Subject: Convert empty constructors to default ones These make the changed constructors match closer to the other ones that are default. --- absl/crc/internal/crc_internal.h | 6 +++--- absl/debugging/leak_check.cc | 4 ++-- absl/flags/internal/commandlineflag.cc | 2 +- absl/log/internal/nullstream.h | 2 +- absl/types/optional.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/absl/crc/internal/crc_internal.h b/absl/crc/internal/crc_internal.h index 0611b383..9f080913 100644 --- a/absl/crc/internal/crc_internal.h +++ b/absl/crc/internal/crc_internal.h @@ -64,7 +64,7 @@ class CRCImpl : public CRC { // Implemention of the abstract class CRC public: using Uint32By256 = uint32_t[256]; - CRCImpl() {} + CRCImpl() = default; ~CRCImpl() override = default; // The internal version of CRC::New(). @@ -96,8 +96,8 @@ class CRCImpl : public CRC { // Implemention of the abstract class CRC // This is the 32-bit implementation. It handles all sizes from 8 to 32. class CRC32 : public CRCImpl { public: - CRC32() {} - ~CRC32() override {} + CRC32() = default; + ~CRC32() override = default; void Extend(uint32_t* crc, const void* bytes, size_t length) const override; void ExtendByZeroes(uint32_t* crc, size_t length) const override; diff --git a/absl/debugging/leak_check.cc b/absl/debugging/leak_check.cc index 195e82bf..fdb8798b 100644 --- a/absl/debugging/leak_check.cc +++ b/absl/debugging/leak_check.cc @@ -65,8 +65,8 @@ bool LeakCheckerIsActive() { return false; } void DoIgnoreLeak(const void*) { } void RegisterLivePointers(const void*, size_t) { } void UnRegisterLivePointers(const void*, size_t) { } -LeakCheckDisabler::LeakCheckDisabler() { } -LeakCheckDisabler::~LeakCheckDisabler() { } +LeakCheckDisabler::LeakCheckDisabler() = default; +LeakCheckDisabler::~LeakCheckDisabler() = default; ABSL_NAMESPACE_END } // namespace absl diff --git a/absl/flags/internal/commandlineflag.cc b/absl/flags/internal/commandlineflag.cc index 4482955c..3c114d10 100644 --- a/absl/flags/internal/commandlineflag.cc +++ b/absl/flags/internal/commandlineflag.cc @@ -19,7 +19,7 @@ namespace absl { ABSL_NAMESPACE_BEGIN namespace flags_internal { -FlagStateInterface::~FlagStateInterface() {} +FlagStateInterface::~FlagStateInterface() = default; } // namespace flags_internal ABSL_NAMESPACE_END diff --git a/absl/log/internal/nullstream.h b/absl/log/internal/nullstream.h index 8ed63d52..16f5f495 100644 --- a/absl/log/internal/nullstream.h +++ b/absl/log/internal/nullstream.h @@ -114,7 +114,7 @@ class NullStreamMaybeFatal final : public NullStream { // and expression-defined severity use `NullStreamMaybeFatal` above. class NullStreamFatal final : public NullStream { public: - NullStreamFatal() {} + NullStreamFatal() = default; // ABSL_ATTRIBUTE_NORETURN doesn't seem to work on destructors with msvc, so // disable msvc's warning about the d'tor never returning. #if defined(_MSC_VER) && !defined(__clang__) diff --git a/absl/types/optional.h b/absl/types/optional.h index 134b2aff..e42ab4d0 100644 --- a/absl/types/optional.h +++ b/absl/types/optional.h @@ -130,7 +130,7 @@ class optional : private optional_internal::optional_data, // Constructs an `optional` holding an empty value, NOT a default constructed // `T`. - constexpr optional() noexcept {} + constexpr optional() noexcept = default; // Constructs an `optional` initialized with `nullopt` to hold an empty value. constexpr optional(nullopt_t) noexcept {} // NOLINT(runtime/explicit) -- cgit v1.2.3 From a0f9b465212aea24d3264b82d315b8ee59e8d7a0 Mon Sep 17 00:00:00 2001 From: Derek Mauro Date: Fri, 17 Feb 2023 14:11:51 -0800 Subject: Update latest Linux container to unbreak Kokoro PiperOrigin-RevId: 510518832 Change-Id: I086c8f2e34312e2b0384b3c67b9c04814d41ddf0 --- ci/linux_docker_containers.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/linux_docker_containers.sh b/ci/linux_docker_containers.sh index 1dd8b74a..dcaf18b9 100644 --- a/ci/linux_docker_containers.sh +++ b/ci/linux_docker_containers.sh @@ -16,6 +16,6 @@ # Test scripts should source this file to get the identifiers. readonly LINUX_ALPINE_CONTAINER="gcr.io/google.com/absl-177019/alpine:20201026" -readonly LINUX_CLANG_LATEST_CONTAINER="gcr.io/google.com/absl-177019/linux_hybrid-latest:20220217" -readonly LINUX_GCC_LATEST_CONTAINER="gcr.io/google.com/absl-177019/linux_hybrid-latest:20220217" +readonly LINUX_CLANG_LATEST_CONTAINER="gcr.io/google.com/absl-177019/linux_hybrid-latest:20230217" +readonly LINUX_GCC_LATEST_CONTAINER="gcr.io/google.com/absl-177019/linux_hybrid-latest:20230217" readonly LINUX_GCC_FLOOR_CONTAINER="gcr.io/google.com/absl-177019/linux_gcc-floor:20230120" -- cgit v1.2.3 From bd624d9f9825f76f14453beb3df81d82b9e17062 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 17 Feb 2023 16:41:38 -0800 Subject: fix(CMake): correct target variable for DLLs The `_dll` variable contains the target name. The code was trying to use `_NAME` when handling ABSL_PROPAGATE_CXX_STD. Probably a cut&paste error from the other places where we handle that option. PiperOrigin-RevId: 510551097 Change-Id: I006435978333dd7a72a300503389a7bf48fb6d0d --- CMake/AbseilDll.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake index 3c0f6901..40219207 100644 --- a/CMake/AbseilDll.cmake +++ b/CMake/AbseilDll.cmake @@ -789,7 +789,7 @@ Cflags: -I\${includedir}${PC_CFLAGS}\n") # Abseil libraries require C++14 as the current minimum standard. When # compiled with C++17 (either because it is the compiler's default or # explicitly requested), then Abseil requires C++17. - _absl_target_compile_features_if_available(${_NAME} PUBLIC ${ABSL_INTERNAL_CXX_STD_FEATURE}) + _absl_target_compile_features_if_available(${_dll} PUBLIC ${ABSL_INTERNAL_CXX_STD_FEATURE}) else() # Note: This is legacy (before CMake 3.8) behavior. Setting the # target-level CXX_STANDARD property to ABSL_CXX_STANDARD (which is @@ -799,8 +799,8 @@ Cflags: -I\${includedir}${PC_CFLAGS}\n") # CXX_STANDARD_REQUIRED does guard against the top-level CMake project # not having enabled CMAKE_CXX_STANDARD_REQUIRED (which prevents # "decaying" to an older standard if the requested one isn't available). - set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD ${ABSL_CXX_STANDARD}) - set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD_REQUIRED ON) + set_property(TARGET ${_dll} PROPERTY CXX_STANDARD ${ABSL_CXX_STANDARD}) + set_property(TARGET ${_dll} PROPERTY CXX_STANDARD_REQUIRED ON) endif() install(TARGETS ${_dll} EXPORT ${PROJECT_NAME}Targets -- cgit v1.2.3 From d290aab6c20264de8c07c613e01ec695c5ede719 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Tue, 21 Feb 2023 09:41:56 -0500 Subject: Remove workaround for gcc 5.1 We support GCC 7 and up, so we can remove this. --- absl/container/internal/raw_hash_set.h | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 33e05736..3fc0381b 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1850,11 +1850,8 @@ class raw_hash_set { // const char* p = "hello"; // s.insert(p); // - // TODO(romanp): Once we stop supporting gcc 5.1 and below, replace - // RequiresInsertable with RequiresInsertable. - // We are hitting this bug: https://godbolt.org/g/1Vht4f. template < - class T, RequiresInsertable = 0, + class T, RequiresInsertable = 0, typename std::enable_if::value, int>::type = 0> std::pair insert(const T& value) { return emplace(value); @@ -1878,11 +1875,8 @@ class raw_hash_set { return insert(std::forward(value)).first; } - // TODO(romanp): Once we stop supporting gcc 5.1 and below, replace - // RequiresInsertable with RequiresInsertable. - // We are hitting this bug: https://godbolt.org/g/1Vht4f. template < - class T, RequiresInsertable = 0, + class T, RequiresInsertable = 0, typename std::enable_if::value, int>::type = 0> iterator insert(const_iterator, const T& value) { return insert(value).first; -- cgit v1.2.3 From 8459e11a508162d02000420f3ced5814a5743c08 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Tue, 21 Feb 2023 09:32:30 -0500 Subject: Remove check for apple_build_version for hash workaround Apple's clang fork has the missing commit now, and we can safely use the above codepath. --- absl/hash/internal/hash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index eb50e2c7..f81b81bf 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -1136,7 +1136,7 @@ class ABSL_DLL MixingHashState : public HashStateBase { // probably per-build and not per-process. ABSL_ATTRIBUTE_ALWAYS_INLINE static uint64_t Seed() { #if (!defined(__clang__) || __clang_major__ > 11) && \ - !defined(__apple_build_version__) + (!defined(__apple_build_version__) || __apple_build_version__ >= 19558921) // Xcode 12 return static_cast(reinterpret_cast(&kSeed)); #else // Workaround the absence of -- cgit v1.2.3 From d9ae096e74b04d3567aa89db234204fd4b11dd3f Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 21 Feb 2023 11:19:15 -0800 Subject: absl: fix potential int overflow in ELF reading Both e_shentsize and e_shstrndx are uint16, so the product elf_header.e_shentsize * elf_header.e_shstrndx can overflow the promoted type int (MAX_UINT16 * MAX_UINT16 > MAX_INT), which is undefined behavior. Not sure if it can affect any real cases or not, though. Cast e_shentsize to loff_t instead of e_shoff. This makes both multiplication and addition to use loff_t type. PiperOrigin-RevId: 511254775 Change-Id: I39c493bfb539cca6742aae807c50718d31e7c001 --- absl/debugging/symbolize_elf.inc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/absl/debugging/symbolize_elf.inc b/absl/debugging/symbolize_elf.inc index ffb4eecf..0fee89f2 100644 --- a/absl/debugging/symbolize_elf.inc +++ b/absl/debugging/symbolize_elf.inc @@ -532,6 +532,11 @@ bool ForEachSection(int fd, return false; } + // Technically it can be larger, but in practice this never happens. + if (elf_header.e_shentsize != sizeof(ElfW(Shdr))) { + return false; + } + ElfW(Shdr) shstrtab; off_t shstrtab_offset = static_cast(elf_header.e_shoff) + elf_header.e_shentsize * elf_header.e_shstrndx; @@ -584,6 +589,11 @@ bool GetSectionHeaderByName(int fd, const char *name, size_t name_len, return false; } + // Technically it can be larger, but in practice this never happens. + if (elf_header.e_shentsize != sizeof(ElfW(Shdr))) { + return false; + } + ElfW(Shdr) shstrtab; off_t shstrtab_offset = static_cast(elf_header.e_shoff) + elf_header.e_shentsize * elf_header.e_shstrndx; -- cgit v1.2.3 From 7a522a573fbf85f9f1c42dc49a37496e64995f0e Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Tue, 21 Feb 2023 11:21:00 -0800 Subject: Refactor swisstable iterator debug messages code. The motivations are (a) distinguish between the "likely erased" and "could have rehashed" cases when generations are enabled, (b) suggest running under ASan when generations aren't enabled and doing so would narrow down the possible error cases, and (c) make ABSL_INTERNAL_ASSERT_IS_FULL not be a macro. PiperOrigin-RevId: 511255275 Change-Id: I5a44a813cd310837d0bd0209d2187b100be201e7 --- absl/container/internal/raw_hash_set.h | 124 +++++++++++++++++---------- absl/container/internal/raw_hash_set_test.cc | 33 ++++--- 2 files changed, 101 insertions(+), 56 deletions(-) diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 33e05736..bc33389b 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -179,6 +179,7 @@ #include #include #include +#include #include #include #include @@ -1038,35 +1039,75 @@ size_t SelectBucketCountForIterRange(InputIter first, InputIter last, return 0; } -#define ABSL_INTERNAL_ASSERT_IS_FULL(ctrl, generation, generation_ptr, \ - operation) \ - do { \ - ABSL_HARDENING_ASSERT((ctrl != nullptr) && operation \ - " called on end() iterator."); \ - ABSL_HARDENING_ASSERT((ctrl != EmptyGroup()) && operation \ - " called on default-constructed iterator."); \ - if (SwisstableGenerationsEnabled() && generation != *generation_ptr) \ - ABSL_INTERNAL_LOG(FATAL, operation \ - " called on invalidated iterator. The table could " \ - "have rehashed since this iterator was initialized."); \ - ABSL_HARDENING_ASSERT( \ - (IsFull(*ctrl)) && operation \ - " called on invalid iterator. The element might have been erased or " \ - "the table might have rehashed."); \ - } while (0) +constexpr bool SwisstableDebugEnabled() { +#if defined(ABSL_SWISSTABLE_ENABLE_GENERATIONS) || \ + ABSL_OPTION_HARDENED == 1 || !defined(NDEBUG) + return true; +#else + return false; +#endif +} + +inline void AssertIsFull(const ctrl_t* ctrl, GenerationType generation, + const GenerationType* generation_ptr, + const char* operation) { + if (!SwisstableDebugEnabled()) return; + if (ctrl == nullptr) { + ABSL_INTERNAL_LOG(FATAL, + std::string(operation) + " called on end() iterator."); + } + if (ctrl == EmptyGroup()) { + ABSL_INTERNAL_LOG(FATAL, std::string(operation) + + " called on default-constructed iterator."); + } + if (SwisstableGenerationsEnabled()) { + if (generation != *generation_ptr) { + ABSL_INTERNAL_LOG(FATAL, + std::string(operation) + + " called on invalid iterator. The table could have " + "rehashed since this iterator was initialized."); + } + if (!IsFull(*ctrl)) { + ABSL_INTERNAL_LOG( + FATAL, + std::string(operation) + + " called on invalid iterator. The element was likely erased."); + } + } else { + if (!IsFull(*ctrl)) { + ABSL_INTERNAL_LOG( + FATAL, + std::string(operation) + + " called on invalid iterator. The element might have been erased " + "or the table might have rehashed. Consider running with " + "--config=asan to diagnose rehashing issues."); + } + } +} // Note that for comparisons, null/end iterators are valid. inline void AssertIsValidForComparison(const ctrl_t* ctrl, GenerationType generation, const GenerationType* generation_ptr) { - ABSL_HARDENING_ASSERT( - (ctrl == nullptr || ctrl == EmptyGroup() || IsFull(*ctrl)) && - "Invalid iterator comparison. The element might have " - "been erased or the table might have rehashed."); - if (SwisstableGenerationsEnabled() && generation != *generation_ptr) { - ABSL_INTERNAL_LOG(FATAL, - "Invalid iterator comparison. The table could have " - "rehashed since this iterator was initialized."); + if (!SwisstableDebugEnabled()) return; + const bool ctrl_is_valid_for_comparison = + ctrl == nullptr || ctrl == EmptyGroup() || IsFull(*ctrl); + if (SwisstableGenerationsEnabled()) { + if (generation != *generation_ptr) { + ABSL_INTERNAL_LOG(FATAL, + "Invalid iterator comparison. The table could have " + "rehashed since this iterator was initialized."); + } + if (!ctrl_is_valid_for_comparison) { + ABSL_INTERNAL_LOG( + FATAL, "Invalid iterator comparison. The element was likely erased."); + } + } else { + ABSL_HARDENING_ASSERT( + ctrl_is_valid_for_comparison && + "Invalid iterator comparison. The element might have been erased or " + "the table might have rehashed. Consider running with --config=asan to " + "diagnose rehashing issues."); } } @@ -1097,8 +1138,7 @@ inline void AssertSameContainer(const ctrl_t* ctrl_a, const ctrl_t* ctrl_b, const void* const& slot_b, const GenerationType* generation_ptr_a, const GenerationType* generation_ptr_b) { -#if defined(ABSL_SWISSTABLE_ENABLE_GENERATIONS) || \ - ABSL_OPTION_HARDENED == 1 || !defined(NDEBUG) + if (!SwisstableDebugEnabled()) return; const bool a_is_default = ctrl_a == EmptyGroup(); const bool b_is_default = ctrl_b == EmptyGroup(); if (a_is_default != b_is_default) { @@ -1108,9 +1148,9 @@ inline void AssertSameContainer(const ctrl_t* ctrl_a, const ctrl_t* ctrl_b, "with non-default-constructed iterator."); } if (a_is_default && b_is_default) return; -#endif - if (SwisstableGenerationsEnabled() && generation_ptr_a != generation_ptr_b) { + if (SwisstableGenerationsEnabled()) { + if (generation_ptr_a == generation_ptr_b) return; const bool a_is_empty = generation_ptr_a == EmptyGeneration(); const bool b_is_empty = generation_ptr_b == EmptyGeneration(); if (a_is_empty != b_is_empty) { @@ -1129,11 +1169,13 @@ inline void AssertSameContainer(const ctrl_t* ctrl_a, const ctrl_t* ctrl_b, ABSL_INTERNAL_LOG(FATAL, "Invalid iterator comparison. Comparing non-end() " "iterators from different hashtables."); + } else { + ABSL_HARDENING_ASSERT( + AreItersFromSameContainer(ctrl_a, ctrl_b, slot_a, slot_b) && + "Invalid iterator comparison. The iterators may be from different " + "containers or the container might have rehashed. Consider running " + "with --config=asan to diagnose rehashing issues."); } - ABSL_HARDENING_ASSERT( - AreItersFromSameContainer(ctrl_a, ctrl_b, slot_a, slot_b) && - "Invalid iterator comparison. The iterators may be from different " - "containers or the container might have rehashed."); } struct FindInfo { @@ -1471,22 +1513,19 @@ class raw_hash_set { // PRECONDITION: not an end() iterator. reference operator*() const { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, generation(), generation_ptr(), - "operator*()"); + AssertIsFull(ctrl_, generation(), generation_ptr(), "operator*()"); return PolicyTraits::element(slot_); } // PRECONDITION: not an end() iterator. pointer operator->() const { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, generation(), generation_ptr(), - "operator->"); + AssertIsFull(ctrl_, generation(), generation_ptr(), "operator->"); return &operator*(); } // PRECONDITION: not an end() iterator. iterator& operator++() { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, generation(), generation_ptr(), - "operator++"); + AssertIsFull(ctrl_, generation(), generation_ptr(), "operator++"); ++ctrl_; ++slot_; skip_empty_or_deleted(); @@ -2052,8 +2091,7 @@ class raw_hash_set { // This overload is necessary because otherwise erase(const K&) would be // a better match if non-const iterator is passed as an argument. void erase(iterator it) { - ABSL_INTERNAL_ASSERT_IS_FULL(it.ctrl_, it.generation(), it.generation_ptr(), - "erase()"); + AssertIsFull(it.ctrl_, it.generation(), it.generation_ptr(), "erase()"); PolicyTraits::destroy(&alloc_ref(), it.slot_); erase_meta_only(it); } @@ -2087,9 +2125,8 @@ class raw_hash_set { } node_type extract(const_iterator position) { - ABSL_INTERNAL_ASSERT_IS_FULL(position.inner_.ctrl_, - position.inner_.generation(), - position.inner_.generation_ptr(), "extract()"); + AssertIsFull(position.inner_.ctrl_, position.inner_.generation(), + position.inner_.generation_ptr(), "extract()"); auto node = CommonAccess::Transfer(alloc_ref(), position.inner_.slot_); erase_meta_only(position); @@ -2739,6 +2776,5 @@ ABSL_NAMESPACE_END } // namespace absl #undef ABSL_SWISSTABLE_ENABLE_GENERATIONS -#undef ABSL_INTERNAL_ASSERT_IS_FULL #endif // ABSL_CONTAINER_INTERNAL_RAW_HASH_SET_H_ diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index 4f2d006d..c13b6757 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -2056,7 +2056,8 @@ bool IsAssertEnabled() { } TEST(TableDeathTest, InvalidIteratorAsserts) { - if (!IsAssertEnabled()) GTEST_SKIP() << "Assertions not enabled."; + if (!IsAssertEnabled() && !SwisstableGenerationsEnabled()) + GTEST_SKIP() << "Assertions not enabled."; IntTable t; // Extra simple "regexp" as regexp support is highly varied across platforms. @@ -2068,9 +2069,12 @@ TEST(TableDeathTest, InvalidIteratorAsserts) { t.insert(0); iter = t.begin(); t.erase(iter); - EXPECT_DEATH_IF_SUPPORTED(++iter, - "operator.* called on invalid iterator. The " - "element might have been erased"); + const char* const kErasedDeathMessage = + SwisstableGenerationsEnabled() + ? "operator.* called on invalid iterator.*was likely erased" + : "operator.* called on invalid iterator.*might have been " + "erased.*config=asan"; + EXPECT_DEATH_IF_SUPPORTED(++iter, kErasedDeathMessage); } // Invalid iterator use can trigger heap-use-after-free in asan, @@ -2087,7 +2091,8 @@ constexpr bool kMsvc = false; #endif TEST(TableDeathTest, IteratorInvalidAssertsEqualityOperator) { - if (!IsAssertEnabled()) GTEST_SKIP() << "Assertions not enabled."; + if (!IsAssertEnabled() && !SwisstableGenerationsEnabled()) + GTEST_SKIP() << "Assertions not enabled."; IntTable t; t.insert(1); @@ -2100,8 +2105,9 @@ TEST(TableDeathTest, IteratorInvalidAssertsEqualityOperator) { t.erase(iter1); // Extra simple "regexp" as regexp support is highly varied across platforms. const char* const kErasedDeathMessage = - "Invalid iterator comparison. The element might have .*been erased or " - "the table might have rehashed."; + SwisstableGenerationsEnabled() + ? "Invalid iterator comparison.*was likely erased" + : "Invalid iterator comparison.*might have been erased.*config=asan"; EXPECT_DEATH_IF_SUPPORTED(void(iter1 == iter2), kErasedDeathMessage); EXPECT_DEATH_IF_SUPPORTED(void(iter2 != iter1), kErasedDeathMessage); t.erase(iter2); @@ -2114,17 +2120,20 @@ TEST(TableDeathTest, IteratorInvalidAssertsEqualityOperator) { iter2 = t2.begin(); const char* const kContainerDiffDeathMessage = SwisstableGenerationsEnabled() - ? "Invalid iterator comparison.*non-end" - : "Invalid iterator comparison. The iterators may be from different " - ".*containers or the container might have rehashed."; + ? "Invalid iterator comparison.*iterators from different hashtables" + : "Invalid iterator comparison.*may be from different " + ".*containers.*config=asan"; EXPECT_DEATH_IF_SUPPORTED(void(iter1 == iter2), kContainerDiffDeathMessage); EXPECT_DEATH_IF_SUPPORTED(void(iter2 == iter1), kContainerDiffDeathMessage); for (int i = 0; i < 10; ++i) t1.insert(i); // There should have been a rehash in t1. if (kMsvc) return; // MSVC doesn't support | in regex. - EXPECT_DEATH_IF_SUPPORTED(void(iter1 == t1.begin()), - kInvalidIteratorDeathMessage); + const char* const kRehashedDeathMessage = + SwisstableGenerationsEnabled() + ? kInvalidIteratorDeathMessage + : "Invalid iterator comparison.*might have rehashed.*config=asan"; + EXPECT_DEATH_IF_SUPPORTED(void(iter1 == t1.begin()), kRehashedDeathMessage); } #if defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE) -- cgit v1.2.3 From 6247f0e918bbc0519955d41b135069dcc2bbd453 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Tue, 21 Feb 2023 09:54:00 -0500 Subject: Resolve TODO: remove C++11 workarounds We no longer support C++11 as per the Google C++ support documentation: we support C++14 and up, so we can remove these workarounds. --- absl/container/fixed_array.h | 2 +- absl/container/inlined_vector.h | 2 +- absl/strings/cord_buffer.h | 4 +--- absl/time/duration.cc | 11 ++--------- absl/types/internal/span.h | 2 +- absl/types/internal/variant.h | 4 ++-- absl/types/optional_test.cc | 13 ------------- absl/types/span_test.cc | 2 +- 8 files changed, 9 insertions(+), 31 deletions(-) diff --git a/absl/container/fixed_array.h b/absl/container/fixed_array.h index b67379cf..efc40a5f 100644 --- a/absl/container/fixed_array.h +++ b/absl/container/fixed_array.h @@ -342,7 +342,7 @@ class FixedArray { // Relational operators. Equality operators are elementwise using // `operator==`, while order operators order FixedArrays lexicographically. friend bool operator==(const FixedArray& lhs, const FixedArray& rhs) { - return absl::equal(lhs.begin(), lhs.end(), rhs.begin(), rhs.end()); + return std::equal(lhs.begin(), lhs.end(), rhs.begin(), rhs.end()); } friend bool operator!=(const FixedArray& lhs, const FixedArray& rhs) { diff --git a/absl/container/inlined_vector.h b/absl/container/inlined_vector.h index 7058f375..42a4f946 100644 --- a/absl/container/inlined_vector.h +++ b/absl/container/inlined_vector.h @@ -843,7 +843,7 @@ bool operator==(const absl::InlinedVector& a, const absl::InlinedVector& b) { auto a_data = a.data(); auto b_data = b.data(); - return absl::equal(a_data, a_data + a.size(), b_data, b_data + b.size()); + return std::equal(a_data, a_data + a.size(), b_data, b_data + b.size()); } // `operator!=(...)` diff --git a/absl/strings/cord_buffer.h b/absl/strings/cord_buffer.h index 15494b31..e39c84cd 100644 --- a/absl/strings/cord_buffer.h +++ b/absl/strings/cord_buffer.h @@ -460,9 +460,7 @@ inline constexpr size_t CordBuffer::MaximumPayload() { } inline constexpr size_t CordBuffer::MaximumPayload(size_t block_size) { - // TODO(absl-team): Use std::min when C++11 support is dropped. - return (kCustomLimit < block_size ? kCustomLimit : block_size) - - cord_internal::kFlatOverhead; + return std::min(kCustomLimit, block_size) - cord_internal::kFlatOverhead; } inline CordBuffer CordBuffer::CreateWithDefaultLimit(size_t capacity) { diff --git a/absl/time/duration.cc b/absl/time/duration.cc index 911e80f8..26fadb31 100644 --- a/absl/time/duration.cc +++ b/absl/time/duration.cc @@ -96,13 +96,6 @@ inline bool IsValidDivisor(double d) { return d != 0.0; } -// Can't use std::round() because it is only available in C++11. -// Note that we ignore the possibility of floating-point over/underflow. -template -inline double Round(Double d) { - return d < 0 ? std::ceil(d - 0.5) : std::floor(d + 0.5); -} - // *sec may be positive or negative. *ticks must be in the range // -kTicksPerSecond < *ticks < kTicksPerSecond. If *ticks is negative it // will be normalized to a positive value by adjusting *sec accordingly. @@ -260,7 +253,7 @@ inline Duration ScaleDouble(Duration d, double r) { double lo_frac = std::modf(lo_doub, &lo_int); // Rolls lo into hi if necessary. - int64_t lo64 = Round(lo_frac * kTicksPerSecond); + int64_t lo64 = std::round(lo_frac * kTicksPerSecond); Duration ans; if (!SafeAddRepHi(hi_int, lo_int, &ans)) return ans; @@ -741,7 +734,7 @@ void AppendNumberUnit(std::string* out, double n, DisplayUnit unit) { char buf[kBufferSize]; // also large enough to hold integer part char* ep = buf + sizeof(buf); double d = 0; - int64_t frac_part = Round(std::modf(n, &d) * unit.pow10); + int64_t frac_part = std::round(std::modf(n, &d) * unit.pow10); int64_t int_part = d; if (int_part != 0 || frac_part != 0) { char* bp = Format64(ep, 0, int_part); // always < 1000 diff --git a/absl/types/internal/span.h b/absl/types/internal/span.h index 344ad4db..a196362a 100644 --- a/absl/types/internal/span.h +++ b/absl/types/internal/span.h @@ -88,7 +88,7 @@ using EnableIfMutable = template