Skip to content

Commit cfe2a93

Browse files
committed
[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.
1 parent 956bab0 commit cfe2a93

File tree

5 files changed

+27
-11
lines changed

5 files changed

+27
-11
lines changed

compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) {
122122
DCHECK_GE(dst, jctx->heap_begin);
123123
DCHECK_LE(dst + size, jctx->heap_begin + jctx->heap_size);
124124
DCHECK_NE(dst, src);
125-
DCHECK_NE(size, 0);
126125

127126
// Assuming it's not running concurrently with threads that do
128127
// memory accesses and mutex operations (stop-the-world phase).
@@ -132,7 +131,7 @@ void __tsan_java_move(jptr src, jptr dst, jptr size) {
132131
// We used to move shadow from src to dst, but the trace format does not
133132
// support that anymore as it contains addresses of accesses.
134133
RawShadow *d = MemToShadow(dst);
135-
RawShadow *dend = MemToShadow(dst + size);
134+
RawShadow *dend = MemToEndShadow(dst + size);
136135
ShadowSet(d, dend, Shadow::kEmpty);
137136
}
138137

compiler-rt/lib/tsan/rtl/tsan_platform.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,24 @@ RawShadow *MemToShadow(uptr x) {
968968
return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(x));
969969
}
970970

971+
struct MemToEndShadowImpl {
972+
template <typename Mapping>
973+
static uptr Apply(uptr x) {
974+
return (((x + kShadowCell - 1) &
975+
~(Mapping::kShadowMsk | (kShadowCell - 1))) ^
976+
Mapping::kShadowXor) *
977+
kShadowMultiplier +
978+
Mapping::kShadowAdd;
979+
}
980+
};
981+
982+
// If addr % kShadowCell == 0, then MemToEndShadow(addr) == MemToShadow(addr)
983+
// Otherwise, MemToEndShadow(addr) == MemToShadow(addr) + kShadowCnt
984+
ALWAYS_INLINE
985+
RawShadow *MemToEndShadow(uptr x) {
986+
return reinterpret_cast<RawShadow *>(SelectMapping<MemToEndShadowImpl>(x));
987+
}
988+
971989
struct MemToMetaImpl {
972990
template <typename Mapping>
973991
static u32 *Apply(uptr x) {

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ static NOINLINE void MapRodata(char* buffer, uptr size) {
195195
!segment.IsWritable() && IsAppMem(segment.start)) {
196196
// Assume it's .rodata
197197
char *shadow_start = (char *)MemToShadow(segment.start);
198-
char *shadow_end = (char *)MemToShadow(segment.end);
198+
char *shadow_end = (char *)MemToEndShadow(segment.end);
199199
for (char *p = shadow_start; p < shadow_end;
200200
p += marker.size() * sizeof(RawShadow)) {
201201
internal_mmap(

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ static void StopBackgroundThread() {
532532

533533
void DontNeedShadowFor(uptr addr, uptr size) {
534534
ReleaseMemoryPagesToOS(reinterpret_cast<uptr>(MemToShadow(addr)),
535-
reinterpret_cast<uptr>(MemToShadow(addr + size)));
535+
reinterpret_cast<uptr>(MemToEndShadow(addr + size)));
536536
}
537537

538538
#if !SANITIZER_GO
@@ -588,12 +588,12 @@ void MapShadow(uptr addr, uptr size) {
588588
// CHECK_EQ(addr, addr & ~((64 << 10) - 1)); // windows wants 64K alignment
589589
const uptr kPageSize = GetPageSizeCached();
590590
uptr shadow_begin = RoundDownTo((uptr)MemToShadow(addr), kPageSize);
591-
uptr shadow_end = RoundUpTo((uptr)MemToShadow(addr + size), kPageSize);
591+
uptr shadow_end = RoundUpTo((uptr)MemToEndShadow(addr + size), kPageSize);
592592
if (!MmapFixedNoReserve(shadow_begin, shadow_end - shadow_begin, "shadow"))
593593
Die();
594594
#else
595595
uptr shadow_begin = RoundDownTo((uptr)MemToShadow(addr), (64 << 10));
596-
uptr shadow_end = RoundUpTo((uptr)MemToShadow(addr + size), (64 << 10));
596+
uptr shadow_end = RoundUpTo((uptr)MemToEndShadow(addr + size), (64 << 10));
597597
VPrintf(2, "MapShadow for (0x%zx-0x%zx), begin/end: (0x%zx-0x%zx)\n",
598598
addr, addr + size, shadow_begin, shadow_end);
599599

compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -688,16 +688,15 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) {
688688
DCHECK(IsShadowMem(shadow_mem));
689689
}
690690

691-
RawShadow* shadow_mem_end = reinterpret_cast<RawShadow*>(
692-
reinterpret_cast<uptr>(shadow_mem) + size * kShadowMultiplier - 1);
693-
if (!IsShadowMem(shadow_mem_end)) {
694-
Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end,
691+
RawShadow* shadow_mem_end = MemToEndShadow(addr + size);
692+
if (size > 0 && !IsShadowMem(shadow_mem_end - 1)) {
693+
Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end - 1,
695694
(void*)(addr + size - 1));
696695
Printf(
697696
"Shadow start addr (ok): %p (%p); size: 0x%zx; kShadowMultiplier: "
698697
"%zx\n",
699698
shadow_mem, (void*)addr, size, kShadowMultiplier);
700-
DCHECK(IsShadowMem(shadow_mem_end));
699+
DCHECK(IsShadowMem(shadow_mem_end - 1));
701700
}
702701
#endif
703702

0 commit comments

Comments
 (0)