sanitizer-coverage vs asan_blacklist

[Timur Iskhodzhanov]

вт, 28 апр. 2015 г. в 20:03, Alexey Samsonov <vonosmas at gmail.com>:

> On Sat, Apr 11, 2015 at 8:38 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
>
>>
>> On Sat, Apr 11, 2015 at 2:36 PM, Timur Iskhodzhanov <timurrrr at google.com>
>> wrote:
>>
>>> __attribute__((no_instrumentation)) ?  Sounds like a perfect fit for the
>>> job
>>>
>>
>> However, IIRC, Clang folks wanted positive attributes
>> ("sanitize_address", "sanitize_memory" etc.) on LLVM
>> functions, not sure "no_instrumentation" will be accepted. Otherwise we
>> will have to emit "sanitize_coverage"
>> attribute on all functions that need to be instrumented, and that would
>> be somewhat unfortunate.
>>
>
> So, essentially you need an attribute to disable coverage instrumentation
> of a particular function
> (as ASan instrumentation is already disabled), right?
>

Yes, or...
What I really want is a way to tell the compiler "hey, I don't want this
function to do any memory accesses other than those required by its code".
Do you think we should maybe have an attribute for exactly this purpose?

I was trying to make the point that you can add
> Clang's __attribute__((no_sanitize_coverage)), and that would be aligned
> with attributes we already have,
> but LLVM function attribute should be "positive": so you will have to
> attach "sanitize_coverage" attributes
> to all the LLVM functions you want to include in coverage.
>
> This can be tricky, as we might not see missing attributes, and don't
> notice that some functions
> (e.g. compiler-generated ones) are excluded from coverage.
>
> However, I'm OK if you decide to take this path - once again, it will be
> consistent with current
> state of affairs with ASan/TSan/MSan.
>
>
>
>>
>>> сб, 11 апр. 2015, 21:58, Alexey Samsonov <vonosmas at gmail.com>:
>>>
>>> On Sat, Apr 11, 2015 at 11:33 AM, Timur Iskhodzhanov <
>>>> timurrrr at google.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> сб, 11 апр. 2015 г. в 20:17, Alexey Samsonov <vonosmas at gmail.com>:
>>>>>
>>>>>> I think that using blacklist to disable instrumentation because of
>>>>>> sandbox is a pretty gross hack, and
>>>>>> the wrong thing to do. I think the better "solution" would be to move
>>>>>> sandbox-related code to a different
>>>>>> source files, and compile them with -fno-sanitize=all. Then you can
>>>>>> also compile them with -fsanitize-coverage=0,
>>>>>> and the problem would be solved.
>>>>>>
>>>>>
>>>>> I don't agree this is a better solution.
>>>>> Changing the file/code structure of a project to please a deficiency
>>>>> of every tool we want to use doesn't scale in general.
>>>>>
>>>>
>>>> Fair enough, but I'd argue we should have added
>>>> __attribute__((no_sanitize_address)) to these functions instead of
>>>> blacklisting them.
>>>> I'm OK with implementing similar attribute for coverage.
>>>>
>>>>>
>>>>> For example, Chromium's sandbox/ code depends on base/ and we do
>>>>> blacklist some code from base/.  Some of that code is located in a header
>>>>> file...
>>>>> I doubt sufficiently changing the file structure would be welcome.
>>>>> I'm not a GYP expert, so the thought of passing an extra compiler flag
>>>>> for select TUs scares me as well.
>>>>>
>>>>>
>>>>>> On Fri, Apr 10, 2015 at 12:16 PM, Timur Iskhodzhanov <
>>>>>> timurrrr at google.com> wrote:
>>>>>>
>>>>>>> (After looking at the code)
>>>>>>> Is it fine to just return
>>>>>>> from SanitizerCoverageModule::runOnFunction if a given function doesn't
>>>>>>> have one of the Sanitize{Address,Thread,Memory} attributes for the
>>>>>>> "current" sanitizer set?
>>>>>>>
>>>>>>
>>>>>> No. Coverage is also supposed to work with DFSan, and with UBSan (the
>>>>>> latter doesn't add any attributes to LLVM functions).
>>>>>>
>>>>>
>>>>> Alright, can we support that exclusion just for ASan for the time
>>>>> being?
>>>>>
>>>>>
>>>>>> Logic could be like that:
>>>>>>> – instrument all the code if we're only doing coverage [w/o
>>>>>>> asan,msan,tsan]
>>>>>>> – instrument all the non-blacklisted code, use the same
>>>>>>> instrument/no-instrument attribute as the "main" sanitizer being used
>>>>>>>
>>>>>>> WDYT?
>>>>>>>
>>>>>>> пт, 10 апр. 2015 г. в 21:32, Timur Iskhodzhanov <timurrrr at google.com
>>>>>>> >:
>>>>>>>
>>>>>>> ... especially since the blacklist option is
>>>>>>>> called -fsanitize-blacklist.
>>>>>>>> Just looking at its name, I'd had assumed it affected
>>>>>>>> -fsanitize-coverage too.
>>>>>>>>
>>>>>>>
>>>>>> I'd try to abstain from using blacklist for coverage instrumentation.
>>>>>> If we would still definitely have to add it for
>>>>>> some reason (see above for suggestion regarding sandbox issue in
>>>>>> Chrome), we can use categories:
>>>>>>
>>>>>> src:*/dont/use/coverage.cc=coverage
>>>>>> fun:DontUseCoverage=coverage
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> пт, 10 апр. 2015 г. в 21:08, Timur Iskhodzhanov <
>>>>>>>> timurrrr at google.com>:
>>>>>>>>
>>>>>>>> Kostya, Alexey,
>>>>>>>>>
>>>>>>>>> I'm currently working on sanitizer coverage on Windows for
>>>>>>>>> Chromium.
>>>>>>>>> As described in http://crbug.com/459166 , we currently blacklist
>>>>>>>>> some sandbox code from ASan instrumentation, otherwise shadow memory is
>>>>>>>>> accessed before the RTL is even loaded.
>>>>>>>>> As it turns out, we should also disable coverage instrumentation
>>>>>>>>> for that code, otherwise the guard variables are checked before the RTL is
>>>>>>>>> loaded.
>>>>>>>>>
>>>>>>>>> That being said, the ASan blacklist should probably be used in the
>>>>>>>>> coverage instrumentation module pass.  Or it should have a separate
>>>>>>>>> blacklist...
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>> ––
>>>>>>>>> Timur
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Alexey Samsonov
>>>>>> vonosmas at gmail.com
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Alexey Samsonov
>>>> vonosmas at gmail.com
>>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150428/5bc34e41/attachment.html>