-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[tsan] Fix calculation of shadow end address in MemoryAccessRangeT #98404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
MemoryAccessRangeT overestimates the size of the shadow region by 8x, occasionally leading to assertion failure: ``` RawShadow* shadow_mem = MemToShadow(addr); ... // Check that end of shadow is valid if (!IsShadowMem(shadow_mem + size * kShadowCnt - 1)) { DCHECK(IsShadowMem(shadow_mem + size * kShadowCnt - 1)); ``` It is erroneous for two separate reasons: - it uses kShadowCnt (== 4) instead of kShadowMultiplier (== 2) - since shadow_mem is a RawShadow*, pointer arithmetic is multiplied by sizeof(RawShadow) == 4 This patch fixes the calculation, and also improves the debugging information. The assertion error was observed on a buildbot (https://lab.llvm.org/staging/#/builders/89/builds/656/steps/13/logs/stdio): ``` Bad shadow addr 0x3000000190bc (7fffffffe85f) ThreadSanitizer: CHECK failed: tsan_rtl_access.cpp:690 "((IsShadowMem(shadow_mem + size * kShadowCnt - 1))) != (0)" (0x0, 0x0) (tid=2202676) ``` Notice that 0x3000000190bc is not the correct shadow for the end address 0x7fffffffe85f. This error is more commonly observed on high-entropy ASLR systems, since ASLR may be disabled (if the randomized memory layout is incompatible), leading to an allocation near the boundaries of the high app memory region (and therefore a shadow end that may be erroneously calculated to be past the end of the shadow region). Also note that the assertion is guarded by SANITIZER_DEBUG.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) ChangesMemoryAccessRangeT overestimates the size of the shadow region by 8x, occasionally leading to assertion failure:
It is erroneous for two separate reasons:
This patch fixes the calculation, and also improves the debugging information. The assertion error was observed on a buildbot (https://lab.llvm.org/staging/#/builders/89/builds/656/steps/13/logs/stdio):
Notice that 0x3000000190bc is not the correct shadow for the end address 0x7fffffffe85f. This error is more commonly observed on high-entropy ASLR systems, since ASLR may be disabled (if the randomized memory layout is incompatible), leading to an allocation near the boundaries of the high app memory region (and therefore a shadow end that may be erroneously calculated to be past the end of the shadow region). Also note that the assertion is guarded by SANITIZER_DEBUG. Full diff: https://github.com/llvm/llvm-project/pull/98404.diff 1 Files Affected:
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
index 8b20984a01000..d03b1863735df 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
@@ -672,22 +672,30 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) {
#if SANITIZER_DEBUG
if (!IsAppMem(addr)) {
- Printf("Access to non app mem %zx\n", addr);
+ Printf("Access to non app mem start: 0x%zx\n", addr);
DCHECK(IsAppMem(addr));
}
if (!IsAppMem(addr + size - 1)) {
- Printf("Access to non app mem %zx\n", addr + size - 1);
+ Printf("Access to non app mem end: 0x%zx\n", addr + size - 1);
DCHECK(IsAppMem(addr + size - 1));
}
if (!IsShadowMem(shadow_mem)) {
- Printf("Bad shadow addr %p (%zx)\n", static_cast<void*>(shadow_mem), addr);
+ Printf("Bad shadow start addr: %p (0x%zx)\n",
+ static_cast<void*>(shadow_mem), addr);
DCHECK(IsShadowMem(shadow_mem));
}
- if (!IsShadowMem(shadow_mem + size * kShadowCnt - 1)) {
- Printf("Bad shadow addr %p (%zx)\n",
- static_cast<void*>(shadow_mem + size * kShadowCnt - 1),
- addr + size - 1);
- DCHECK(IsShadowMem(shadow_mem + size * kShadowCnt - 1));
+
+ // Be careful to cast to uptr before performing arithmetic.
+ RawShadow* shadow_mem_end = static_cast<RawShadow*>(
+ (void*)(((uptr)shadow_mem) + size * kShadowMultiplier - 1));
+ if (!IsShadowMem(shadow_mem_end)) {
+ Printf("Bad shadow end addr: %p (0x%zx)\n",
+ static_cast<void*>(shadow_mem_end), addr + size - 1);
+ Printf(
+ "Shadow start addr (ok): %p (0x%zx); size: 0x%zx; kShadowMultiplier: "
+ "%zx\n",
+ static_cast<void*>(shadow_mem), addr, size, kShadowMultiplier);
+ DCHECK(IsShadowMem(shadow_mem_end));
}
#endif
|
…lvm#98404) MemoryAccessRangeT overestimates the size of the shadow region by 8x, occasionally leading to assertion failure: ``` RawShadow* shadow_mem = MemToShadow(addr); ... // Check that end of shadow is valid if (!IsShadowMem(shadow_mem + size * kShadowCnt - 1)) { DCHECK(IsShadowMem(shadow_mem + size * kShadowCnt - 1)); ``` It is erroneous for two separate reasons: - it uses kShadowCnt (== 4) instead of kShadowMultiplier (== 2) - since shadow_mem is a RawShadow*, pointer arithmetic is multiplied by sizeof(RawShadow) == 4 This patch fixes the calculation, and also improves the debugging information. The assertion error was observed on a buildbot (https://lab.llvm.org/staging/#/builders/89/builds/656/steps/13/logs/stdio): ``` Bad shadow addr 0x3000000190bc (7fffffffe85f) ThreadSanitizer: CHECK failed: tsan_rtl_access.cpp:690 "((IsShadowMem(shadow_mem + size * kShadowCnt - 1))) != (0)" (0x0, 0x0) (tid=2202676) ``` Notice that 0x3000000190bc is not the correct shadow for the end address 0x7fffffffe85f. This error is more commonly observed on high-entropy ASLR systems, since ASLR may be disabled (if the randomized memory layout is incompatible), leading to an allocation near the boundaries of the high app memory region (and therefore a shadow end that may be erroneously calculated to be past the end of the shadow region). Also note that the assertion is guarded by SANITIZER_DEBUG. --------- Co-authored-by: Vitaly Buka <[email protected]>
MemoryAccessRangeT overestimates the size of the shadow region by 8x, occasionally leading to assertion failure:
It is erroneous for two separate reasons:
This patch fixes the calculation, and also improves the debugging information.
The assertion error was observed on a buildbot (https://lab.llvm.org/staging/#/builders/89/builds/656/steps/13/logs/stdio):
Notice that 0x3000000190bc is not the correct shadow for the end address 0x7fffffffe85f.
This error is more commonly observed on high-entropy ASLR systems, since ASLR may be disabled (if the randomized memory layout is incompatible), leading to an allocation near the boundaries of the high app memory region (and therefore a shadow end that may be erroneously calculated to be past the end of the shadow region). Also note that the assertion is guarded by SANITIZER_DEBUG.