<div dir="ltr">(Super delayed reply from after you sent the patches)<div><br></div><div>I think in general runtime flags are preferred to build configurations and macros. At this point, you've probably spent more time than most people measuring Windows ASan performance, and any data you have is certainly the freshest. If you think the extra branches and heap ownership checks are acceptable and it doesn't affect the main `free` codepath on Linux where performance is more closely monitored, then it should be acceptable.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 23, 2019 at 11:52 AM Matthew McGovern <<a href="mailto:Matthew.Mcgovern@microsoft.com">Matthew.Mcgovern@microsoft.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">





<div lang="EN-US">
<div class="gmail-m_859371452532363752WordSection1">
<p class="MsoNormal">Windows sanitizer community,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I have a patch to add interception for the Rtl*Heap family of allocation functions. The change  requires some additions to the existing regular Heap* interceptors since we are now also dealing with allocations that were created very early
 on during initialization.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">The current source code mentions why this isn’t ideal:<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><i>// We don't currently intercept all calls to HeapAlloc. If we did, we would have to check on<u></u><u></u></i></p>
<p class="MsoNormal"><i>// HeapFree whether the pointer came from ASan or from the system.<u></u><u></u></i></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">SO… We implemented it and it does get sort of gross, especially in the ReAllocate case. There’s also the issue of RtlSizeHeap and RtlReAllocateHeap being undocumented (for now). Some  Rtl*Heap functions are documented in the Windows Driver
 development kit and it’s possible that we can have ReAlloc and Size added.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Because of all the previously mentioned issues I would like to add this interception code behind a preprocessor macro. I’m guessing everyone will not neccesarily need it and the code will introduce a performance penalty when enabled.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Does the community have any input before I push this patch out for review? Do you think there is a better approach for hiding this code, do you think it should be a runtime option instead? Is there anyone opposed to having this in the compiler-rt
 windows source for some reason I haven’t considered?<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Thank you all for your time and input,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-family:"Calibri Light",sans-serif">Matthew G McGovern | Security Software Engineer<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Calibri Light",sans-serif">Microsoft COSINE | Platform Security and Vulnerabilty Research<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Calibri Light",sans-serif">One Microsoft Way | Redmond, WA 98052<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>

</blockquote></div>