Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
3e60f355db5afd7a864591d81a6c383b6c0a0780 by Samuel Benzaquen <sbenza@google.com>:

Internal change

PiperOrigin-RevId: 272531442

--
6d189240b8cebe3a390c730de491156d03049229 by Andy Getzendanner <durandal@google.com>:

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 <absl-team@google.com>:

exclude emscripten from running tests involving long doubles

PiperOrigin-RevId: 272497628
GitOrigin-RevId: 3e60f355db5afd7a864591d81a6c383b6c0a0780
Change-Id: I3c8a8f5acaf7652a06ef40cf028ef5d2e142f81b
  • Loading branch information
Abseil Team authored and CJ-Johnson committed Oct 3, 2019
1 parent aad33fe commit 25597bd
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 12 deletions.
14 changes: 14 additions & 0 deletions absl/base/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -215,6 +228,7 @@ cc_test(
linkopts = ABSL_DEFAULT_LINKOPTS,
deps = [
":atomic_hook",
":atomic_hook_test_helper",
":core_headers",
"@com_google_googletest//:gtest_main",
],
Expand Down
15 changes: 15 additions & 0 deletions absl/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
)

Expand Down
26 changes: 18 additions & 8 deletions absl/base/internal/atomic_hook.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Expand All @@ -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 {
Expand All @@ -33,16 +34,17 @@ namespace base_internal {
template <typename T>
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<void(*)()> my_hook(DefaultAction);
// and then changed at runtime via a call to Store().
// `ABSL_CONST_INIT static AtomicHook<void(*)()> my_hook(DefaultAction);`
// and then changed at runtime via a call to `Store()`.
//
// Reads and writes guarantee memory_order_acquire/memory_order_release
// semantics.
Expand All @@ -57,12 +59,19 @@ class AtomicHook<ReturnType (*)(Args...)> {

// 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.
Expand Down Expand Up @@ -158,6 +167,7 @@ class AtomicHook<ReturnType (*)(Args...)> {
};

#undef ABSL_HAVE_WORKING_ATOMIC_POINTER
#undef ABSL_HAVE_WORKING_CONSTEXPR_STATIC_INIT

} // namespace base_internal
} // namespace absl
Expand Down
24 changes: 24 additions & 0 deletions absl/base/internal/atomic_hook_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -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
29 changes: 29 additions & 0 deletions absl/base/internal/atomic_hook_test_helper.cc
Original file line number Diff line number Diff line change
@@ -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<VoidF> 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
32 changes: 32 additions & 0 deletions absl/base/internal/atomic_hook_test_helper.h
Original file line number Diff line number Diff line change
@@ -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<VoidF> 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_
20 changes: 19 additions & 1 deletion absl/container/internal/hashtablez_sampler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,26 @@ int64_t HashtablezSampler::Iterate(
return dropped_samples_.load(std::memory_order_relaxed);
}

static bool ShouldForceSampling() {
enum ForceState {
kDontForce,
kForce,
kUninitialized
};
ABSL_CONST_INIT static std::atomic<ForceState> global_state{
kUninitialized};
ForceState state = global_state.load(std::memory_order_relaxed);
if (ABSL_PREDICT_TRUE(state == kDontForce)) return false;

if (state == kUninitialized) {
state = AbslContainerInternalSampleEverything() ? kForce : kDontForce;
global_state.store(state, std::memory_order_relaxed);
}
return state == kForce;
}

HashtablezInfo* SampleSlow(int64_t* next_sample) {
if (kAbslContainerInternalSampleEverything) {
if (ABSL_PREDICT_FALSE(ShouldForceSampling())) {
*next_sample = 1;
return HashtablezSampler::Global().Register();
}
Expand Down
2 changes: 1 addition & 1 deletion absl/container/internal/hashtablez_sampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ void SetHashtablezMaxSamples(int32_t max);
// initialization of static storage duration objects.
// The definition of this constant is weak, which allows us to inject a
// different value for it at link time.
extern "C" const bool kAbslContainerInternalSampleEverything;
extern "C" bool AbslContainerInternalSampleEverything();

} // namespace container_internal
} // namespace absl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ namespace absl {
namespace container_internal {

// See hashtablez_sampler.h for details.
extern "C" ABSL_ATTRIBUTE_WEAK const bool
kAbslContainerInternalSampleEverything = false;
extern "C" ABSL_ATTRIBUTE_WEAK bool AbslContainerInternalSampleEverything() {
return false;
}

} // namespace container_internal
} // namespace absl
4 changes: 4 additions & 0 deletions absl/random/exponential_distribution_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ using absl::random_internal::kChiSquared;
template <typename RealType>
class ExponentialDistributionTypedTest : public ::testing::Test {};

#if defined(__EMSCRIPTEN__)
using RealTypes = ::testing::Types<float, double>;
#else
using RealTypes = ::testing::Types<float, double, long double>;
#endif // defined(__EMSCRIPTEN__)
TYPED_TEST_CASE(ExponentialDistributionTypedTest, RealTypes);

TYPED_TEST(ExponentialDistributionTypedTest, SerializeTest) {
Expand Down
2 changes: 2 additions & 0 deletions absl/random/internal/iostream_state_saver_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ TEST(IOStreamStateSaver, RoundTripDoubles) {
}
}

#if !defined(__EMSCRIPTEN__)
TEST(IOStreamStateSaver, RoundTripLongDoubles) {
// Technically, C++ only guarantees that long double is at least as large as a
// double. Practically it varies from 64-bits to 128-bits.
Expand Down Expand Up @@ -349,6 +350,7 @@ TEST(IOStreamStateSaver, RoundTripLongDoubles) {
}
}
}
#endif // !defined(__EMSCRIPTEN__)

TEST(StrToDTest, DoubleMin) {
const char kV[] = "2.22507385850720138e-308";
Expand Down
5 changes: 5 additions & 0 deletions absl/random/uniform_real_distribution_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ namespace {
template <typename RealType>
class UniformRealDistributionTest : public ::testing::Test {};

#if defined(__EMSCRIPTEN__)
using RealTypes = ::testing::Types<float, double>;
#else
using RealTypes = ::testing::Types<float, double, long double>;
#endif // defined(__EMSCRIPTEN__)

TYPED_TEST_SUITE(UniformRealDistributionTest, RealTypes);

TYPED_TEST(UniformRealDistributionTest, ParamSerializeTest) {
Expand Down

0 comments on commit 25597bd

Please sign in to comment.