[compiler-rt] [TSan] Fix potentially problematic shadow end calculations (PR #144648)
Kunqiu Chen via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 18 00:13:34 PDT 2025
https://github.com/Camsyn created https://github.com/llvm/llvm-project/pull/144648
This is an improvement that enhances the robustness of the code.
Previously, the correct calculation of exclusive EndShadow relied on the assumption that `addr_end % kShadowCell == 0`; however, in many current usages, this assumption was not strictly guaranteed (although it did in fact meet).
In addition, computing EndShadow does not require the corresponding address to be AppMem; for example, HighAppEnd is not AppMem, but can still be used to calculate EndShadow.
For example, for the AppMem range [0, 1), `s = MemToShadow(0)` is equal to `MemToShadow(1)`. The previous logic would incorrectly deduce an empty shadow range [s, s).
The correct shadow range should be [s, s + kShadowSize * kShadowCnt) to cover all the shadow memory for the accessed cell.
This commit addresses this in two ways:
1. It introduces a dedicated utility function, i.e., `MemToEndShadow`, to correctly calculate the end of a shadow memory range, accounting for the memory cell granularity.
2. It replaces existing (and potentially incorrect) calculations of the shadow end with this new utility function.
Additionally, the previous commit 4052de6 resolved a problem with overestimating the shadow end; it did not consider `kShadowCell` and could therefore lead to underestimates.
This is also corrected by utilizing the `MemToEndShadow` function.
>From 49f9872d3fa7b476bc55386b3340322d66f90d7d Mon Sep 17 00:00:00 2001
From: Camsyn <camsyn at foxmail.com>
Date: Tue, 17 Jun 2025 15:33:15 +0800
Subject: [PATCH] [TSan] Fix potentially problematic shadow end calculations
This is an improvement that enhances the robustness of the code.
Previously, the correct calculation of exclusive EndShadow relied on the
assumption that `addr_end % kShadowCell == 0`; however, in many current
usages, this assumption was not strictly guaranteed (although it did in
fact meet).
In addition, computing EndShadow does not require the corresponding
address to be AppMem; for example, HighAppEnd is not AppMem, but can
still be used to calculate EndShadow.
For example, for the AppMem range [0, 1), `s = MemToShadow(0)` is equal
to `MemToShadow(1)`. The previous logic would incorrectly deduce an
empty shadow range [s, s) while the correct shadow range should be
[s, s + kShadowSize * kShadowCnt) to cover all the related shadow memory
for the accessed cell.
This commit addresses this in two ways:
1. It introduces a dedicated utility function, i.e., `MemToEndShadow`,
to correctly calculate the end of a shadow memory range, accounting
for the memory cell granularity.
2. It replaces existing (and potentially incorrect) calculations of the
shadow end with this new utility function.
Additionally, the previous commit 4052de6 resolved a problem with
overestimating the shadow end; it did not consider `kShadowCell` and
could therefore lead to underestimates. This is also corrected by
utilizing the `MemToEndShadow` function.
---
.../lib/tsan/rtl/tsan_interface_java.cpp | 3 +--
compiler-rt/lib/tsan/rtl/tsan_platform.h | 18 ++++++++++++++++++
.../lib/tsan/rtl/tsan_platform_linux.cpp | 2 +-
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp | 6 +++---
compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp | 9 ++++-----
5 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
index 7c15a16388268..3e324eef9cb8d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
@@ -122,7 +122,6 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) {
DCHECK_GE(dst, jctx->heap_begin);
DCHECK_LE(dst + size, jctx->heap_begin + jctx->heap_size);
DCHECK_NE(dst, src);
- DCHECK_NE(size, 0);
// Assuming it's not running concurrently with threads that do
// memory accesses and mutex operations (stop-the-world phase).
@@ -132,7 +131,7 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) {
// We used to move shadow from src to dst, but the trace format does not
// support that anymore as it contains addresses of accesses.
RawShadow *d = MemToShadow(dst);
- RawShadow *dend = MemToShadow(dst + size);
+ RawShadow *dend = MemToEndShadow(dst + size);
ShadowSet(d, dend, Shadow::kEmpty);
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform.h b/compiler-rt/lib/tsan/rtl/tsan_platform.h
index 354f6da6a64a1..5b589df482256 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform.h
@@ -968,6 +968,24 @@ RawShadow *MemToShadow(uptr x) {
return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(x));
}
+struct MemToEndShadowImpl {
+ template <typename Mapping>
+ static uptr Apply(uptr x) {
+ return (((x + kShadowCell - 1) &
+ ~(Mapping::kShadowMsk | (kShadowCell - 1))) ^
+ Mapping::kShadowXor) *
+ kShadowMultiplier +
+ Mapping::kShadowAdd;
+ }
+};
+
+// If addr % kShadowCell == 0, then MemToEndShadow(addr) == MemToShadow(addr)
+// Otherwise, MemToEndShadow(addr) == MemToShadow(addr) + kShadowCnt
+ALWAYS_INLINE
+RawShadow *MemToEndShadow(uptr x) {
+ return reinterpret_cast<RawShadow *>(SelectMapping<MemToEndShadowImpl>(x));
+}
+
struct MemToMetaImpl {
template <typename Mapping>
static u32 *Apply(uptr x) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index 2c55645a15479..dbf583b362359 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -195,7 +195,7 @@ static NOINLINE void MapRodata(char* buffer, uptr size) {
!segment.IsWritable() && IsAppMem(segment.start)) {
// Assume it's .rodata
char *shadow_start = (char *)MemToShadow(segment.start);
- char *shadow_end = (char *)MemToShadow(segment.end);
+ char *shadow_end = (char *)MemToEndShadow(segment.end);
for (char *p = shadow_start; p < shadow_end;
p += marker.size() * sizeof(RawShadow)) {
internal_mmap(
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index c83efec8eaca2..4afb84e89936a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -532,7 +532,7 @@ static void StopBackgroundThread() {
void DontNeedShadowFor(uptr addr, uptr size) {
ReleaseMemoryPagesToOS(reinterpret_cast<uptr>(MemToShadow(addr)),
- reinterpret_cast<uptr>(MemToShadow(addr + size)));
+ reinterpret_cast<uptr>(MemToEndShadow(addr + size)));
}
#if !SANITIZER_GO
@@ -588,12 +588,12 @@ void MapShadow(uptr addr, uptr size) {
// CHECK_EQ(addr, addr & ~((64 << 10) - 1)); // windows wants 64K alignment
const uptr kPageSize = GetPageSizeCached();
uptr shadow_begin = RoundDownTo((uptr)MemToShadow(addr), kPageSize);
- uptr shadow_end = RoundUpTo((uptr)MemToShadow(addr + size), kPageSize);
+ uptr shadow_end = RoundUpTo((uptr)MemToEndShadow(addr + size), kPageSize);
if (!MmapFixedNoReserve(shadow_begin, shadow_end - shadow_begin, "shadow"))
Die();
#else
uptr shadow_begin = RoundDownTo((uptr)MemToShadow(addr), (64 << 10));
- uptr shadow_end = RoundUpTo((uptr)MemToShadow(addr + size), (64 << 10));
+ uptr shadow_end = RoundUpTo((uptr)MemToEndShadow(addr + size), (64 << 10));
VPrintf(2, "MapShadow for (0x%zx-0x%zx), begin/end: (0x%zx-0x%zx)\n",
addr, addr + size, shadow_begin, shadow_end);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
index cf07686d968dc..9652cd0267a79 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
@@ -684,16 +684,15 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) {
DCHECK(IsShadowMem(shadow_mem));
}
- RawShadow* shadow_mem_end = reinterpret_cast<RawShadow*>(
- reinterpret_cast<uptr>(shadow_mem) + size * kShadowMultiplier - 1);
- if (!IsShadowMem(shadow_mem_end)) {
- Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end,
+ RawShadow* shadow_mem_end = MemToEndShadow(addr + size);
+ if (size > 0 && !IsShadowMem(shadow_mem_end - 1)) {
+ Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end - 1,
(void*)(addr + size - 1));
Printf(
"Shadow start addr (ok): %p (%p); size: 0x%zx; kShadowMultiplier: "
"%zx\n",
shadow_mem, (void*)addr, size, kShadowMultiplier);
- DCHECK(IsShadowMem(shadow_mem_end));
+ DCHECK(IsShadowMem(shadow_mem_end - 1));
}
#endif
More information about the llvm-commits
mailing list