[all-commits] [llvm/llvm-project] bc9016: [TSan] Clarify and enforce shadow end alignment (#...

[Kunqiu Chen via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: bc90166a50c4197b7666889c128e70c8f8fad2f3
      https://github.com/llvm/llvm-project/commit/bc90166a50c4197b7666889c128e70c8f8fad2f3
  Author: Kunqiu Chen <camsyn at foxmail.com>
  Date:   2025-06-27 (Fri, 27 Jun 2025)

  Changed paths:
    M compiler-rt/lib/tsan/rtl/tsan_interface_java.cpp
    M compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
    M compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
    M compiler-rt/lib/tsan/rtl/tsan_sync.cpp
    M compiler-rt/test/tsan/java_heap_init2.cpp
    M compiler-rt/test/tsan/munmap_clear_shadow.c

  Log Message:
  -----------
  [TSan] Clarify and enforce shadow end alignment (#144648)

In TSan, every `k` bytes of application memory (where `k = 8`) maps to a
single shadow/meta cell. This design leads to two distinct outcomes when
calculating the end of a shadow range using `MemToShadow(addr_end)`,
depending on the alignment of `addr_end`:

- **Exclusive End:** If `addr_end` is aligned (`addr_end % k == 0`),
`MemToShadow(addr_end)` points to the first shadow cell *past* the
intended range. This address is an exclusive boundary marker, not a cell
to be operated on.
- **Inclusive End:** If `addr_end` is not aligned (`addr_end % k != 0`),
`MemToShadow(addr_end)` points to the last shadow cell that *is* part of
the range (i.e., the same cell as `MemToShadow(addr_end - 1)`).

Different TSan functions have different expectations for whether the
shadow end should be inclusive or exclusive. However, these expectations
are not always explicitly enforced, which can lead to subtle bugs or
reliance on unstated invariants.


The core of this patch is to ensure that functions ONLY requiring an
**exclusive shadow end** behave correctly.

1.  Enforcing Existing Invariants:
For functions like `MetaMap::MoveMemory` and `MapShadow`, the assumption
is that the end address is always `k`-aligned. While this holds true in
the current codebase (e.g., due to some external implicit conditions),
this invariant is not guaranteed by the function's internal context. We
add explicit assertions to make this requirement clear and to catch any
future changes that might violate this assumption.

2.  Fixing Latent Bugs:
In other cases, unaligned end addresses are possible, representing a
latent bug. This was the case in `UnmapShadow`. The `size` of a memory
region being unmapped is not always a multiple of `k`. When this
happens, `UnmapShadow` would fail to clear the final (tail) portion of
the shadow memory.

This patch fixes `UnmapShadow` by rounding up the `size` to the next
multiple of `k` before clearing the shadow memory. This is safe because
the underlying OS `unmap` operation is page-granular, and the page size
is guaranteed to be a multiple of `k`.

Notably, this fix makes `UnmapShadow` consistent with its inverse
operation, `MemoryRangeImitateWriteOrResetRange`, which already performs
a similar size round-up.

In summary, this PR:

- **Adds assertions** to `MetaMap::MoveMemory` and `MapShadow` to
enforce their implicit requirement for k-aligned end addresses.
- **Fixes a latent bug** in `UnmapShadow` by rounding up the size to
ensure the entire shadow range is cleared. Two new test cases have been
added to cover this scenario.
  - Removes a redundant assertion in `__tsan_java_move`.
- Fixes an incorrect shadow end calculation introduced in commit
4052de6. The previous logic, while fixing an overestimation issue, did
not properly account for `kShadowCell` alignment and could lead to
underestimation.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications