[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