<div dir="ltr"><div class="gmail_default" style>LGTM. Thanks!</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jan 18, 2013 at 2:28 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Clang side looks great to me, thanks!<br>
<br>
Alexey: Does this look OK from your end?<br>
<div class="HOEnZb"><div class="h5"><br>
On Thu, Jan 17, 2013 at 8:53 AM, Will Dietz <<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>> wrote:<br>
> Great, thank you both for your feedback.  I also agree the approach<br>
> originally submitted was messy, and Richard you nailed the conflict<br>
> that drove me to accept it.  Thank you for giving me a better<br>
> alternative :).<br>
><br>
> Updated patches attached.<br>
><br>
> Notes:<br>
> * It was unclear to what IR clang should generate for a blacklisted<br>
> function/module when using address sanitizer.  I've added a test<br>
> verifying a blacklisted function does /not/ get given the<br>
> address-safety attribute, which is a deviation from existing<br>
> functionality.  Looking at the Asan LLVM transform suggests this is<br>
> fine (and seems like the right thing to do from clang's perspective<br>
> regardless), but would appreciate confirmation this is correct.<br>
> * I extended the AST serialization code to write out the sanitization<br>
> flags, only because that's what the code does presently.  Note that<br>
> all users of ASTReader seem to ignore benign language options (such as<br>
> sanitization flags).  One result is that AFAICT testing this would<br>
> require creating additional code to dump LangOpts or similar.  Is this<br>
> worth pursuing?<br>
> * At the time of CodeGenFunction construction we presently do not give<br>
> the ctor sufficient information to query the blacklist (function<br>
> name), so a conditionally bound reference didn't work.  Using a<br>
> pointer seemed preferable to the refactoring required to alleviate<br>
> this issue.<br>
><br>
> Thanks!<br>
><br>
> ~Will<br>
><br>
> On Thu, Jan 17, 2013 at 2:24 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>> wrote:<br>
>> I think that making blacklist available in Clang is a good idea.<br>
>> I agree with Richard's comment as well.<br>
>><br>
>><br>
>> On Thu, Jan 17, 2013 at 12:50 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> wrote:<br>
>>><br>
>>> On the clang side, it seems messy and error-prone to distribute the<br>
>>> checking for blacklisting across all the callers of EmitCheck, but I<br>
>>> understand that you're trying to avoid generating the check condition<br>
>>> for blacklisted functions. Maybe we could make a separate struct<br>
>>> containing just the Sanitize* flags, and in the CodeGenFunction<br>
>>> constructor bind a reference to either the global set of flags or a<br>
>>> set of all-false flags, depending on whether we're blacklisted? This<br>
>>> would also make it easier to blacklist specific sanitizers in specific<br>
>>> functions later.<br>
>>><br>
>>> On Wed, Jan 16, 2013 at 12:08 PM, Will Dietz <<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>> wrote:<br>
>>> > Minor touchup to clang patch, thanks!<br>
>>> ><br>
>>> > ~Will<br>
>>> ><br>
>>> > On Wed, Jan 16, 2013 at 11:37 AM, Will Dietz <<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>><br>
>>> > wrote:<br>
>>> >> Attached are two patches.<br>
>>> >><br>
>>> >> First applies to clang and uses the blacklist to avoid instrumenting<br>
>>> >> the source files or functions specified.  Lit test included.<br>
>>> >><br>
>>> >> The other is a small change to llvm to make the "Blacklist" class<br>
>>> >> visible to Clang.  Not sure I understand the header organization<br>
>>> >> system well enough, let me know if it belongs elsewhere.<br>
>>> >><br>
>>> >> Thanks!<br>
>>> >><br>
>>> >> ~Will<br>
>>> ><br>
>>> > _______________________________________________<br>
>>> > cfe-commits mailing list<br>
>>> > <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>>> ><br>
>>> _______________________________________________<br>
>>> cfe-commits mailing list<br>
>>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>><br>
>><br>
>><br>
>><br>
>> --<br>
>> Alexey Samsonov, MSK<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div>