android: reachedcode: avoid a global std::atomic

In C++20 constructing an std::atomic is not 'trivial'. The various
reasons for that are reachable from this discussion: [1]. The change
will prevent the array from being put to BSS and be cheap when unused.

Make the array a regular uint32_t array and refer to it as an array of
std::atomic later (using reinterpret_cast). The std::atomic has the
standard layout by the spec, making it hard to imagine an implementation
of C++ that would construct the std::atomic<uint32_t> to anything
different than (uint32_t)0.

The Linux/Android memory page zero-fill is atomic and can only be done
once. Therefore the new behavior should be equivalent to the current
one. The code generated by Clang today is identical with/without this
patch.

This large chunk of zeroes in BSS still _may_ have a performance or code
size impact because of increased immediates in relative references to
.data and .bss. Resolving it may be even more controversial, hence
suggesting to do it only if there is a confirmed regression.

Disclaimer: I know this is .. bad. And I feel bad.

[1] Discussion: <atomic>: In pre-C++20 mode, the constructor should be
    trivial
    microsoft/STL#661

Bug: None
Change-Id: I06798f225bd557844072d154d47ace2d85606df5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3282196
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#943056}

base/android/reached_addresses_bitset.cc

@@ -14,18 +14,35 @@ namespace android {
 namespace {
 constexpr size_t kBitsPerElement = sizeof(uint32_t) * 8;
 
-#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
+// Below an array of uint32_t in BSS is introduced and then casted to an array
+// of std::atomic<uint32_t>. In C++20 constructing an std::atomic is not
+// 'trivial'. See https://github.com/microsoft/STL/issues/661 for reasons of
+// this change in the standard.
+//
+// Assert that both types have the same size. The sizes do not have to match
+// according to a note in [atomics.types.generic] in C++17. With this assertion
+// in place it is unlikely that the constructor produces the value other than
+// (uint32_t)0.
+static_assert(sizeof(uint32_t) == sizeof(std::atomic<uint32_t>), "");
+
+// Keep the array in BSS only for non-official builds to avoid potential harm to
+// data locality and unspecified behavior from the reinterpret_cast below. In
+// order to start new experiments with base::Feature(ReachedCodeProfiler) on
+// Canary/Dev this array will need to be reintroduced to official builds.
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING) && !defined(OFFICIAL_BUILD)
 // Enough for 1 << 29 bytes of code, 512MB.
 constexpr size_t kTextBitfieldSize = 1 << 20;
-std::atomic<uint32_t> g_text_bitfield[kTextBitfieldSize];
+uint32_t g_text_bitfield[kTextBitfieldSize];
 #endif
 }  // namespace
 
 // static
 ReachedAddressesBitset* ReachedAddressesBitset::GetTextBitset() {
-#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
-  static ReachedAddressesBitset text_bitset(kStartOfText, kEndOfText,
-                                            g_text_bitfield, kTextBitfieldSize);
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING) && !defined(OFFICIAL_BUILD)
+  static ReachedAddressesBitset text_bitset(
+      kStartOfText, kEndOfText,
+      reinterpret_cast<std::atomic<uint32_t>*>(g_text_bitfield),
+      kTextBitfieldSize);
   return &text_bitset;
 #else
   return nullptr;