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

Matthew G McGovern via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 20 13:20:40 PDT 2019


mcgov marked 2 inline comments as done and an inline comment as not done.
mcgov added a comment.

addressing comments from @vitalybuka



================
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) &&
----------------
mcgov wrote:
> vitalybuka wrote:
> > 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.
> > 
> These if's are handling some edge cases when you hook the RTL allocators. There are some allocations which will occur before interception happens, so you have to check where the allocation came from.
> 
> I agree these look pretty gross and aren't super clear. I'll see if I can clean it up or write some comments to make them obvious.
I've cleaned these up, they look a little weird at first but the new code makes sure to split up unsupported flags at allocation time when the runtime flag is set. In that case it is enough to check who owns the allocation, since the user can allocate a memory section without the unsupported flags but then use them on a later call to HeapSize or HeapReAlloc. When the RTL interception is turned on it's enough to just check who owns it, since if the user wants to use HEAP_NO_SERIALIZE on Free or Size it will just get ignored in the event ASAN owns the allocation. If the allocation is passed into the RTL library it will pass the flag through.

If the flag is turned off we keep the original behavior of asserting on those illegal flags, since there is no falling back to the original allocator in that version.

That's a lot, does that make sense or am I talking myself into garbage lol


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D62927





More information about the llvm-commits mailing list