[libvirt] [PATCH] build: make ATTRIBUTE_NONNULL() a NOP unless STATIC_ANALYSIS is on

[Eric Blake]

On 04/26/2012 12:56 AM, Laine Stump wrote:
> The ATTRIBUTE_NONNULL(m) macro normally resolves to the gcc builtin
> __attribute__((__nonnull__(m))). The effect of this in gcc is
> unfortunately only to make gcc believe that "m" can never possibly be
> NULL, *not* to add in any checks to guarantee that it isn't ever NULL
> (i.e. it is an optimization aid, *not* something to verify code
> correctness.) - see the following gcc bug report for more details:
> 
>   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
> 
> Static source analyzers such as clang and coverity apparently can use
> ATTRIBUTE_NONNULL(), though, to detect dead code (in the case that the
> arg really is guaranteed non-NULL), as well as situations where an
> obviously NULL arg is given to the function.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=815270 is a good example
> of a bug caused by erroneous application of ATTRIBUTE_NONNULL().
> Several people spent a long time staring at this code and not finding
> the problem, because the problem wasn't in the function itself, but in
> the prototype that specified ATTRIBUTE_NONNULL() for an arg that
> actually *wasn't* always non-NULL, and caused a segv when dereferenced
> (even though the code that dereferenced the pointer was inside an if()
> that checked for a NULL pointer, that code was optimized out by gcc).
> 
> There may be some very small gain to be had from the optimizations
> that can be inferred from ATTRIBUTE_NONNULL(), but it seems safer to
> err on the side of generating code that behaves as expected, while
> turning on the attribute for static analyzers.
> 
> (dissenting opinions welcome :-)

None from me :)

> ---
>  src/internal.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/internal.h b/src/internal.h
> index ef81cda..83f468d 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -182,7 +182,7 @@
>  #  endif
>  
>  #  ifndef ATTRIBUTE_NONNULL
> -#   if __GNUC_PREREQ (3, 3)
> +#   if __GNUC_PREREQ (3, 3) && STATIC_ANALYSIS
>  #    define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m)))

ACK.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120426/dc9bc3d1/attachment-0001.sig>