From 82b1e3b4122d8602556fbd439c7e5fc1f7f7eb4f Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 26 Apr 2023 16:26:48 -0700 Subject: [PATCH] [HWASAN] Use InTaggableRegion in basic tagging functions For primary use-case when !HWASAN_ALIASING_MODE the function is constant true and should be eliminated by optimizations. In case HWASAN_ALIASING_MODE all new calls to the functions were missing in the first place. We just not use this mode for anything but tests, so we didn't noticed. Addressing @thurston comment on D149293 Reviewed By: thurston Differential Revision: https://reviews.llvm.org/D149305 --- compiler-rt/lib/hwasan/hwasan.h | 9 +++++--- compiler-rt/lib/hwasan/hwasan_allocator.cpp | 35 +++++++++++------------------ 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/compiler-rt/lib/hwasan/hwasan.h b/compiler-rt/lib/hwasan/hwasan.h index 0e85b1b..37ef482 100644 --- a/compiler-rt/lib/hwasan/hwasan.h +++ b/compiler-rt/lib/hwasan/hwasan.h @@ -90,11 +90,12 @@ static inline bool InTaggableRegion(uptr addr) { } static inline tag_t GetTagFromPointer(uptr p) { - return (p >> kAddressTagShift) & kTagMask; + return InTaggableRegion(p) ? ((p >> kAddressTagShift) & kTagMask) : 0; } static inline uptr UntagAddr(uptr tagged_addr) { - return tagged_addr & ~kAddressTagMask; + return InTaggableRegion(tagged_addr) ? (tagged_addr & ~kAddressTagMask) + : tagged_addr; } static inline void *UntagPtr(const void *tagged_ptr) { @@ -103,7 +104,9 @@ static inline void *UntagPtr(const void *tagged_ptr) { } static inline uptr AddTagToPointer(uptr p, tag_t tag) { - return (p & ~kAddressTagMask) | ((uptr)tag << kAddressTagShift); + return InTaggableRegion(p) + ? ((p & ~kAddressTagMask) | ((uptr)tag << kAddressTagShift)) + : p; } namespace __hwasan { diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp index e7650c0..3b59741 100644 --- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp +++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp @@ -290,9 +290,7 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { CHECK(tagged_ptr); RunFreeHooks(tagged_ptr); - bool in_taggable_region = - InTaggableRegion(reinterpret_cast(tagged_ptr)); - void *untagged_ptr = in_taggable_region ? UntagPtr(tagged_ptr) : tagged_ptr; + void *untagged_ptr = UntagPtr(tagged_ptr); if (CheckInvalidFree(stack, untagged_ptr, tagged_ptr)) return; @@ -311,6 +309,9 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { u32 alloc_context_id = meta->GetAllocStackId(); u32 alloc_thread_id = meta->GetAllocThreadId(); + bool in_taggable_region = + InTaggableRegion(reinterpret_cast(tagged_ptr)); + // Check tail magic. uptr tagged_size = TaggedSize(orig_size); if (flags()->free_checks_tail_magic && orig_size && @@ -373,10 +374,7 @@ static void HwasanDeallocate(StackTrace *stack, void *tagged_ptr) { static void *HwasanReallocate(StackTrace *stack, void *tagged_ptr_old, uptr new_size, uptr alignment) { - void *untagged_ptr_old = - InTaggableRegion(reinterpret_cast(tagged_ptr_old)) - ? UntagPtr(tagged_ptr_old) - : tagged_ptr_old; + void *untagged_ptr_old = UntagPtr(tagged_ptr_old); if (CheckInvalidFree(stack, untagged_ptr_old, tagged_ptr_old)) return nullptr; void *tagged_ptr_new = @@ -384,10 +382,7 @@ static void *HwasanReallocate(StackTrace *stack, void *tagged_ptr_old, if (tagged_ptr_old && tagged_ptr_new) { Metadata *meta = reinterpret_cast(allocator.GetMetaData(untagged_ptr_old)); - void *untagged_ptr_new = - InTaggableRegion(reinterpret_cast(tagged_ptr_new)) - ? UntagPtr(tagged_ptr_new) - : tagged_ptr_new; + void *untagged_ptr_new = UntagPtr(tagged_ptr_new); internal_memcpy(untagged_ptr_new, untagged_ptr_old, Min(new_size, static_cast(meta->GetRequestedSize()))); HwasanDeallocate(stack, tagged_ptr_old); @@ -416,8 +411,7 @@ HwasanChunkView FindHeapChunkByAddress(uptr address) { } static const void *AllocationBegin(const void *p) { - const void *untagged_ptr = - InTaggableRegion(reinterpret_cast(p)) ? UntagPtr(p) : p; + const void *untagged_ptr = UntagPtr(p); if (!untagged_ptr) return nullptr; @@ -434,8 +428,7 @@ static const void *AllocationBegin(const void *p) { } static uptr AllocationSize(const void *p) { - const void *untagged_ptr = - InTaggableRegion(reinterpret_cast(p)) ? UntagPtr(p) : p; + const void *untagged_ptr = UntagPtr(p); if (!untagged_ptr) return 0; const void *beg = allocator.GetBlockBegin(untagged_ptr); if (!beg) @@ -549,7 +542,7 @@ void GetAllocatorGlobalRange(uptr *begin, uptr *end) { } uptr PointsIntoChunk(void *p) { - p = InTaggableRegion(reinterpret_cast(p)) ? UntagPtr(p) : p; + p = UntagPtr(p); uptr addr = reinterpret_cast(p); uptr chunk = reinterpret_cast(__hwasan::allocator.GetBlockBeginFastLocked(p)); @@ -567,8 +560,7 @@ uptr PointsIntoChunk(void *p) { } uptr GetUserBegin(uptr chunk) { - if (InTaggableRegion(chunk)) - CHECK_EQ(UntagAddr(chunk), chunk); + CHECK_EQ(UntagAddr(chunk), chunk); void *block = __hwasan::allocator.GetBlockBeginFastLocked( reinterpret_cast(chunk)); if (!block) @@ -582,15 +574,14 @@ uptr GetUserBegin(uptr chunk) { } uptr GetUserAddr(uptr chunk) { - tag_t mem_tag = *(tag_t *)__hwasan::MemToShadow(chunk); if (!InTaggableRegion(chunk)) return chunk; + tag_t mem_tag = *(tag_t *)__hwasan::MemToShadow(chunk); return AddTagToPointer(chunk, mem_tag); } LsanMetadata::LsanMetadata(uptr chunk) { - if (InTaggableRegion(chunk)) - CHECK_EQ(UntagAddr(chunk), chunk); + CHECK_EQ(UntagAddr(chunk), chunk); metadata_ = chunk ? __hwasan::allocator.GetMetaData(reinterpret_cast(chunk)) : nullptr; @@ -628,7 +619,7 @@ void ForEachChunk(ForEachChunkCallback callback, void *arg) { } IgnoreObjectResult IgnoreObject(const void *p) { - p = InTaggableRegion(reinterpret_cast(p)) ? UntagPtr(p) : p; + p = UntagPtr(p); uptr addr = reinterpret_cast(p); uptr chunk = reinterpret_cast(__hwasan::allocator.GetBlockBegin(p)); if (!chunk) -- 2.7.4