<div dir="ltr">Ok, looks like I wasn't updating this thread for a while, sorry about that. The current status is:<div>-fsanitize=returns-nonnull-attribute is implemented in Clang trunk</div><div>-fsanitize=nonnull-attribute is now being reviewed (<a href="http://reviews.llvm.org/D5082">http://reviews.llvm.org/D5082</a>). There were delays here due to numerous problems with __attribute__((nonnull)) implementation in Clang and uncertainties about nonnull semantic:</div>
<div><br></div><div> __attribute__((nonnull)) void foo(int a, ...);</div><div> void call_foo() { foo(42, (void*)0); } //should we report error in this case? Probably yes, as GCC prints compile-time warning here.</div><div>
<br></div><div>Now, the question is about the __ubsan_handle_nonnull_arg parameters. Originally you proposed, to pass SourceLocation of a call in static data and argument index in dynamic data (to be able to merge locations). However, I think the error reports can benefit if we:</div>
<div>1) pinpoint the location of a specific argument that incorrectly evaluates to NULL.</div><div>2) print the location of a function declaration which contains the nonnull attribute.</div><div>Note that if we do (1) we might as well pass argument index in static data, as locations of different arguments will not be merged together. Richard also raised a valid concern that passing argument index increases the code size instead of the .data, which might be not desirable for functions with few nonnull arguments.</div>
<div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 11, 2014 at 1:29 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div class="">On Mon, Aug 11, 2014 at 12:39 PM, Jakub Jelinek <span dir="ltr"><<a href="mailto:jakub@redhat.com" target="_blank">jakub@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Mon, Aug 11, 2014 at 12:05:00PM -0700, Alexey Samsonov wrote:<br>
> 1) Frontend flag naming. We can add new values: -fsanitize=returns-nonnull<br>
> and -fsanitize=nonnull. But it looks confusing to<br>
> me: having both "-fsanitize=null" and "-fsanitize=nonnull" is weird. Clang<br>
> user manual tells that -fsanitize=null detects<br>
> "Use of a null pointer or creation of a null reference." Maybe, we should<br>
> just put both new checks under "-fsanitize=null"<br>
> and make manual say "invalid use of a null pointer or creation of a null<br>
> reference". Passing null pointer to function annotated<br>
> with __attribute__((nonnull)) conforms to "invalid use of a null pointer".<br>
> As an alternative, we can introduce new "-fsanitize=nonnull-attributes" for<br>
> both new checks.<br>
<br>
</div>Sticking it under null means limiting user's choice, and just passing or<br>
returning a pointer isn't a use of the pointer. As one attribute is nonnull<br>
and another is returns_nonnull, calling them nonnull-attributes is weird to<br>
me. I don't find nonnull/returns-nonnull confusing, but could live with<br>
nonnull-attr/returns-nonnull-attr or<br>
nonnull-attribute/returns-nonnull-attribute.<br>
Perhaps nonnull could enable also those two, but then it would be a group<br>
name as well as sanitization name, which would confuse users too.<br></blockquote><div><br></div></div><div>Personally, I also feel that fine granularity is important for UBSan options</div><div>(and is a huge help in tool deployment), but let's agree with Richard on that.</div>
<div class="">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> 2) Handler arguments. Is there any particular reason to not include ArgNo<br>
> into NonNullArgData? Is it done to reduce the number of function calls /<br>
> basic blocks?<br>
<br>
</div>Yeah. Many functions have several nonnull arguments, including memcpy,<br>
memmove etc., and nonnull attribute without arguments after all means all<br>
pointer arguments must be nonnull. So, by not sticking the ArgNo into<br>
the structure the same data can be used for several calls.<br></blockquote><div><br></div></div><div>Sure, makes sense.</div><div class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> Another question is the meaning of SourceLocation in NonNullArgData and<br>
> NonNullRetData. I assume that SourceLocation in the first one should point<br>
> to the<br>
> invalid call of function with attributes, while SourceLocation in the<br>
> second one should point to the bad return statement inside the function<br>
> with attributes. Is that so?<br>
<br>
</div>Sure, it is the locus of the call statement for nonnull attribute and<br>
locus of the return statement on returns_nonnull attribute.<br>
<span><font color="#888888"><br>
Jakub<br>
</font></span></blockquote></div></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</div>