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

[Will Dietz]

Sorry about that, attached is a patch that should fix the tests.

Can you confirm that this fixes the failures you're seeing?

Thanks!

~Will



On Fri, Jan 18, 2013 at 9:12 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> It behaves unexpectedly to pass "src:x:\path\to\file.cpp" in the
> blacklist, due to try to match whole DOSish path without escaping with
> Regex.
>
> Two tests have been suppressed on win32 since r172820, FYI.
>
> ...Takumi
>
> 2013/1/18 Will Dietz <wdietz2 at illinois.edu>:
>> 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
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-wildcards-to-match-paths-in-tests-using-fsanitiz.patch
Type: application/octet-stream
Size: 2195 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130119/4cb027fa/attachment.obj>