sanitizer-coverage vs asan_blacklist

[Alexey Samsonov]

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? 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/0e09bebe/attachment.html>