From 681db064d221e9eef024ce0aef6165caa37fbfd2 Mon Sep 17 00:00:00 2001 From: Kunqiu Chen Date: Thu, 19 Jun 2025 16:25:51 +0800 Subject: [PATCH] [TSan] Make Shadow/Meta region inclusive-exclusive (#144647) This commit changes the interval shadow/meta address check from inclusive-inclusive ( $[\mathrm{start}, \mathrm{end}]$ ) to inclusive-exclusive ( $[\mathrm{start}, \mathrm{end})$ ), to resolve the ambiguity of the end point address. This also aligns the logic with the check for `isAppMem` (i.e., inclusive-exclusive), ensuring consistent behavior across all memory classifications. 1. The `isShadowMem` and `isMetaMem` checks previously used an inclusive-inclusive interval, i.e., $[\mathrm{start}, \mathrm{end}]$, which could lead to a boundary address being incorrectly classified as both Shadow and Meta memory, e.g., 0x3000_0000_0000 in `Mapping48AddressSpace`. - What's more, even when Shadow doesn't border Meta, `ShadowMem::end` cannot be considered a legal shadow address, as TSan protects the gap, i.e., `ProtectRange(ShadowEnd(), MetaShadowBeg());` 2. `ShadowMem`/`MetaMem` addresses are derived from `AppMem` using an affine-like transformation (`* factor + bias`). This transformation includes two extra modifications: high- and low-order bits are masked out, and for Shadow Memory, an optional XOR operation may be applied to prevent conflicts with certain AppMem regions. - Given that all AppMem regions are defined as inclusive-exclusive intervals, $[\mathrm{start}, \mathrm{end})$, the resulting Shadow/Meta regions should logically also be inclusive-exclusive. Note: This change is purely for improving code consistency and should have no functional impact. In practice, the exact endpoint addresses of the Shadow/Meta regions are generally not reached. --- compiler-rt/lib/tsan/rtl/tsan_platform.h | 4 ++-- compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform.h b/compiler-rt/lib/tsan/rtl/tsan_platform.h index 354f6da6a64a..ada594bc11fc 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_platform.h +++ b/compiler-rt/lib/tsan/rtl/tsan_platform.h @@ -931,7 +931,7 @@ bool IsAppMem(uptr mem) { return SelectMapping(mem); } struct IsShadowMemImpl { template static bool Apply(uptr mem) { - return mem >= Mapping::kShadowBeg && mem <= Mapping::kShadowEnd; + return mem >= Mapping::kShadowBeg && mem < Mapping::kShadowEnd; } }; @@ -943,7 +943,7 @@ bool IsShadowMem(RawShadow *p) { struct IsMetaMemImpl { template static bool Apply(uptr mem) { - return mem >= Mapping::kMetaShadowBeg && mem <= Mapping::kMetaShadowEnd; + return mem >= Mapping::kMetaShadowBeg && mem < Mapping::kMetaShadowEnd; } }; diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp index cf07686d968d..dbdc6359d92a 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp @@ -525,7 +525,7 @@ SECOND: void ShadowSet(RawShadow* p, RawShadow* end, RawShadow v) { DCHECK_LE(p, end); DCHECK(IsShadowMem(p)); - DCHECK(IsShadowMem(end)); + DCHECK(p == end || IsShadowMem(end - 1)); UNUSED const uptr kAlign = kShadowCnt * kShadowSize; DCHECK_EQ(reinterpret_cast(p) % kAlign, 0); DCHECK_EQ(reinterpret_cast(end) % kAlign, 0); @@ -669,7 +669,7 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) { RawShadow* shadow_mem = MemToShadow(addr); DPrintf2("#%d: MemoryAccessRange: @%p %p size=%d is_read=%d\n", thr->tid, (void*)pc, (void*)addr, (int)size, is_read); - + DCHECK_NE(size, 0); #if SANITIZER_DEBUG if (!IsAppMem(addr)) { Printf("Access to non app mem start: %p\n", (void*)addr);