[HWASAN] Implemented LsanMetadata in HWASAN.

[kstoimenov]

Summary

Please note that that this code is not used. It will be connected at some point when the rest of the code is ready.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.h

34–36

we should pack this into 16 bytes:

private:
atomic_uint64_t alloc_context_id;
u32 requested_size_low;
u16 requested_size_high;
public:
u8 right_aligned;
u8 lsan_tag;

37–38

please make alloc_context_id private so only it can be accessed with getters?

37–38

Naming consistency would be nice, but it's better to rename get_requested_size and set_requested_size in followup patch

51–52

please add static_assert(sizeof(Metadata) == 16)

[kstoimenov]

Addressed comments.

[kstoimenov]

compiler-rt/lib/hwasan/hwasan_allocator.h

37–38

ACK.

[kstoimenov]

Lint.

[kstoimenov]

Fixed constuctor.

[kstoimenov]

Removed ForEachChunk.

compiler-rt/lib/hwasan/hwasan_allocator.cpp

486

I am basing this code on the ASAN implentation and there it stores the asan::AsanChunk * in the metadata_ variable. I could modify this code to actually store hwasan::Metadata. What do you think?

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

91

CheckInvalidFree tries to match tag, but for LSAN we need this info even without user pointer, so we need something else

alloc_context_id is optional info, we cannot use it for the check
and get_requested_size is probably too ok for now, but TODO is not move into own flag,

note, right_aligned is always false, so unused. I recommend to remove it in a separate patch and use this byte for is_allocated

486

I am basing this code on the ASAN implentation and there it stores the asan::AsanChunk * in the metadata_ variable. I could modify this code to actually store hwasan::Metadata. What do you think?

Please clarify, I don't understand the question

495

We need something like atomic_uint8_t chunk_state of asan
LSAN one is not atomic, and I believe it's wrong.

compiler-rt/lib/hwasan/hwasan_allocator.h

38

Maybe move after the class?

[kstoimenov]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

486

What I meant is to have this code like so:

LsanMetadata::LsanMetadata(uptr chunk) {
  auto *c = chunk ? reinterpret_cast<__hwasan::HwasanChunkView *>(
                        chunk - __hwasan::kChunkHeaderSize)
                  : nullptr;
  metadata_ = c->metadata_;
}

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

495

There is no HwasanChunkView in the memory,
HwasanChunkView is a temp helper

[kstoimenov]

Rebase.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.h

39

right_aligned? something wrong with rebase

[kstoimenov]

Added chunk_state and inline.

[kstoimenov]

Renamed methods to match the reset of the code.

[kstoimenov]

Added header file.

[kstoimenov]

After rebase + merge.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

79–80

this must be be the last store
and I don't like that SetAllocContext also changes chunk_state

if you list combine them I would call it
SetAllocated(stack, size)

