GWP-ASan: Refactor allocator singleton.

A subsequent refactor moves the singleton into client/. Remove it from
common, turn :common back to a static_library, and restructure the
constructor back into an Init() method to accomodate the new singleton
design.

Bug: 896019
Change-Id: I7bfe4b996b165ba34831565160c8ef46f3e48498
Reviewed-on: https://chromium-review.googlesource.com/c/1322111
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606194}

components/gwp_asan/common/BUILD.gn

@@ -4,17 +4,13 @@
 
 assert(is_win, "GWP-ASan currently only supports Windows.")
 
-component("common") {
-  output_name = "gwp_asan_common"
+static_library("common") {
   sources = [
-    "export.h",
     "guarded_page_allocator.cc",
     "guarded_page_allocator.h",
     "guarded_page_allocator_win.cc",
   ]
 
-  defines = [ "GWP_ASAN_IMPLEMENTATION" ]
-
   deps = [
     "//base",
   ]

components/gwp_asan/common/export.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef COMPONENTS_GWP_ASAN_COMMON_EXPORT_H_
-#define COMPONENTS_GWP_ASAN_COMMON_EXPORT_H_
-
-#if defined(COMPONENT_BUILD)
-#if defined(WIN32)
-
-#if defined(GWP_ASAN_IMPLEMENTATION)
-#define GWP_ASAN_EXPORT __declspec(dllexport)
-#else
-#define GWP_ASAN_EXPORT __declspec(dllimport)
-#endif  // defined(GWP_ASAN_IMPLEMENTATION)
-
-#else  // defined(WIN32)
-#if defined(GWP_ASAN_IMPLEMENTATION)
-#define GWP_ASAN_EXPORT __attribute__((visibility("default")))
-#else
-#define GWP_ASAN_EXPORT
-#endif
-#endif
-
-#else  // defined(COMPONENT_BUILD)
-#define GWP_ASAN_EXPORT
-#endif
-
-#endif  // COMPONENTS_GWP_ASAN_COMMON_EXPORT_H_

components/gwp_asan/common/guarded_page_allocator.cc

@@ -22,19 +22,9 @@ constexpr size_t GuardedPageAllocator::kGpaMaxPages;
 constexpr size_t GuardedPageAllocator::kGpaAllocAlignment;
 constexpr size_t GuardedPageAllocator::kFreePagesNumBits;
 
-GuardedPageAllocator& GuardedPageAllocator::InitializeSingleton(
-    size_t num_pages) {
-  static base::NoDestructor<GuardedPageAllocator> gpa(num_pages);
-  return *gpa;
-}
-
-GuardedPageAllocator& GuardedPageAllocator::Get() {
-  // The constructor will fail if it is called with num_pages = 0, forcing
-  // InitializeSingleton() to be called first.
-  return InitializeSingleton(0);
-}
+GuardedPageAllocator::GuardedPageAllocator() {}
 
-GuardedPageAllocator::GuardedPageAllocator(size_t num_pages) {
+void GuardedPageAllocator::Init(size_t num_pages) {
   CHECK_GT(num_pages, 0U);
   CHECK_LE(num_pages, kFreePagesNumBits);
   num_pages_ = num_pages;
@@ -42,15 +32,21 @@ GuardedPageAllocator::GuardedPageAllocator(size_t num_pages) {
   page_size_ = base::GetPageSize();
   CHECK(MapPages());
 
-  free_pages_ =
-      (num_pages_ == kFreePagesNumBits) ? ~0ULL : (1ULL << num_pages_) - 1;
+  {
+    // Obtain this lock exclusively to satisfy the thread-safety annotations,
+    // there should be no risk of a race here.
+    base::AutoLock lock(lock_);
+    free_pages_ =
+        (num_pages_ == kFreePagesNumBits) ? ~0ULL : (1ULL << num_pages_) - 1;
+  }
 
   for (size_t i = 0; i < num_pages_; i++)
     data_[i].Init();
 }
 
 GuardedPageAllocator::~GuardedPageAllocator() {
-  UnmapPages();
+  if (num_pages_)
+    UnmapPages();
 }
 
 void* GuardedPageAllocator::Allocate(size_t size, size_t align) {
@@ -119,11 +115,6 @@ size_t GuardedPageAllocator::GetRequestedSize(const void* ptr) const {
   return data_[slot].alloc_size;
 }
 
-bool GuardedPageAllocator::PointerIsMine(const void* ptr) const {
-  uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
-  return pages_base_addr_ <= addr && addr < pages_end_addr_;
-}
-
 // Selects a random slot in O(1) time by rotating the free_pages bitmap by a
 // random amount, using an intrinsic to get the least-significant 1-bit after
 // the rotation, and then computing the position of the bit before the rotation.

components/gwp_asan/common/guarded_page_allocator.h

@@ -13,7 +13,6 @@
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
 #include "base/threading/platform_thread.h"
-#include "components/gwp_asan/common/export.h"
 
 namespace gwp_asan {
 namespace internal {
@@ -23,7 +22,7 @@ namespace internal {
 // platforms.)
 unsigned CountTrailingZeroBits64(uint64_t x);
 
-class GWP_ASAN_EXPORT GuardedPageAllocator {
+class GuardedPageAllocator {
  public:
   // Maximum number of pages this class can allocate.
   static constexpr size_t kGpaMaxPages = 64;
@@ -39,13 +38,10 @@ class GWP_ASAN_EXPORT GuardedPageAllocator {
     kUnknown = 4,
   };
 
-  // Initialize the singleton. Used to configure the allocator to map memory
-  // for num_pages pages (excluding guard pages). num_pages must be in the range
-  // [1, kGpaMaxPages].
-  static GuardedPageAllocator& InitializeSingleton(size_t num_pages);
-
-  // Returns the global allocator singleton.
-  static GuardedPageAllocator& Get();
+  // Configures this allocator to map memory for num_pages pages (excluding
+  // guard pages). num_pages must be in the range [1, kGpaMaxPages]. Init should
+  // only be called once.
+  void Init(size_t num_pages);
 
   // On success, returns a pointer to size bytes of page-guarded memory. On
   // failure, returns nullptr. The allocation is not guaranteed to be
@@ -56,7 +52,7 @@ class GWP_ASAN_EXPORT GuardedPageAllocator {
   // It must be less than or equal to the allocation size. If it's left as zero
   // it will default to the default alignment the allocator chooses.
   //
-  // Precondition: align <= size <= page_size_
+  // Precondition: Init() must have been called, align <= size <= page_size_
   void* Allocate(size_t size, size_t align = 0);
 
   // Deallocates memory pointed to by ptr. ptr must have been previously
@@ -68,7 +64,10 @@ class GWP_ASAN_EXPORT GuardedPageAllocator {
   size_t GetRequestedSize(const void* ptr) const;
 
   // Returns true if ptr points to memory managed by this class.
-  bool PointerIsMine(const void* ptr) const;
+  inline bool PointerIsMine(const void* ptr) const {
+    uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
+    return pages_base_addr_ <= addr && addr < pages_end_addr_;
+  }
 
  private:
   using BitMap = uint64_t;
@@ -122,16 +121,11 @@ class GWP_ASAN_EXPORT GuardedPageAllocator {
   // Number of bits in the free_pages_ bitmap.
   static constexpr size_t kFreePagesNumBits = sizeof(BitMap) * 8;
 
-  // Configures this allocator to map memory for num_pages pages (excluding
-  // guard pages). num_pages must be in the range [1, kGpaMaxPages].
-  //
-  // Marked private so that the singleton Get() method is the only way to obtain
-  // an instance.
-  explicit GuardedPageAllocator(size_t num_pages);
+  // Does not allocate any memory for the allocator, to finish initializing call
+  // Init().
+  explicit GuardedPageAllocator();
 
-  // Unmaps memory allocated by this class.
-  //
-  // This method should be called only once to complete destruction.
+  // Unmaps memory allocated by this class, if Init was called.
   ~GuardedPageAllocator();
 
   // Maps pages into memory and sets pages_base_addr_, first_page_addr_, and
@@ -190,7 +184,7 @@ class GWP_ASAN_EXPORT GuardedPageAllocator {
   // Set to true if a double free has occurred.
   std::atomic<bool> double_free_detected_{false};
 
-  // Required to access the constructor in Get().
+  // Required for a singleton to access the constructor.
   friend base::NoDestructor<GuardedPageAllocator>;
 
   DISALLOW_COPY_AND_ASSIGN(GuardedPageAllocator);

components/gwp_asan/common/guarded_page_allocator_unittest.cc

@@ -20,8 +20,9 @@ static constexpr size_t kGpaMaxPages = GuardedPageAllocator::kGpaMaxPages;
 
 class GuardedPageAllocatorTest : public testing::Test {
  protected:
-  explicit GuardedPageAllocatorTest(size_t num_pages = kGpaMaxPages)
-      : gpa_(num_pages) {}
+  explicit GuardedPageAllocatorTest(size_t num_pages = kGpaMaxPages) {
+    gpa_.Init(num_pages);
+  }
 
   // Get a left- or right- aligned allocation (or nullptr on error.)
   char* GetAlignedAllocation(bool left_aligned, size_t sz, size_t align = 0) {