[cfe-dev] clang attributes to disable asan/tsan/msan

[Richard Smith]

On Wed, Feb 20, 2013 at 8:55 PM, Kostya Serebryany <kcc at google.com> wrote:

>
>
>
> On Wed, Feb 20, 2013 at 6:53 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Mon, Feb 18, 2013 at 9:19 PM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>>
>>>
>>>
>>> On Tue, Feb 19, 2013 at 9:13 AM, Chandler Carruth <chandlerc at google.com>wrote:
>>>
>>>> On Mon, Feb 18, 2013 at 9:10 PM, Kostya Serebryany <kcc at google.com>wrote:
>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Feb 19, 2013 at 3:55 AM, Chandler Carruth <
>>>>> chandlerc at google.com> wrote:
>>>>>
>>>>>> On Mon, Feb 18, 2013 at 10:35 AM, Sean Silva <silvas at purdue.edu>wrote:
>>>>>>
>>>>>>> On Mon, Feb 18, 2013 at 8:31 AM, Kostya Serebryany <kcc at google.com>
>>>>>>> wrote:
>>>>>>> > Hi,
>>>>>>> >
>>>>>>> > Clang has two attributes to disable bug detection tools in a given
>>>>>>> function:
>>>>>>> >
>>>>>>> > __attribute__((no_thread_safety_analysis)) disables clang's
>>>>>>> *static*
>>>>>>> > thread-safety analysis.
>>>>>>> > (
>>>>>>> http://clang.llvm.org/docs/LanguageExtensions.html#thread-safety-annotation-checking
>>>>>>> )
>>>>>>> >
>>>>>>> > __attribute__((no_address_safety_analysis)) disables
>>>>>>> AddressSanitizer
>>>>>>> > (*dynamic* analysis)
>>>>>>> >
>>>>>>> http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-dynamic-analysis
>>>>>>> >
>>>>>>> > Now we need two more attributes to disable
>>>>>>> > ThreadSanitizer (http://clang.llvm.org/docs/ThreadSanitizer.html)
>>>>>>> > and MemorySanitizer (
>>>>>>> http://clang.llvm.org/docs/MemorySanitizer.html)
>>>>>>> >
>>>>>>> > For MemorySanitizer I propose
>>>>>>> __attribute__((no_uninitialized_checks))
>>>>>>> > Objections? Better naming suggestion?
>>>>>>> > Maybe __attribute__((no_memory_sanitizer))?
>>>>>>> > (We deliberately named no-asan attribute
>>>>>>> "no_address_safety_analysis" w/o
>>>>>>> > mentioning asan
>>>>>>> > in the name to make this attribute usable for other tools, e.g.
>>>>>>> SAFECode.
>>>>>>> > So,
>>>>>>> > we may not want to tie the no-msan attribute to msan)
>>>>>>>
>>>>>>> It seems to me like it is going to be simpler and more transparent to
>>>>>>> have the attribute explicitly mention the sanitizer, e.g.`
>>>>>>> __attribute__((no_sanitize("memory")))`; then the user knows exactly
>>>>>>> what they are getting (since the name corresponds with the command
>>>>>>> line option). If other tools want to use those attributes it's not
>>>>>>> hard to look for them.
>>>>>>>
>>>>>>> It also isn't entirely clear to me that the attribute would have
>>>>>>> exactly the same semantics for the sanitizers and some other tool.
>>>>>>> AFAIK the term "address safety" has no independent meaning and
>>>>>>> basically means "the things that asan checks", so the term "address"
>>>>>>> in `__attribute__((no_address_safety_analysis))` is already asan
>>>>>>> specific in that regard, and it would be clearer to just say
>>>>>>> `no_sanitize("memory")`.
>>>>>>>
>>>>>>> If we really want the attributes to be tool-agnostic, then they
>>>>>>> should
>>>>>>> describe what the function does that is naughty, e.g.
>>>>>>> `__attribute__((reads_unintialized_memory_on_purpose))`, and let the
>>>>>>> tool interpret that information and behave appropriately.
>>>>>>>
>>>>>>
>>>>>> This summarizes my feelings exactly.
>>>>>>
>>>>>> I think that even if we grow a set of attributes that describe the
>>>>>> semantic oddity of a function (such as reading uninitialized memory, etc),
>>>>>> we would still want an escape hatch to just turn off the sanitizer. And
>>>>>> when we do that, we really do want to use the exact same terminology that
>>>>>> we use in the flags.
>>>>>>
>>>>>> I don't think it matters whether its one attribute or N attributes as
>>>>>> long as we get some naming consistency. I would propose (for simplicity of
>>>>>> implementation mostly):
>>>>>>
>>>>>> __attribute__((no_sanitize_address))
>>>>>> __attribute__((no_sanitize_memory))
>>>>>> __attribute__((no_sanitize_thread))
>>>>>> __attribute__((no_sanitize_undefined))
>>>>>>
>>>>>
>>>>> I like the simplicity (also because we will have to implement these
>>>>> attributes in gcc too).
>>>>>
>>>>> How about this?
>>>>> __attribute__((no_sanitize_address)) is a synonym for
>>>>> __attribute__((no_address_safety_analysis)), i.e. disables AddressSanitizer
>>>>> checking.
>>>>> (or maybe we should just leave no_address_safety_analysis?)
>>>>>
>>>>> __attribute__((no_sanitize_memory)) disables MemorySanitizer checking,
>>>>> but still keeps the instrumentation required to avoid false positives.
>>>>>
>>>>> __attribute__((no_sanitize_thread)) disables ThreadSanitizer checking
>>>>> for plain (non-atomic) loads/stores, but still keeps the instrumentation
>>>>> required to avoid false positives.
>>>>>
>>>>
>>>> I like it. I would add all three so that we can update code to be
>>>> consistent.
>>>>
>>>
>>> Thanks, I'll send a patch  later unless I hear objections to this plan.
>>>
>>
>> I would like for us to aim towards having some harmony between this and
>> the sanitizer blacklist file. As a strawman, it would be nice if we could
>> say that
>>
>> __attribute__((no_sanitize("foo"))) void f();
>>
>> has the same effect as adding
>>
>> foo:_Z1fv
>>
>
> "foo" here is one of "fun", "src", "global" or "global-init".
> So, what would "  __attribute__((no_sanitize("glob"))) void f(); " mean?
> Or even "  __attribute__((no_sanitize("fun"))) void f(); " ?
>

I see, the blacklist file doesn't have the ability to control individual
sanitizers other than the global-init sanitization. Never mind then. We can
discuss what would work in the blacklist file if we come to add
finer-grained controls to it.


> --kcc
>
>
>
>
>> to the blacklist file. (This suggests that the attribute should be
>> allowed on globals for the global-init sanitizer, etc.)
>>
>>
>>>  Keep an eye out for a use case for an all-inclusive 'no_sanitize' that
>>>> turns everything off.
>>>>
>>>
>>> That's controversial.
>>> On one hand, I've seen a couple of cases where we need both
>>> no_sanitize_address and no_sanitize_memory (mostly, custom stack unwinders
>>> that read random bytes on stack).
>>> I can also imagine cases where no_sanitize_address
>>> and no_sanitize_thread could be used together (e.g. deliberate out of
>>> bounds reads within aligned 8-byte word).
>>> But we risk that no_sanitize will be abused to turn off all sanitizers
>>> when only two should really be disabled.
>>>
>>> I'd rather not add this for now.
>>>
>>> --kcc
>>>
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>> --kcc
>>>>>
>>>>>
>>>>>>  ...
>>>>>>
>>>>>> This pattern should be easy to remember and understand, and removes a
>>>>>> lot of ambiguity of which attribute goes with which sanitizer. It also
>>>>>> makes it very clear that these are attributes pertaining to the dynamic
>>>>>> analysis toolset, not to any static analysis toolset.
>>>>>>
>>>>>> Of course, I think we should support the existing attributes for
>>>>>> backwards compatibility, at least for several releases.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130220/497972f8/attachment.html>