GWP-ASan: Refactor SlotMetadata

Get rid of the SlotMetadata constructor/destructor and replace the
destructor with a Destroy() method. Also change alloc_ptr to be a
uintptr_t. This refactor makes SlotMetadata safer to initialize with
out-of-process memory in an upcoming crash handler change, because the
SlotMetadata we read should not be automatically destructed (the members
will be from another memory space). Since alloc_ptr is only used in the
crash handler there's no reason to not make it a uintptr_t to make it
appear less like a pointer in crash handler's memory space.

Bug: 896019
Change-Id: I3f1f22eac6277722de9594a3331b8daef618d29d
Reviewed-on: https://chromium-review.googlesource.com/c/1340868
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608935}

components/gwp_asan/common/guarded_page_allocator.cc

@@ -45,8 +45,12 @@ void GuardedPageAllocator::Init(size_t num_pages) {
 }
 
 GuardedPageAllocator::~GuardedPageAllocator() {
-  if (num_pages_)
+  if (num_pages_) {
     UnmapPages();
+
+    for (size_t i = 0; i < num_pages_; i++)
+      data_[i].Destroy();
+  }
 }
 
 void* GuardedPageAllocator::Allocate(size_t size, size_t align) {
@@ -92,7 +96,7 @@ void GuardedPageAllocator::Deallocate(void* ptr) {
   MarkPageInaccessible(reinterpret_cast<void*>(GetPageAddr(addr)));
 
   size_t slot = AddrToSlot(GetPageAddr(addr));
-  DCHECK_EQ(ptr, data_[slot].alloc_ptr);
+  DCHECK_EQ(addr, data_[slot].alloc_ptr);
   // Check for double free.
   if (data_[slot].dealloc.trace_addr) {
     double_free_detected_ = true;
@@ -110,7 +114,7 @@ size_t GuardedPageAllocator::GetRequestedSize(const void* ptr) const {
   DCHECK(PointerIsMine(ptr));
   const uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
   size_t slot = AddrToSlot(GetPageAddr(addr));
-  DCHECK_EQ(ptr, data_[slot].alloc_ptr);
+  DCHECK_EQ(addr, data_[slot].alloc_ptr);
   return data_[slot].alloc_size;
 }
 
@@ -216,18 +220,6 @@ size_t GuardedPageAllocator::AddrToSlot(uintptr_t addr) const {
   return slot;
 }
 
-GuardedPageAllocator::SlotMetadata::SlotMetadata() {}
-
-GuardedPageAllocator::SlotMetadata::~SlotMetadata() {
-  if (!alloc.stacktrace)
-    return;
-
-  Reset();
-
-  free(alloc.stacktrace);
-  free(dealloc.stacktrace);
-}
-
 void GuardedPageAllocator::SlotMetadata::Init() {
   // new is not used so that we can explicitly call the constructor when we
   // want to collect a stack trace.
@@ -239,6 +231,16 @@ void GuardedPageAllocator::SlotMetadata::Init() {
   CHECK(dealloc.stacktrace);
 }
 
+void GuardedPageAllocator::SlotMetadata::Destroy() {
+  CHECK(alloc.stacktrace);
+  CHECK(dealloc.stacktrace);
+
+  Reset();
+
+  free(alloc.stacktrace);
+  free(dealloc.stacktrace);
+}
+
 void GuardedPageAllocator::SlotMetadata::Reset() {
   // Destruct previous allocation/deallocation traces. The constructor was only
   // called if trace_addr is non-null.
@@ -253,7 +255,7 @@ void GuardedPageAllocator::SlotMetadata::RecordAllocation(size_t size,
   Reset();
 
   alloc_size = size;
-  alloc_ptr = ptr;
+  alloc_ptr = reinterpret_cast<uintptr_t>(ptr);
 
   alloc.tid = base::PlatformThread::CurrentId();
   new (alloc.stacktrace) StackTrace();

components/gwp_asan/common/guarded_page_allocator.h

@@ -96,14 +96,14 @@ class GuardedPageAllocator {
       friend struct SlotMetadata;
     };
 
-    SlotMetadata();
-    ~SlotMetadata();
-
     // Allocate internal data (StackTraces) for this slot. StackTrace objects
     // are large so we only allocate them if they're required (instead of
     // having them be statically allocated in the SlotMetadata itself.)
     void Init();
 
+    // Frees internal data. May only be called after Init().
+    void Destroy();
+
     // Update slot metadata on an allocation with the given size and pointer.
     void RecordAllocation(size_t size, void* ptr);
 
@@ -113,7 +113,7 @@ class GuardedPageAllocator {
     // Size of the allocation
     size_t alloc_size = 0;
     // The allocation address.
-    void* alloc_ptr = nullptr;
+    uintptr_t alloc_ptr = 0;
 
     AllocationInfo alloc;
     AllocationInfo dealloc;