[PATCH] D66616: [TSan] Add interceptors for mach_vm_[de]allocate

[Julian Lettner via Phabricator via llvm-commits]

yln marked 2 inline comments as done.
yln added inline comments.


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interceptors.cpp:748-749
 
+// Zero out addr if it points into shadow memory and was provided as a hint
+// only, i.e., MAP_FIXED is not set.
 static bool fix_mmap_addr(void **addr, long_t sz, int flags) {
----------------
kubamracek wrote:
> is this related? or just a co-incidental change?
Related. It took me a while to understand what it is doing, so I left a comment.
```
void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset);

1) addr=0, flags=... -> kernel picks address, best most portable option
2) addr=0x123, flags=... -> kernel picks address, 'addr' is treated as a hint, no guarantees
3) addr=0x123, flag=MAP_FIXED -> mmap fails if allocation cannot be placed at 'addr'
```
This function ensures that we never call the real mmap with an `addr` that points into the shadow. For 2) it discards the hint, for 3) we return with failure without forwarding to the real mmap.

`vm_mach_allocate` only has cases 1) and 3). I named the function `intersects_with_shadow`


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interceptors_mach_vm.cpp:42-45
+    if (thr->ignore_reads_and_writes == 0)
+      MemoryRangeImitateWrite(thr, pc, *address, size);
+    else
+      MemoryResetRange(thr, pc, *address, size);
----------------
kubamracek wrote:
> This is copied from mmap, right? Can it be extracted into a common function instead?
I tried extracting a function and then parameterizing it for most of the interceptor, but that turned messy really quickly. I can certainly do it for the above 4 lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66616/new/

https://reviews.llvm.org/D66616