[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