From 25597bdfc148e91e27678ec30efa52f4fc8c164f Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 2 Oct 2019 15:20:57 -0700 Subject: Export of internal Abseil changes -- 3e60f355db5afd7a864591d81a6c383b6c0a0780 by Samuel Benzaquen : Internal change PiperOrigin-RevId: 272531442 -- 6d189240b8cebe3a390c730de491156d03049229 by Andy Getzendanner : Fix AtomicHook init-order fiasco under MSVC 2019. On this platform, constexpr static init sometimes happens after dynamic init =/. When it does, we should not zero hook_ (overwriting the value written there by dynamic init); instead we should leave it alone. This works even when constexpr static init goes first since all uses of AtomicHook should have static storage duration and be zero-initialized. https://developercommunity.visualstudio.com/content/problem/336946/class-with-constexpr-constructor-not-using-static.html PiperOrigin-RevId: 272525226 -- d01b14fc06bc75b41c51976ed32e7c304ea1aab7 by Abseil Team : exclude emscripten from running tests involving long doubles PiperOrigin-RevId: 272497628 GitOrigin-RevId: 3e60f355db5afd7a864591d81a6c383b6c0a0780 Change-Id: I3c8a8f5acaf7652a06ef40cf028ef5d2e142f81b --- absl/base/BUILD.bazel | 14 ++++++++++++ absl/base/CMakeLists.txt | 15 +++++++++++++ absl/base/internal/atomic_hook.h | 26 +++++++++++++++------- absl/base/internal/atomic_hook_test.cc | 24 ++++++++++++++++++++ absl/base/internal/atomic_hook_test_helper.cc | 29 ++++++++++++++++++++++++ absl/base/internal/atomic_hook_test_helper.h | 32 +++++++++++++++++++++++++++ 6 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 absl/base/internal/atomic_hook_test_helper.cc create mode 100644 absl/base/internal/atomic_hook_test_helper.h (limited to 'absl/base') diff --git a/absl/base/BUILD.bazel b/absl/base/BUILD.bazel index 57caa0656e59..1e1f0d2de318 100644 --- a/absl/base/BUILD.bazel +++ b/absl/base/BUILD.bazel @@ -207,6 +207,19 @@ cc_library( ], ) +cc_library( + name = "atomic_hook_test_helper", + testonly = 1, + srcs = ["internal/atomic_hook_test_helper.cc"], + hdrs = ["internal/atomic_hook_test_helper.h"], + copts = ABSL_DEFAULT_COPTS, + linkopts = ABSL_DEFAULT_LINKOPTS, + deps = [ + ":atomic_hook", + ":core_headers", + ], +) + cc_test( name = "atomic_hook_test", size = "small", @@ -215,6 +228,7 @@ cc_test( linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ ":atomic_hook", + ":atomic_hook_test_helper", ":core_headers", "@com_google_googletest//:gtest_main", ], diff --git a/absl/base/CMakeLists.txt b/absl/base/CMakeLists.txt index 2f11ef86556f..51705a5a102c 100644 --- a/absl/base/CMakeLists.txt +++ b/absl/base/CMakeLists.txt @@ -254,6 +254,19 @@ absl_cc_test( gtest_main ) +absl_cc_library( + NAME + atomic_hook_test_helper + SRCS + "internal/atomic_hook_test_helper.cc" + COPTS + ${ABSL_TEST_COPTS} + DEPS + absl::atomic_hook + absl::core_headers + TESTONLY +) + absl_cc_test( NAME atomic_hook_test @@ -262,8 +275,10 @@ absl_cc_test( COPTS ${ABSL_TEST_COPTS} DEPS + absl::atomic_hook_test_helper absl::atomic_hook absl::core_headers + gmock gtest_main ) diff --git a/absl/base/internal/atomic_hook.h b/absl/base/internal/atomic_hook.h index 803e9059761c..09f763d0742b 100644 --- a/absl/base/internal/atomic_hook.h +++ b/absl/base/internal/atomic_hook.h @@ -11,7 +11,6 @@ // 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. -// #ifndef ABSL_BASE_INTERNAL_ATOMIC_HOOK_H_ #define ABSL_BASE_INTERNAL_ATOMIC_HOOK_H_ @@ -23,8 +22,10 @@ #ifdef _MSC_FULL_VER #define ABSL_HAVE_WORKING_ATOMIC_POINTER 0 +#define ABSL_HAVE_WORKING_CONSTEXPR_STATIC_INIT 0 #else #define ABSL_HAVE_WORKING_ATOMIC_POINTER 1 +#define ABSL_HAVE_WORKING_CONSTEXPR_STATIC_INIT 1 #endif namespace absl { @@ -33,16 +34,17 @@ namespace base_internal { template class AtomicHook; -// AtomicHook is a helper class, templatized on a raw function pointer type, for -// implementing Abseil customization hooks. It is a callable object that -// dispatches to the registered hook. +// `AtomicHook` is a helper class, templatized on a raw function pointer type, +// for implementing Abseil customization hooks. It is a callable object that +// dispatches to the registered hook. Objects of type `AtomicHook` must have +// static or thread storage duration. // // A default constructed object performs a no-op (and returns a default // constructed object) if no hook has been registered. // // Hooks can be pre-registered via constant initialization, for example, -// ABSL_CONST_INIT static AtomicHook my_hook(DefaultAction); -// and then changed at runtime via a call to Store(). +// `ABSL_CONST_INIT static AtomicHook my_hook(DefaultAction);` +// and then changed at runtime via a call to `Store()`. // // Reads and writes guarantee memory_order_acquire/memory_order_release // semantics. @@ -57,12 +59,19 @@ class AtomicHook { // Constructs an object that by default dispatches to/returns the // pre-registered default_fn when no hook has been registered at runtime. -#if ABSL_HAVE_WORKING_ATOMIC_POINTER +#if ABSL_HAVE_WORKING_ATOMIC_POINTER && ABSL_HAVE_WORKING_CONSTEXPR_STATIC_INIT explicit constexpr AtomicHook(FnPtr default_fn) : hook_(default_fn), default_fn_(default_fn) {} #else + // On MSVC, this function sometimes executes after dynamic initiazliation =(. + // If a non-zero `hook_` has been installed by a dynamic initializer, we want + // to preserve it. If not, `hook_` will be zero initialized and we have no + // need to set it to `kUninitialized`. + // https://developercommunity.visualstudio.com/content/problem/336946/class-with-constexpr-constructor-not-using-static.html explicit constexpr AtomicHook(FnPtr default_fn) - : hook_(kUninitialized), default_fn_(default_fn) {} + : /* hook_(deliberately omitted), */ default_fn_(default_fn) { + static_assert(kUninitialized == 0, "here we rely on zero-initialization"); + } #endif // Stores the provided function pointer as the value for this hook. @@ -158,6 +167,7 @@ class AtomicHook { }; #undef ABSL_HAVE_WORKING_ATOMIC_POINTER +#undef ABSL_HAVE_WORKING_CONSTEXPR_STATIC_INIT } // namespace base_internal } // namespace absl diff --git a/absl/base/internal/atomic_hook_test.cc b/absl/base/internal/atomic_hook_test.cc index ecc80406b003..794072ee2fc2 100644 --- a/absl/base/internal/atomic_hook_test.cc +++ b/absl/base/internal/atomic_hook_test.cc @@ -14,11 +14,15 @@ #include "absl/base/internal/atomic_hook.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/base/attributes.h" +#include "absl/base/internal/atomic_hook_test_helper.h" namespace { +using ::testing::Eq; + int value = 0; void TestHook(int x) { value = x; } @@ -67,4 +71,24 @@ TEST(AtomicHookTest, WithDefaultFunction) { EXPECT_EQ(value, 2); } +ABSL_CONST_INIT int override_func_calls = 0; +void OverrideFunc() { override_func_calls++; } +static struct OverrideInstaller { + OverrideInstaller() { absl::atomic_hook_internal::func.Store(OverrideFunc); } +} override_installer; + +TEST(AtomicHookTest, DynamicInitFromAnotherTU) { + // MSVC 14.2 doesn't do constexpr static init correctly; in particular it + // tends to sequence static init (i.e. defaults) of `AtomicHook` objects + // after their dynamic init (i.e. overrides), overwriting whatever value was + // written during dynamic init. This regression test validates the fix. + // https://developercommunity.visualstudio.com/content/problem/336946/class-with-constexpr-constructor-not-using-static.html + EXPECT_THAT(absl::atomic_hook_internal::default_func_calls, Eq(0)); + EXPECT_THAT(override_func_calls, Eq(0)); + absl::atomic_hook_internal::func(); + EXPECT_THAT(absl::atomic_hook_internal::default_func_calls, Eq(0)); + EXPECT_THAT(override_func_calls, Eq(1)); + EXPECT_THAT(absl::atomic_hook_internal::func.Load(), Eq(OverrideFunc)); +} + } // namespace diff --git a/absl/base/internal/atomic_hook_test_helper.cc b/absl/base/internal/atomic_hook_test_helper.cc new file mode 100644 index 000000000000..fad7a89a6ffa --- /dev/null +++ b/absl/base/internal/atomic_hook_test_helper.cc @@ -0,0 +1,29 @@ +// Copyright 2017 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/internal/atomic_hook_test_helper.h" + +#include "absl/base/attributes.h" +#include "absl/base/internal/atomic_hook.h" + +namespace absl { +namespace atomic_hook_internal { + +ABSL_CONST_INIT absl::base_internal::AtomicHook func(DefaultFunc); +ABSL_CONST_INIT int default_func_calls = 0; +void DefaultFunc() { default_func_calls++; } +void RegisterFunc(VoidF f) { func.Store(f); } + +} // namespace atomic_hook_internal +} // namespace absl diff --git a/absl/base/internal/atomic_hook_test_helper.h b/absl/base/internal/atomic_hook_test_helper.h new file mode 100644 index 000000000000..44ff780d6b93 --- /dev/null +++ b/absl/base/internal/atomic_hook_test_helper.h @@ -0,0 +1,32 @@ +// Copyright 2017 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. + +#ifndef ABSL_BASE_ATOMIC_HOOK_TEST_HELPER_H_ +#define ABSL_BASE_ATOMIC_HOOK_TEST_HELPER_H_ + +#include "absl/base/internal/atomic_hook.h" + +namespace absl { +namespace atomic_hook_internal { + +using VoidF = void (*)(); +extern absl::base_internal::AtomicHook func; +extern int default_func_calls; +void DefaultFunc(); +void RegisterFunc(VoidF func); + +} // namespace atomic_hook_internal +} // namespace absl + +#endif // ABSL_BASE_ATOMIC_HOOK_TEST_HELPER_H_ -- cgit 1.4.1