sanitizer-coverage vs asan_blacklist

Tue Apr 28 10:48:24 PDT 2015

On Tue, Apr 28, 2015 at 10:23 AM, Timur Iskhodzhanov <timurrrr at google.com>
wrote:

>
>
> вт, 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?
>

Oh, so this would be not really relevant to sanitizers. This attribute
makes sense, but... how would you enforce it? It will have to be supported
by
all sanitizers, coverage instrumentation, profiling, and maybe even
optimization passes.

>
> 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
>>
>

-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150428/e617a0f7/attachment.html>