Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<locale>: Double-checked locking for locale::classic #3048

Merged
merged 8 commits into from
Oct 27, 2022
3 changes: 2 additions & 1 deletion benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,6 @@ function(add_benchmark name)
target_compile_definitions(benchmark-${name} PRIVATE BENCHMARK_STATIC_DEFINE)
endfunction()

add_benchmark(std_copy src/std_copy.cpp)
add_benchmark(locale_classic src/locale_classic.cpp)
add_benchmark(random_integer_generation src/random_integer_generation.cpp)
add_benchmark(std_copy src/std_copy.cpp)
16 changes: 16 additions & 0 deletions benchmarks/src/locale_classic.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <benchmark/benchmark.h>
#include <locale>
using namespace std;

// GH-3048 <locale>: Double-checked locking for locale::classic
static void BM_locale_classic(benchmark::State& state) {
for (auto _ : state) {
benchmark::DoNotOptimize(locale::classic());
}
}
BENCHMARK(BM_locale_classic);

BENCHMARK_MAIN();
36 changes: 32 additions & 4 deletions stl/src/locale0.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <crtdbg.h>
#include <internal_shared.h>
#include <xatomic.h>
#include <xfacet>

// This must be as small as possible, because its contents are
Expand Down Expand Up @@ -136,7 +137,22 @@ __PURE_APPDOMAIN_GLOBAL locale::id ctype<unsigned short>::id(0);
__PURE_APPDOMAIN_GLOBAL locale::id codecvt<unsigned short, char, mbstate_t>::id(0);

_MRTIMP2_PURE const locale& __CLRCALL_PURE_OR_CDECL locale::classic() { // get reference to "C" locale
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Init();
#if !defined(_M_CEE_PURE)
const auto mem = reinterpret_cast<const intptr_t*>(&locale::_Locimp::_Clocptr);
intptr_t as_bytes;
#ifdef _WIN64
as_bytes = __iso_volatile_load64(mem);
#else // ^^^ 64-bit / 32-bit vvv
as_bytes = __iso_volatile_load32(mem);
#endif // ^^^ 32-bit ^^^
_Compiler_or_memory_barrier();
const auto ptr = reinterpret_cast<locale::_Locimp*>(as_bytes);
if (ptr == nullptr)
#endif // !defined(_M_CEE_PURE)
{
_Init();
}

return classic_locale;
}

Expand All @@ -157,9 +173,21 @@ _MRTIMP2_PURE locale::_Locimp* __CLRCALL_PURE_OR_CDECL locale::_Init(bool _Do_in
ptr->_Catmask = all; // set current locale to "C"
ptr->_Name = "C";

_Locimp::_Clocptr = ptr; // set classic to match
_Locimp::_Clocptr->_Incref();
::new (&classic_locale) locale(_Locimp::_Clocptr);
// set classic to match
ptr->_Incref();
::new (&classic_locale) locale(ptr);
#if defined(_M_CEE_PURE)
locale::_Locimp::_Clocptr = ptr;
#else // ^^^ _M_CEE_PURE / !_M_CEE_PURE vvv
const auto mem = reinterpret_cast<volatile intptr_t*>(&locale::_Locimp::_Clocptr);
const auto as_bytes = reinterpret_cast<intptr_t>(ptr);
_Compiler_or_memory_barrier();
#ifdef _WIN64
__iso_volatile_store64(mem, as_bytes);
#else // ^^^ 64-bit / 32-bit vvv
__iso_volatile_store32(mem, as_bytes);
#endif // ^^^ 32-bit ^^^
#endif // ^^^ !defined(_M_CEE_PURE) ^^^
}

if (_Do_incref) {
Expand Down