[hwasan] Record allocation thread id in HeapAllocationRecord

[Enna1]

Summary

Extend HeapAllocationRecord to record allocation thread id, print thread id in memory allocation stack trace.

[Enna1]

update

[Enna1]

clang-format

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.h

120

thread id is part of alloc_context_id
why do we need another field?

[Enna1]

compiler-rt/lib/hwasan/hwasan_allocator.h

120

Yes, thread id is part of alloc_context_id stored as atomic_uint64_t alloc_context_id in Metadata, u32 alloc_context_id stored in HeapAllocationRecord is without thread id.
What do you think add a function named inline u64 GetAllocContextId() const; and directly store u64 alloc_context_id in HeapAllocationRecord?

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

359

@eugenis Unrelated to the patch but seems Deallocate must be called after ha->push

compiler-rt/lib/hwasan/hwasan_allocator.h

117–118

I guess you just resolved TODO, so please remove

120

missaligned, please clang-format

120

Yes, thread id is part of alloc_context_id stored as atomic_uint64_t alloc_context_id in Metadata, u32 alloc_context_id stored in HeapAllocationRecord is without thread id.
What do you think add a function named inline u64 GetAllocContextId() const; and directly store u64 alloc_context_id in HeapAllocationRecord?

thanks for pointing to this, I didn't noticed that first time

compiler-rt/lib/hwasan/hwasan_report.cpp

334

u32 should be %u

476

481

[Enna1]

address review comments

[Enna1]

compiler-rt/lib/hwasan/hwasan_allocator.h

120

clang-format complained about keeping this aligned

[Enna1]

format

compiler-rt/lib/hwasan/hwasan_allocator.h

 
  public:
   inline void SetAllocated(u32 stack, u64 size);
   inline void SetUnallocated();
 
   inline bool IsAllocated() const;
   inline u64 GetRequestedSize() const;
   inline u32 GetAllocStackId() const;
