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

[Richard Smith]

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

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/20130219/6689769d/attachment.html>