Still make sure atomic_store(&chunk_state is the last action on allocation HwasanAllocate
E.g. I believe zoreinit and tagging must happen before atomic_store(&chunk_state so leak sanitizer did not try to interpret data which is not allocated yet.

231

I would expect here meta->SetState
just before RunMallocHooks

499

and below

[kstoimenov]

Addressed comments.

[kstoimenov]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

75–76

Question: do we even need thread id? Where is is used?

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

75–76

asan uses thread id
lsan does not, at least yet

you you want to drop it, maybe just make "uptr requested_size"

86–89

Here we do an oppisit acquire stuff, e.g. like in AtomicallySetQuarantineFlagIfAllocated called from asan Deallocate
we need to make sure that other fields are relevant up to the point we flip chunk_state

283

alloc_context_id is better name here

302–303

// TODO: consider meta->SetUnallocated(free_context_id);

[kstoimenov]

Addressed more comments.

compiler-rt/lib/hwasan/hwasan_allocator.cpp

75–76

I will keep it as is.

283

Should we use the whole thing then? I mean stack + tid instead of just stack?

302–303

Just to confirm this would check if free_context_id is the same in SetUnallocated to make sure that we are deallocating the right chunk?

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_allocator.cpp

93–94

283

Sure, you can name it alloc_stack_id, but please rename in a separate patch together with free_context_id

302–303

No, we may use free_context_id instead of alloc_context_id.
It can be used for reporting.

Now, for some reasons, t->heap_allocations() is used to keep free_context_id

compiler-rt/lib/hwasan/hwasan_allocator.h

 
 #ifndef HWASAN_ALLOCATOR_H
 #define HWASAN_ALLOCATOR_H
 
 #include "hwasan.h"
 #include "hwasan_interface_internal.h"
 #include "hwasan_mapping.h"
 #include "hwasan_poisoning.h"
+#include "lsan/lsan_common.h"
 #include "sanitizer_common/sanitizer_allocator.h"
 #include "sanitizer_common/sanitizer_allocator_checks.h"
 #include "sanitizer_common/sanitizer_allocator_interface.h"
 #include "sanitizer_common/sanitizer_allocator_report.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_ring_buffer.h"
 
 #if !defined(__aarch64__) && !defined(__x86_64__) && !(SANITIZER_RISCV64)
 #  error Unsupported platform
 #endif
 
 namespace __hwasan {
 
 struct Metadata {
+ private:
+  atomic_uint64_t alloc_context_id;
   u32 requested_size_low;
-  u32 requested_size_high;
-  u32 alloc_context_id;
-  u64 GetRequestedSize() {
-    return (static_cast<u64>(requested_size_high) << 32) + requested_size_low;
-  }
-  void SetRequestedSize(u64 size) {
-    requested_size_low = size & ((1ul << 32) - 1);
-    requested_size_high = size >> 32;
-  }
+  u16 requested_size_high;
+  atomic_uint8_t chunk_state;
+  u8 lsan_tag;
+
+ public:
+  inline void SetAllocated(u32 stack, u64 size);
+  inline void SetUnallocated();
+
+  inline bool IsAllocated() const;
+  inline u64 GetRequestedSize() const;
+  inline u32 GetAllocStackId() 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 {
     // We are about to unmap a chunk of user memory.
     // It can return as user-requested mmap() or another thread stack.
     // Make it accessible with zero-tagged pointer.
     TagMemory(p, size, 0);
public:
   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;
   bool FromSmallHeap() 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.

compiler-rt/lib/hwasan/hwasan_allocator.cpp

 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "hwasan.h"
 #include "hwasan_allocator.h"
 #include "hwasan_checks.h"
 #include "hwasan_mapping.h"
 #include "hwasan_malloc_bisect.h"
 #include "hwasan_thread.h"
 #include "hwasan_report.h"
+#include "lsan/lsan_common.h"
 
 namespace __hwasan {
 
 static Allocator allocator;
 static AllocatorCache fallback_allocator_cache;
 static SpinMutex fallback_mutex;
 static atomic_uint8_t hwasan_allocator_tagging_enabled;
 
 static constexpr tag_t kFallbackAllocTag = 0xBB & kTagMask;
 static constexpr tag_t kFallbackFreeTag = 0xBC;
 
-enum RightAlignMode {
-  kRightAlignNever,
-  kRightAlignSometimes,
-  kRightAlignAlways
+enum {
+  // Either just allocated by underlying allocator, but AsanChunk is not yet
+  // ready, or almost returned to undelying allocator and AsanChunk is already
+  // meaningless.
+  CHUNK_INVALID = 0,
+  // The chunk is allocated and not yet freed.
+  CHUNK_ALLOCATED = 1,
 };
 
+
 // Initialized in HwasanAllocatorInit, an never changed.
 static ALIGNED(16) u8 tail_magic[kShadowAlignment - 1];
 
 bool HwasanChunkView::IsAllocated() const {
-  return metadata_ && metadata_->alloc_context_id &&
-         metadata_->GetRequestedSize();
+  return metadata_ && metadata_->IsAllocated();
 }
 
 uptr HwasanChunkView::Beg() const {
   return block_;
 }
 uptr HwasanChunkView::End() const {
   return Beg() + UsedSize();
 }
 uptr HwasanChunkView::UsedSize() const {
   return metadata_->GetRequestedSize();
 }
 u32 HwasanChunkView::GetAllocStackId() const {
-  return metadata_->alloc_context_id;
+  return metadata_->GetAllocStackId();
 }
 
 uptr HwasanChunkView::ActualSize() const {
   return allocator.GetActuallyAllocatedSize(reinterpret_cast<void *>(block_));
 }
 
 bool HwasanChunkView::FromSmallHeap() const {
   return allocator.FromPrimary(reinterpret_cast<void *>(block_));
 }
 
+inline void Metadata::SetAllocated(u32 stack, u64 size) {
+  Thread *t = GetCurrentThread();
+  u64 context = t ? t->unique_id() : kMainTid;
+  context <<= 32;
+  context += stack;
+  requested_size_low = size & ((1ul << 32) - 1);
+  requested_size_high = size >> 32;
+  atomic_store(&alloc_context_id, context, memory_order_relaxed);
+  atomic_store(&chunk_state, CHUNK_ALLOCATED, memory_order_release);
+}
+
+inline void Metadata::SetUnallocated() {
+  atomic_store(&chunk_state, CHUNK_INVALID, memory_order_release);
+  requested_size_low = 0;
+  requested_size_high = 0;
+  atomic_store(&alloc_context_id, 0, memory_order_relaxed);
+}
+
+inline bool Metadata::IsAllocated() const {
+  return atomic_load(&chunk_state, memory_order_relaxed) == CHUNK_ALLOCATED &&
+         GetRequestedSize();
+}
+
+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);
+}
+
+static const uptr kChunkHeaderSize = sizeof(HwasanChunkView);
+
 void GetAllocatorStats(AllocatorStatCounters s) {
   allocator.GetStats(s);
 }
 
+inline void Metadata::SetLsanTag(__lsan::ChunkTag tag) {
+  lsan_tag = tag;
+}
+
+inline __lsan::ChunkTag Metadata::GetLsanTag() const {
+  return static_cast<__lsan::ChunkTag>(lsan_tag);
+}
+
 uptr GetAliasRegionStart() {
 #if defined(HWASAN_ALIASING_MODE)
   constexpr uptr kAliasRegionOffset = 1ULL << (kTaggableRegionCheckShift - 1);
   uptr AliasRegionStart =
       __hwasan_shadow_memory_dynamic_address + kAliasRegionOffset;
 
   CHECK_EQ(AliasRegionStart >> kTaggableRegionCheckShift,
            __hwasan_shadow_memory_dynamic_address >> kTaggableRegionCheckShift);
if (t) {
     allocated = allocator.Allocate(cache, size, alignment);
   }
   if (UNLIKELY(!allocated)) {
     SetAllocatorOutOfMemory();
     if (AllocatorMayReturnNull())
       return nullptr;
     ReportOutOfMemory(size, stack);
   }
-  Metadata *meta =
-      reinterpret_cast<Metadata *>(allocator.GetMetaData(allocated));
-  meta->SetRequestedSize(orig_size);
-  meta->alloc_context_id = StackDepotPut(*stack);
   if (zeroise) {
     internal_memset(allocated, 0, size);
   } else if (flags()->max_malloc_fill_size > 0) {
     uptr fill_size = Min(size, (uptr)flags()->max_malloc_fill_size);
     internal_memset(allocated, flags()->malloc_fill_byte, fill_size);
   }
   if (size != orig_size) {
     u8 *tail = reinterpret_cast<u8 *>(allocated) + orig_size;
if (flags()->tag_in_malloc && malloc_bisect(stack, orig_size)) {
             reinterpret_cast<u8 *>(allocated) + full_granule_size;
         TagMemoryAligned((uptr)short_granule, kShadowAlignment,
                          tag_size % kShadowAlignment);
         short_granule[kShadowAlignment - 1] = tag;
       }
     } else {
       user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
     }
   }
 
+  Metadata *meta =
+      reinterpret_cast<Metadata *>(allocator.GetMetaData(allocated));
+  meta->SetAllocated(StackDepotPut(*stack), orig_size);
   RunMallocHooks(user_ptr, size);
   return user_ptr;
 }
 
 static bool PointerAndMemoryTagsMatch(void *tagged_ptr) {
   CHECK(tagged_ptr);
   uptr tagged_uptr = reinterpret_cast<uptr>(tagged_ptr);
   if (!InTaggableRegion(tagged_uptr))
static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) {
   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->alloc_context_id;
+  u32 alloc_context_id = meta->GetAllocStackId();
 
   // 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 *>(
         reinterpret_cast<uptr>(aligned_ptr) + orig_size);
     tag_t short_granule_memtag = *(reinterpret_cast<tag_t *>(
         reinterpret_cast<uptr>(tail_beg) + tail_size));
     if (tail_size &&
         (internal_memcmp(tail_beg, tail_magic, tail_size) ||
          (in_taggable_region && pointer_tag != short_granule_memtag)))
       ReportTailOverwritten(stack, reinterpret_cast<uptr>(tagged_ptr),
                             orig_size, tail_magic);
   }
 
-  meta->SetRequestedSize(0);
-  meta->alloc_context_id = 0;
+  // TODO(kstoimenov): consider meta->SetUnallocated(free_context_id).
+  meta->SetUnallocated();
   // This memory will not be reused by anyone else, so we are free to keep it
   // poisoned.
   Thread *t = GetCurrentThread();
   if (flags()->max_free_fill_size > 0) {
     uptr fill_size =
         Min(TaggedSize(orig_size), (uptr)flags()->max_free_fill_size);
     internal_memset(aligned_ptr, flags()->free_fill_byte, fill_size);
   }

 }
 
 void hwasan_free(void *ptr, StackTrace *stack) {
   return HwasanDeallocate(stack, ptr);
 }
 
 }  // namespace __hwasan
 
+// --- Implementation of LSan-specific functions --- {{{1
+namespace __lsan {
+
+LsanMetadata::LsanMetadata(uptr chunk) {
+  metadata_ = chunk ? reinterpret_cast<__hwasan::Metadata *>(
+                          chunk - __hwasan::kChunkHeaderSize)
+                    : nullptr;
+}
+
+bool LsanMetadata::allocated() const {
+  if (!metadata_)
+    return false;
+  __hwasan::Metadata *m = reinterpret_cast<__hwasan::Metadata *>(metadata_);
+  return m->IsAllocated();
+}
+
+ChunkTag LsanMetadata::tag() const {
+  __hwasan::Metadata *m = reinterpret_cast<__hwasan::Metadata *>(metadata_);
+  return m->GetLsanTag();
+}
+
+void LsanMetadata::set_tag(ChunkTag value) {
+  __hwasan::Metadata *m = reinterpret_cast<__hwasan::Metadata *>(metadata_);
+  m->SetLsanTag(value);
+}
+
+uptr LsanMetadata::requested_size() const {
+  __hwasan::Metadata *m = reinterpret_cast<__hwasan::Metadata *>(metadata_);
+  return m->GetRequestedSize();
+}
+
+u32 LsanMetadata::stack_trace_id() const {
+  __hwasan::Metadata *m = reinterpret_cast<__hwasan::Metadata *>(metadata_);
+  return m->GetAllocStackId();
+}
+
+}  // namespace __lsan
+
 using namespace __hwasan;
 
 void __hwasan_enable_allocator_tagging() {
   atomic_store_relaxed(&hwasan_allocator_tagging_enabled, 1);
 }
 
 void __hwasan_disable_allocator_tagging() {
   atomic_store_relaxed(&hwasan_allocator_tagging_enabled, 0);