+  inline u32 GetAllocThreadId() const;
   inline void SetLsanTag(__lsan::ChunkTag tag);
   inline __lsan::ChunkTag GetLsanTag() const;
 };
 static_assert(sizeof(Metadata) == 16);
 
 struct HwasanMapUnmapCallback {
   void OnMap(uptr p, uptr size) const { UpdateMemoryUsage(); }
   void OnUnmap(uptr p, uptr size) const {
public:
   HwasanChunkView(uptr block, Metadata *metadata)
       : block_(block), metadata_(metadata) {}
   bool IsAllocated() const;    // Checks if the memory is currently allocated
   uptr Beg() const;            // First byte of user memory
   uptr End() const;            // Last byte of user memory
   uptr UsedSize() const;       // Size requested by the user
   uptr ActualSize() const;     // Size allocated by the allocator.
   u32 GetAllocStackId() const;
+  u32 GetAllocThreadId() const;
   bool FromSmallHeap() const;
   bool AddrIsInside(uptr addr) const;
 
  private:
   friend class __lsan::LsanMetadata;
   uptr block_;
   Metadata *const metadata_;
 };
 
 HwasanChunkView FindHeapChunkByAddress(uptr address);
 
 // Information about one (de)allocation that happened in the past.
 // These are recorded in a thread-local ring buffer.
-// TODO: this is currently 24 bytes (20 bytes + alignment).
-// Compress it to 16 bytes or extend it to be more useful.
 struct HeapAllocationRecord {
   uptr tagged_addr;
-  u32  alloc_context_id;
-  u32  free_context_id;
-  u32  requested_size;
+  u32 alloc_thread_id;
+  u32 alloc_context_id;
+  u32 free_context_id;
+  u32 requested_size;
 };
 
 typedef RingBuffer<HeapAllocationRecord> HeapAllocationsRingBuffer;
 
 void GetAllocatorStats(AllocatorStatCounters s);
 
 inline bool InTaggableRegion(uptr addr) {
 #if defined(HWASAN_ALIASING_MODE)

compiler-rt/lib/hwasan/hwasan_allocator.cpp

 }
 uptr HwasanChunkView::UsedSize() const {
   return metadata_->GetRequestedSize();
 }
 u32 HwasanChunkView::GetAllocStackId() const {
   return metadata_->GetAllocStackId();
 }
 
+u32 HwasanChunkView::GetAllocThreadId() const {
+  return metadata_->GetAllocThreadId();
+}
+
 uptr HwasanChunkView::ActualSize() const {
   return allocator.GetActuallyAllocatedSize(reinterpret_cast<void *>(block_));
 }
 
 bool HwasanChunkView::FromSmallHeap() const {
   return allocator.FromPrimary(reinterpret_cast<void *>(block_));
 }
 

 inline u64 Metadata::GetRequestedSize() const {
   return (static_cast<u64>(requested_size_high) << 32) + requested_size_low;
 }
 
 inline u32 Metadata::GetAllocStackId() const {
   return atomic_load(&alloc_context_id, memory_order_relaxed);
 }
 
+inline u32 Metadata::GetAllocThreadId() const {
+  u64 context = atomic_load(&alloc_context_id, memory_order_relaxed);
+  u32 tid = context >> 32;
+  return tid;
+}
+
 void GetAllocatorStats(AllocatorStatCounters s) {
   allocator.GetStats(s);
 }
 
 inline void Metadata::SetLsanTag(__lsan::ChunkTag tag) {
   lsan_tag = tag;
 }
 
Metadata *meta =
       reinterpret_cast<Metadata *>(allocator.GetMetaData(aligned_ptr));
   if (!meta) {
     ReportInvalidFree(stack, reinterpret_cast<uptr>(tagged_ptr));
     return;
   }
   uptr orig_size = meta->GetRequestedSize();
   u32 free_context_id = StackDepotPut(*stack);
   u32 alloc_context_id = meta->GetAllocStackId();
+  u32 alloc_thread_id = meta->GetAllocThreadId();
 
   // Check tail magic.
   uptr tagged_size = TaggedSize(orig_size);
   if (flags()->free_checks_tail_magic && orig_size &&
       tagged_size != orig_size) {
     uptr tail_size = tagged_size - orig_size - 1;
     CHECK_LT(tail_size, kShadowAlignment);
     void *tail_beg = reinterpret_cast<void *>(
if (t) {
       static_assert(kFallbackFreeTag >= kShadowAlignment,
                     "fallback tag must not be a short granule tag.");
       tag = kFallbackFreeTag;
     }
     TagMemoryAligned(reinterpret_cast<uptr>(aligned_ptr), TaggedSize(orig_size),
                      tag);
   }
   if (t) {
     allocator.Deallocate(t->allocator_cache(), aligned_ptr);
     if (auto *ha = t->heap_allocations())
-      ha->push({reinterpret_cast<uptr>(tagged_ptr), alloc_context_id,
-                free_context_id, static_cast<u32>(orig_size)});
+      ha->push({reinterpret_cast<uptr>(tagged_ptr), alloc_thread_id,
+                alloc_context_id, free_context_id,
+                static_cast<u32>(orig_size)});
   } else {
     SpinMutexLock l(&fallback_mutex);
     AllocatorCache *cache = &fallback_allocator_cache;
     allocator.Deallocate(cache, aligned_ptr);
   }
 }
 
 static void *HwasanReallocate(StackTrace *stack, void *tagged_ptr_old,

compiler-rt/lib/hwasan/hwasan_report.cpp

if (chunk.IsAllocated()) {
     Printf("%s", d.Error());
     Printf("\nCause: heap-buffer-overflow\n");
     Printf("%s", d.Default());
     Printf("%s", d.Location());
     Printf("%p is located %zd bytes %s a %zd-byte region [%p,%p)\n",
            untagged_addr, offset, whence, chunk.UsedSize(), chunk.Beg(),
            chunk.End());
     Printf("%s", d.Allocation());
-    Printf("allocated here:\n");
+    Printf("allocated by thread T%u here:\n", chunk.GetAllocThreadId());
     Printf("%s", d.Default());
     GetStackTraceFromId(chunk.GetAllocStackId()).Print();
     return;
   }
   // Check whether the address points into a loaded library. If so, this is
   // most likely a global variable.
   const char *module_name;
   uptr module_address;
if (FindHeapAllocation(t->heap_allocations(), tagged_addr, &har,
       Printf("%s", d.Error());
       Printf("\nCause: use-after-free\n");
       Printf("%s", d.Location());
       Printf("%p is located %zd bytes inside a %zd-byte region [%p,%p)\n",
              untagged_addr, untagged_addr - UntagAddr(har.tagged_addr),
              har.requested_size, UntagAddr(har.tagged_addr),
              UntagAddr(har.tagged_addr) + har.requested_size);
       Printf("%s", d.Allocation());
-      Printf("freed by thread T%zd here:\n", t->unique_id());
+      Printf("freed by thread T%u here:\n", t->unique_id());
       Printf("%s", d.Default());
       GetStackTraceFromId(har.free_context_id).Print();
 
       Printf("%s", d.Allocation());
-      Printf("previously allocated here:\n", t);
+      Printf("previously allocated by thread T%u here:\n", har.alloc_thread_id);
       Printf("%s", d.Default());
       GetStackTraceFromId(har.alloc_context_id).Print();
 
       // Print a developer note: the index of this heap object
       // in the thread's deallocation ring buffer.
       Printf("hwasan_dev_note_heap_rb_distance: %zd %zd\n", ring_index + 1,
              flags()->heap_history_size);
       Printf("hwasan_dev_note_num_matching_addrs: %zd\n", num_matching_addrs);

compiler-rt/test/hwasan/TestCases/double-free.c

int main() {
   free(x);
   // CHECK: ERROR: HWAddressSanitizer: invalid-free on address {{.*}} at pc {{[0x]+}}[[PC:.*]] on thread T{{[0-9]+}}
   // CHECK: tags: [[PTR_TAG:..]]/[[MEM_TAG:..]] (ptr/mem)
   // CHECK: #0 {{[0x]+}}{{.*}}[[PC]]
   // If we instrument using calls (default on x86), free is not the top frame
   // of the fault. With TCO the free frame can be replaced with the interceptor.
   // CHECK: in {{.*}}free
   // CHECK: freed by thread {{.*}} here:
-  // CHECK: previously allocated here:
+  // CHECK: previously allocated by thread {{.*}} here:
   // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes):
   // CHECK: =>{{.*}}[[MEM_TAG]]
   fprintf(stderr, "DONE\n");
   __hwasan_disable_allocator_tagging();
 // CHECK-NOT: DONE
 }

compiler-rt/test/hwasan/TestCases/hwasan_symbolize.cpp

 
 int main(int argc, char **argv) {
   __hwasan_enable_allocator_tagging();
   char *volatile x = (char *)malloc(10);
   sink = x[100];
   // LINKIFY: <a href="http://test.invalid/hwasan_symbolize.cpp:[[@LINE-1]]">
   // CHECK: hwasan_symbolize.cpp:[[@LINE-2]]
   // CHECK: Cause: heap-buffer-overflow
-  // CHECK: allocated here:
+  // CHECK: allocated by thread {{.*}} here:
   // LINKIFY: <a href="http://test.invalid/hwasan_symbolize.cpp:[[@LINE-6]]">
   // CHECK: hwasan_symbolize.cpp:[[@LINE-7]]
   return 0;
 }

compiler-rt/test/hwasan/TestCases/realloc-after-free.c

 
 int main(int argc, char **argv) {
   __hwasan_enable_allocator_tagging();
   if (argc != 2) return 0;
   int realloc_size = atoi(argv[1]);
   char * volatile x = (char*)malloc(40);
   free(x);
   x = realloc(x, realloc_size);
   // CHECK: ERROR: HWAddressSanitizer: invalid-free on address
   // CHECK: tags: [[PTR_TAG:..]]/[[MEM_TAG:..]] (ptr/mem)
   // CHECK: freed by thread {{.*}} here:
-// CHECK: previously allocated here:
+  // CHECK: previously allocated by thread {{.*}} here:
   // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes):
   // CHECK: =>{{.*}}[[MEM_TAG]]
   fprintf(stderr, "DONE\n");
   __hwasan_disable_allocator_tagging();
 // CHECK-NOT: DONE
 }

compiler-rt/test/hwasan/TestCases/set-error-report-callback.cpp

 int main() {
   __hwasan_enable_allocator_tagging();
   __hwasan_set_error_report_callback(&callback);
   char *volatile p = (char *)malloc(16);
   p[16] = 1;
   free(p);
   // CHECK: ERROR: HWAddressSanitizer:
   // CHECK: WRITE of size 1 at
-  // CHECK: allocated here:
+  // CHECK: allocated by thread {{.*}} here:
   // CHECK: Memory tags around the buggy address
 
   // CHECK: == error start
   // CHECK: ERROR: HWAddressSanitizer:
   // CHECK: WRITE of size 1 at
-  // CHECK: allocated here:
+  // CHECK: allocated by thread {{.*}} here:
   // CHECK: Memory tags around the buggy address
   // CHECK: == error end
 }

compiler-rt/test/hwasan/TestCases/tag_in_free.c

for (int i = 0; i < 100; ++i) {
     char * volatile p = (char*)malloc(10);
     free(p);
   }
 
   char * volatile p = (char*)malloc(10);
 #ifdef MALLOC
   // MALLOC: READ of size 1 at
   // MALLOC: is located 6 bytes after a 10-byte region
-  // MALLOC: allocated here:
+  // MALLOC: allocated by thread T0 here:
   char volatile x = p[16];
 #endif
   free(p);
 #ifdef FREE
   // FREE: READ of size 1 at
   // FREE: is located 0 bytes inside a 10-byte region
   // FREE: freed by thread T0 here:
-  // FREE: previously allocated here:
+  // FREE: previously allocated by thread T0 here:
   char volatile y = p[0];
 #endif
 
   __hwasan_disable_allocator_tagging();
 
   return 0;
 }

compiler-rt/test/hwasan/TestCases/thread-uaf.c

 void *Use(void *arg) {
   x[5] = 42;
   // CHECK: ERROR: HWAddressSanitizer: tag-mismatch on address
   // CHECK: WRITE of size 1 {{.*}} in thread T3
   // CHECK: thread-uaf.c:[[@LINE-3]]
   // CHECK: Cause: use-after-free
   // CHECK: freed by thread T2 here
   // CHECK: in Deallocate
-  // CHECK: previously allocated here:
+  // CHECK: previously allocated by thread T1 here:
   // CHECK: in Allocate
   // CHECK-DAG: Thread: T2 0x
   // CHECK-DAG: Thread: T3 0x
   // CHECK-DAG: Thread: T0 0x
   // CHECK-DAG: Thread: T1 0x
   __sync_fetch_and_add(&state, 1);
   return NULL;
 }

compiler-rt/test/hwasan/TestCases/use-after-free.c

int main() {
   // CHECK: is a small unallocated heap chunk; size: {{16|32}} offset: {{5|11}}
   // CHECK: Cause: use-after-free
   // CHECK: is located 5 bytes inside a 10-byte region
   //
   // CHECK: freed by thread {{.*}} here:
   // CHECK: #0 {{.*}} in {{.*}}free{{.*}} {{.*}}hwasan_allocation_functions.cpp
   // CHECK: #1 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-19]]
 
-  // CHECK: previously allocated here:
+  // CHECK: previously allocated by thread {{.*}} here:
   // CHECK: #0 {{.*}} in {{.*}}malloc{{.*}} {{.*}}hwasan_allocation_functions.cpp
   // CHECK: #1 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-24]]
   // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes):
   // CHECK: =>{{.*}}[[MEM_TAG]]
   // CHECK: SUMMARY: HWAddressSanitizer: tag-mismatch
   return r;
 }