[PATCH] D25973: [tsan] Implement WriteMemoryProfile for Darwin

[Dmitry Vyukov via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:108
 
-void FlushShadowMemory() {
+static void RegionMemUsage(uptr start, uptr end, uptr *res, uptr *dirty) {
+  vm_address_t address = start;
----------------
zaks.anna wrote:
> This is not TSan-specific. Should it go into sanitizer-common?
I would leave it here, unless you have a plan to implement memory profiler for asan on mac.
sanitizer_common contains too much stuff already.
E.g. linux GetMemoryProfile was moved from tsan to sanitizer_common, but it is still used only in tsan. While moving empty stubs were added for other OSes, but the interface is actually such that it cannot be possibly ported to other OSes.
Generalizing only one use case does not usually lead to good results.


================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:143
+  RegionMemUsage(LoAppMemBeg(), LoAppMemEnd(), &low_res, &low_dirty);
+  RegionMemUsage(HeapMemBeg(), HeapMemEnd(), &heap_res, &heap_dirty);
+  RegionMemUsage(HiAppMemBeg(), HiAppMemEnd(), &high_res, &high_dirty);
----------------
I suspect this won't compile when SANITIZER_GO is defined. Go does not have HeapMemBeg.
You can surround it all with #if !SANITIZER_GO as WriteMemoryProfile is not called in Go.
You can test it by running lib/tsan/go/buildgo.sh


================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:158
+    ShadowBeg(), ShadowEnd(), shadow_res / 1024, shadow_dirty / 1024,
+    MetaShadowBeg(), MetaShadowEnd(), shadow_res / 1024, shadow_dirty / 1024,
+    TraceMemBeg(), TraceMemEnd(), trace_res / 1024, trace_dirty / 1024,
----------------
s/shadow_res/meta_res/
s/shadow_dirty/meta_dirty/


Repository:
  rL LLVM

https://reviews.llvm.org/D25973