[compiler-rt] [TSan] Make Shadow/Meta region inclusive-exclusive (PR #144647)
Kunqiu Chen via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 20 00:34:25 PDT 2025
Camsyn wrote:
This change introduces some test fails of `mmap_lots.cpp`.
- https://lab.llvm.org/buildbot/#/builders/51/builds/18222
- https://lab.llvm.org/buildbot/#/builders/72/builds/12303
After analysis, I believe it is due to the following reasons:
> Large `mmap` -> `MemoryRangeSet` -> only set the first and last page of shadow.
```cpp
static void MemoryRangeSet(uptr addr, uptr size, RawShadow val) {
if (size == 0)
return;
RawShadow* begin = MemToShadow(addr);
// end == ShadowMem::end
RawShadow* end = begin + size / kShadowCell * kShadowCnt;
...
RawShadow* mid2 = RoundDown(end, kPageSize);
// What if mid2 == end == ShadowMem::end?
ShadowSet(mid2, end, val);
void ShadowSet(RawShadow* p, RawShadow* end, RawShadow v) {
DCHECK_LE(p, end); // O: p == end
DCHECK(IsShadowMem(p)); // X : p == ShadowMem::end
DCHECK(p == end || IsShadowMem(end - 1));
```
I think this actually exposes some degree of flaw in the current implementation.
---
Possible fixes:
1. Do not allow `p == end` in ShadowSet
- This is reasonable because both callers of `ShadowSet` (i.e., `MemoryRangeSet` and `__tsan_java_move`) are actually guaranteed that size > 0.
- Although `MemoryRangeSet`'s dedicated handling for overly large `mmap` bypasses this limit, we can append a guard that checks for size > 0, i.e., `if (mid2 < end) ShadowSet(mid2, end, val);`
```cpp
// DCHECK_LE(p, end);
DCHECK_LT(p, end);
```
2. Modify the assertion in `ShadowSet` as follows:
```cpp
// DCHECK(IsShadowMem(p));
DCHECK(p == end || IsShadowMem(p));
```
3. `ShadowSet` : Just return while `p == end`.
```cpp
void ShadowSet(RawShadow* p, RawShadow* end, RawShadow v) {
if (p == end) return;
```
https://github.com/llvm/llvm-project/pull/144647
More information about the llvm-commits
mailing list