Skip to content

Commit 9eac5f7

Browse files
authored
Revert "[TSan] Clarify and enforce shadow end alignment" (#146674)
Reverts #144648 due to a test failure of the new added test case `munmap_clear_shadow.c` in IOS .
1 parent 6ec9b1b commit 9eac5f7

File tree

6 files changed

+14
-42
lines changed

6 files changed

+14
-42
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ 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);
125126

126127
// Assuming it's not running concurrently with threads that do
127128
// memory accesses and mutex operations (stop-the-world phase).

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -566,32 +566,17 @@ static bool IsValidMmapRange(uptr addr, uptr size) {
566566
return false;
567567
}
568568

569-
void UnmapShadow(ThreadState* thr, uptr addr, uptr size) {
569+
void UnmapShadow(ThreadState *thr, uptr addr, uptr size) {
570570
if (size == 0 || !IsValidMmapRange(addr, size))
571571
return;
572-
// unmap shadow is related to semantic of mmap/munmap, so we
573-
// should clear the whole shadow range, including the tail shadow
574-
// while addr + size % kShadowCell != 0.
575-
uptr rounded_size_shadow = RoundUp(addr + size, kShadowCell) - addr;
576-
DontNeedShadowFor(addr, rounded_size_shadow);
572+
DontNeedShadowFor(addr, size);
577573
ScopedGlobalProcessor sgp;
578574
SlotLocker locker(thr, true);
579-
uptr rounded_size_meta = RoundUp(addr + size, kMetaShadowCell) - addr;
580-
ctx->metamap.ResetRange(thr->proc(), addr, rounded_size_meta, true);
575+
ctx->metamap.ResetRange(thr->proc(), addr, size, true);
581576
}
582577
#endif
583578

584579
void MapShadow(uptr addr, uptr size) {
585-
// Although named MapShadow, this function's semantic is unrelated to
586-
// UnmapShadow. This function currently only used for Go's lazy allocation
587-
// of shadow, whose targets are program section (e.g., bss, data, etc.).
588-
// Therefore, we can guarantee that the addr and size align to kShadowCell
589-
// and kMetaShadowCell by the following assertions.
590-
DCHECK_EQ(addr % kShadowCell, 0);
591-
DCHECK_EQ(size % kShadowCell, 0);
592-
DCHECK_EQ(addr % kMetaShadowCell, 0);
593-
DCHECK_EQ(size % kMetaShadowCell, 0);
594-
595580
// Ensure thead registry lock held, so as to synchronize
596581
// with DoReset, which also access the mapped_shadow_* ctxt fields.
597582
ThreadRegistryLock lock0(&ctx->thread_registry);

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

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

691-
uptr rounded_size =
692-
(RoundUpTo(addr + size, kShadowCell) - RoundDownTo(addr, kShadowCell));
693-
RawShadow* shadow_mem_end =
694-
shadow_mem + rounded_size / kShadowCell * kShadowCnt;
695-
if (!IsShadowMem(shadow_mem_end - 1)) {
696-
Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end - 1,
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,
697695
(void*)(addr + size - 1));
698696
Printf(
699-
"Shadow start addr (ok): %p (%p); size: 0x%zx; rounded_size: 0x%zx; "
700-
"kShadowMultiplier: %zx\n",
701-
shadow_mem, (void*)addr, size, rounded_size, kShadowMultiplier);
702-
DCHECK(IsShadowMem(shadow_mem_end - 1));
697+
"Shadow start addr (ok): %p (%p); size: 0x%zx; kShadowMultiplier: "
698+
"%zx\n",
699+
shadow_mem, (void*)addr, size, kShadowMultiplier);
700+
DCHECK(IsShadowMem(shadow_mem_end));
703701
}
704702
#endif
705703

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -246,20 +246,6 @@ void MetaMap::MoveMemory(uptr src, uptr dst, uptr sz) {
246246
// there are no concurrent accesses to the regions (e.g. stop-the-world).
247247
CHECK_NE(src, dst);
248248
CHECK_NE(sz, 0);
249-
250-
// The current MoveMemory implementation behaves incorrectly when src, dst,
251-
// and sz are not aligned to kMetaShadowCell.
252-
// For example, with kMetaShadowCell == 8:
253-
// - src = 4: unexpectedly clears the metadata for the range [0, 4).
254-
// - src = 16, dst = 4, size = 8: A sync variable for addr = 20, which should
255-
// be moved to the metadata for address 8, is incorrectly moved to the
256-
// metadata for address 0 instead.
257-
// - src = 0, sz = 4: fails to move the tail metadata.
258-
// Therefore, the following assertions is needed.
259-
DCHECK_EQ(src % kMetaShadowCell, 0);
260-
DCHECK_EQ(dst % kMetaShadowCell, 0);
261-
DCHECK_EQ(sz % kMetaShadowCell, 0);
262-
263249
uptr diff = dst - src;
264250
u32 *src_meta, *dst_meta, *src_meta_end;
265251
uptr inc;

compiler-rt/test/tsan/java_heap_init2.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
2+
// XFAIL: *
23

34
#include "java.h"
45
#include <errno.h>

compiler-rt/test/tsan/munmap_clear_shadow.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_tsan %s -o %t && %run %t | FileCheck %s
2+
// XFAIL: *
23

34
#include "test.h"
45
#include <assert.h>

0 commit comments

Comments
 (0)