[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] clang attributes to disable asan/tsan/msan
From: Matthieu Monrocq <matthieu.monrocq () gmail ! com>
Date: 2013-02-19 18:13:37
Message-ID: CAKE6Rfj4HL56jiTU4x026ArSpy1gerQWDYqD0ZwgnSXMLtGjRA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Tue, Feb 19, 2013 at 6:18 PM, Jordan Rose <jordan_rose@apple.com> wrote:
>
> On Feb 18, 2013, at 21:13 , Chandler Carruth <chandlerc@google.com> wrote=
:
>
> On Mon, Feb 18, 2013 at 9:10 PM, Kostya Serebryany <kcc@google.com> wrote=
:
>
>>
>>
>>
>> On Tue, Feb 19, 2013 at 3:55 AM, Chandler Carruth <chandlerc@google.com>=
wrote:
>>
>>> On Mon, Feb 18, 2013 at 10:35 AM, Sean Silva <silvas@purdue.edu> wrote:
>>>
>>>> On Mon, Feb 18, 2013 at 8:31 AM, Kostya Serebryany <kcc@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-annot=
ation-checking
>>>> )
>>>> >
>>>> > __attribute__((no_address_safety_analysis)) disables AddressSanitize=
r
>>>> > (*dynamic* analysis)
>>>> >
>>>> http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-dyna=
mic-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, et=
c),
>>> 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 th=
at
>>> 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 AddressSaniti=
zer
>> 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 fo=
r
>> 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.
>
> Keep an eye out for a use case for an all-inclusive 'no_sanitize' that
> turns everything off.
>
>
> I don't have a huge stake in this, but it kind of makes sense to me to
> have a single attribute that takes an argument, or list of arguments.
>
> __attribute__((no_sanitize(address)))
> __attribute__((no_sanitize(memory,thread)))
>
> This way, if more sanitizers are added, we don't need to add new
> attributes. And since these are *disabling* checks, they're automatically
> backwards-compatible=97that is, Clang could accept
>
> __attribute__((no_sanitize(address,insanity)))
>
> without warning that it doesn't know what the insanity sanitizer is. (Thi=
s
> is probably more important on GCC, which AFAIK still doesn't have an
> equivalent of has_attribute.)
>
> Just my two cents,
> Jordan
>
>
>
> I am always wary about the "no-warning". What happens if I write
no_sanitize(adress) ? (In French, there is a single "d" in adresse so
it's a common mistake) I would rather the compiler warned me that discover
after the whole compile-link-test cycle that I did not nail it.
If people want to disable the warning, let them.
-- Matthieu
> _______________________________________________
> cfe-dev mailing list
> cfe-dev@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">On Tue, Feb 19, 2013 at 6:18 PM, Jordan Rose <span \
dir="ltr"><<a href="mailto:jordan_rose@apple.com" \
target="_blank">jordan_rose@apple.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <div style="word-wrap:break-word"><div><div \
class="h5"><br><div><div>On Feb 18, 2013, at 21:13 , Chandler Carruth <<a \
href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>> \
wrote:</div><br><blockquote type="cite"> <div dir="ltr">On Mon, Feb 18, 2013 at 9:10 \
PM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" \
target="_blank">kcc@google.com</a>></span> wrote:<br><div class="gmail_extra"><div \
class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div \
class="gmail_quote"><div><div>On Tue, Feb 19, 2013 at 3:55 AM, Chandler Carruth <span \
dir="ltr"><<a href="mailto:chandlerc@google.com" \
target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr"><div>On Mon, Feb 18, 2013 at 10:35 AM, Sean Silva <span dir="ltr"><<a \
href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> \
wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>On \
Mon, Feb 18, 2013 at 8:31 AM, Kostya Serebryany <<a href="mailto:kcc@google.com" \
target="_blank">kcc@google.com</a>> wrote:<br>
> Hi,<br>
><br>
> Clang has two attributes to disable bug detection tools in a given function:<br>
><br>
> __attribute__((no_thread_safety_analysis)) disables clang's *static*<br>
> thread-safety analysis.<br>
> (<a href="http://clang.llvm.org/docs/LanguageExtensions.html#thread-safety-annotation-checking" \
target="_blank">http://clang.llvm.org/docs/LanguageExtensions.html#thread-safety-annotation-checking</a>)<br>
><br>
> __attribute__((no_address_safety_analysis)) disables AddressSanitizer<br>
> (*dynamic* analysis)<br>
> <a href="http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-dynamic-analysis" \
target="_blank">http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-dynamic-analysis</a><br>
><br>
> Now we need two more attributes to disable<br>
> ThreadSanitizer (<a href="http://clang.llvm.org/docs/ThreadSanitizer.html" \
target="_blank">http://clang.llvm.org/docs/ThreadSanitizer.html</a>)<br> > and \
MemorySanitizer (<a href="http://clang.llvm.org/docs/MemorySanitizer.html" \
target="_blank">http://clang.llvm.org/docs/MemorySanitizer.html</a>)<br> ><br>
> For MemorySanitizer I propose __attribute__((no_uninitialized_checks))<br>
> Objections? Better naming suggestion?<br>
> Maybe __attribute__((no_memory_sanitizer))?<br>
> (We deliberately named no-asan attribute "no_address_safety_analysis" \
w/o<br> > mentioning asan<br>
> in the name to make this attribute usable for other tools, e.g. SAFECode.<br>
> So,<br>
> we may not want to tie the no-msan attribute to msan)<br>
<br>
</div>It seems to me like it is going to be simpler and more transparent to<br>
have the attribute explicitly mention the sanitizer, e.g.`<br>
__attribute__((no_sanitize("memory")))`; then the user knows exactly<br>
what they are getting (since the name corresponds with the command<br>
line option). If other tools want to use those attributes it's not<br>
hard to look for them.<br>
<br>
It also isn't entirely clear to me that the attribute would have<br>
exactly the same semantics for the sanitizers and some other tool.<br>
AFAIK the term "address safety" has no independent meaning and<br>
basically means "the things that asan checks", so the term \
"address"<br> in `__attribute__((no_address_safety_analysis))` is already \
asan<br> specific in that regard, and it would be clearer to just say<br>
`no_sanitize("memory")`.<br>
<br>
If we really want the attributes to be tool-agnostic, then they should<br>
describe what the function does that is naughty, e.g.<br>
`__attribute__((reads_unintialized_memory_on_purpose))`, and let the<br>
tool interpret that information and behave \
appropriately.<br></blockquote><div><br></div></div><div>This summarizes my feelings \
exactly.</div><div><br></div><div>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.</div>
<div><br></div><div>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):</div><div> \
<br></div><div>__attribute__((no_sanitize_address))<br></div><div>__attribute__((no_sa \
nitize_memory))<br></div><div><div>__attribute__((no_sanitize_thread))<br></div><div>__attribute__((no_sanitize_undefined))</div></div>
</div></div></div></blockquote><div><br></div></div></div><div>I like the simplicity \
(also because we will have to implement these attributes in gcc too). \
</div><div><br></div><div>How about this? \
</div><div>__attribute__((no_sanitize_address)) is a synonym for \
__attribute__((no_address_safety_analysis)), i.e. disables AddressSanitizer checking. \
<br>
</div><div>(or maybe we should just leave \
no_address_safety_analysis?)</div><div><br></div><div>__attribute__((no_sanitize_memory)) \
disables MemorySanitizer checking, but still keeps the instrumentation required to \
avoid false positives. <br>
</div><div><br></div><div>__attribute__((no_sanitize_thread)) disables \
ThreadSanitizer checking for plain (non-atomic) loads/stores, but still keeps the \
instrumentation required to avoid false positives.<br></div></div></div>
</div></blockquote><div><br></div><div>I like it. I would add all three so that we \
can update code to be consistent.</div><div><br></div><div>Keep an eye out for a use \
case for an all-inclusive 'no_sanitize' that turns everything off.</div>
</div></div></div></blockquote><br></div></div></div><div>I don't have a huge \
stake in this, but it kind of makes sense to me to have a single attribute that takes \
an argument, or list of arguments.</div><div><br></div> \
<div>__attribute__((no_sanitize(address)))</div><div>__attribute__((no_sanitize(memory,thread)))</div><div><br></div><div>This \
way, if more sanitizers are added, we don't need to add new attributes. And since \
these are <i>disabling</i> checks, they're automatically \
backwards-compatible—that is, Clang could accept</div> \
<div><br></div><div>__attribute__((no_sanitize(address,insanity)))</div><div><br></div><div>without \
warning that it doesn't know what the insanity sanitizer is. (This is probably \
more important on GCC, which AFAIK still doesn't have an equivalent of \
has_attribute.)</div> <div><br></div><div>Just my two \
cents,</div><div>Jordan</div><div><br></div><br></div><br></blockquote><div>I am \
always wary about the "no-warning". What happens if I write \
no_sanitize(adress) ? (In French, there is a single "d" in adresse so \
it's a common mistake) I would rather the compiler warned me that discover after \
the whole compile-link-test cycle that I did not nail it.<br> <br>If people want to \
disable the warning, let them.<br><br>-- Matthieu<br> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" \
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br> \
<br></blockquote></div><br>
_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic