[PATCH] D62927: [sanitizers][windows] Rtl-Heap Interception and tests

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_malloc_win.cc:53

----------------
can you please upload the patch with "arc" tool, otherwise context is not visible


================
Comment at: compiler-rt/lib/asan/asan_malloc_win.cc:209
+  if (flags()->windows_hook_rtl_allocators)
+  {
+    if (UNLIKELY(!asan_inited || (!__sanitizer_get_ownership(lpMem) &&
----------------
Not sure I understand this "if".
Can we get here when windows_hook_rtl_allocators==false?
If true, I don't see much difference after asan_inited set to true.



================
Comment at: compiler-rt/lib/asan/asan_malloc_win.cc:211
+    if (UNLIKELY(!asan_inited || (!__sanitizer_get_ownership(lpMem) &&
+                                  HeapValidate(hHeap, 0, lpMem)))) {
+      return REAL(HeapSize)(hHeap, dwFlags, lpMem);
----------------
can you please clang-format the patch?


================
Comment at: compiler-rt/lib/asan/asan_malloc_win.cc:218
+    CHECK(dwFlags == 0 && "unsupported heap flags");
+  }
+  GET_CURRENT_PC_BP_SP;
----------------
I'd expect this check is needed unconditionally, we can get to asan_malloc_usable_size even "if-then" about was executed.


================
Comment at: compiler-rt/lib/asan/asan_malloc_win.cc:462
+  void *p;
+  // Reading MSDN suggests that the *entire* usable allocation is zeroed out.
+  // Otherwise it is difficult to HeapReAlloc with HEAP_ZERO_MEMORY.
----------------
Isn't better to use REAL in cases above instead of CHECK(Flags...) ?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D62927