[cfe-commits] [PATCH] [ubsan] Add support for -fsanitize-blacklist

[Will Dietz]

Committed as r172806 (llvm) and r172808 (clang).  Thanks!

~Will

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