GWP-ASan: Refactor SlotMetadata

Refactor SlotMetadata to break out matching allocation/deallocation info
into a common struct.

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

components/gwp_asan/common/guarded_page_allocator.cc

@@ -94,7 +94,7 @@ void GuardedPageAllocator::Deallocate(void* ptr) {
   size_t slot = AddrToSlot(GetPageAddr(addr));
   DCHECK_EQ(ptr, data_[slot].alloc_ptr);
   // Check for double free.
-  if (data_[slot].dealloc_trace_addr) {
+  if (data_[slot].dealloc.trace_addr) {
     double_free_detected_ = true;
     *reinterpret_cast<char*>(ptr) = 'X';  // Trigger exception.
     __builtin_trap();
@@ -219,33 +219,33 @@ size_t GuardedPageAllocator::AddrToSlot(uintptr_t addr) const {
 GuardedPageAllocator::SlotMetadata::SlotMetadata() {}
 
 GuardedPageAllocator::SlotMetadata::~SlotMetadata() {
-  if (!stacktrace_alloc)
+  if (!alloc.stacktrace)
     return;
 
   Reset();
 
-  free(stacktrace_alloc);
-  free(stacktrace_dealloc);
+  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.
-  stacktrace_alloc =
-      static_cast<StackTrace*>(malloc(sizeof(*stacktrace_alloc)));
-  CHECK(stacktrace_alloc);
-  stacktrace_dealloc =
-      static_cast<StackTrace*>(malloc(sizeof(*stacktrace_dealloc)));
-  CHECK(stacktrace_dealloc);
+  alloc.stacktrace =
+      static_cast<StackTrace*>(malloc(sizeof(*alloc.stacktrace)));
+  CHECK(alloc.stacktrace);
+  dealloc.stacktrace =
+      static_cast<StackTrace*>(malloc(sizeof(*dealloc.stacktrace)));
+  CHECK(dealloc.stacktrace);
 }
 
 void GuardedPageAllocator::SlotMetadata::Reset() {
   // Destruct previous allocation/deallocation traces. The constructor was only
-  // called if (de)alloc_trace_addr is non-null.
-  if (alloc_trace_addr)
-    stacktrace_alloc->~StackTrace();
-  if (dealloc_trace_addr)
-    stacktrace_dealloc->~StackTrace();
+  // called if trace_addr is non-null.
+  if (alloc.trace_addr)
+    alloc.stacktrace->~StackTrace();
+  if (dealloc.trace_addr)
+    dealloc.stacktrace->~StackTrace();
 }
 
 void GuardedPageAllocator::SlotMetadata::RecordAllocation(size_t size,
@@ -255,19 +255,19 @@ void GuardedPageAllocator::SlotMetadata::RecordAllocation(size_t size,
   alloc_size = size;
   alloc_ptr = ptr;
 
-  alloc_tid = base::PlatformThread::CurrentId();
-  new (stacktrace_alloc) StackTrace();
-  alloc_trace_addr = stacktrace_alloc->Addresses(&alloc_trace_len);
+  alloc.tid = base::PlatformThread::CurrentId();
+  new (alloc.stacktrace) StackTrace();
+  alloc.trace_addr = alloc.stacktrace->Addresses(&alloc.trace_len);
 
-  dealloc_tid = base::kInvalidThreadId;
-  dealloc_trace_addr = nullptr;
-  dealloc_trace_len = 0;
+  dealloc.tid = base::kInvalidThreadId;
+  dealloc.trace_addr = nullptr;
+  dealloc.trace_len = 0;
 }
 
 void GuardedPageAllocator::SlotMetadata::RecordDeallocation() {
-  dealloc_tid = base::PlatformThread::CurrentId();
-  new (stacktrace_dealloc) StackTrace();
-  dealloc_trace_addr = stacktrace_dealloc->Addresses(&dealloc_trace_len);
+  dealloc.tid = base::PlatformThread::CurrentId();
+  new (dealloc.stacktrace) StackTrace();
+  dealloc.trace_addr = dealloc.stacktrace->Addresses(&dealloc.trace_len);
 }
 
 }  // namespace internal

components/gwp_asan/common/guarded_page_allocator.h

@@ -75,6 +75,27 @@ class GuardedPageAllocator {
 
   // Structure for storing data about a slot.
   struct SlotMetadata {
+    // Information saved for allocations and deallocations.
+    struct AllocationInfo {
+      // (De)allocation thread id or base::kInvalidThreadId if no (de)allocation
+      // occurred.
+      base::PlatformThreadId tid = base::kInvalidThreadId;
+
+      // Pointer to stack trace addresses or null if no (de)allocation occurred.
+      const void* const* trace_addr = nullptr;
+
+      // Stack trace length or 0 if no (de)allocation occurred.
+      size_t trace_len = 0;
+
+     private:
+      // StackTrace object for this slot, it's allocated in
+      // SlotMetadata()::Init() and only used internally, trace_addr/len should
+      // be used by external consumers of the stack trace data.
+      base::debug::StackTrace* stacktrace = nullptr;
+
+      friend struct SlotMetadata;
+    };
+
     SlotMetadata();
     ~SlotMetadata();
 
@@ -94,29 +115,13 @@ class GuardedPageAllocator {
     // The allocation address.
     void* alloc_ptr = nullptr;
 
-    // (De)allocation thread id or base::kInvalidThreadId if no (de)allocation
-    // occurred.
-    base::PlatformThreadId alloc_tid = base::kInvalidThreadId;
-    base::PlatformThreadId dealloc_tid = base::kInvalidThreadId;
-
-    // Pointer to stack trace addresses or null if no (de)allocation occurred.
-    const void* const* alloc_trace_addr = nullptr;
-    const void* const* dealloc_trace_addr = nullptr;
-
-    // Stack trace length or 0 if no (de)allocation occurred.
-    size_t alloc_trace_len = 0;
-    size_t dealloc_trace_len = 0;
+    AllocationInfo alloc;
+    AllocationInfo dealloc;
 
    private:
-    // Call destructors on stacktrace_alloc and stacktrace_dealloc if
-    // constructors for them have previously been called.
+    // Call destructors on (de)alloc.stacktrace if constructors for them have
+    // previously been called.
     void Reset();
-
-    // StackTrace objects for this slot, they are allocated by Init() and only
-    // used internally, (de)alloc_trace_addr/len should be used by external
-    // consumers of the stack trace data.
-    base::debug::StackTrace* stacktrace_alloc = nullptr;
-    base::debug::StackTrace* stacktrace_dealloc = nullptr;
   };
 
   // Number of bits in the free_pages_ bitmap.