[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 :-)
---
 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)))
 #   else
 #    define ATTRIBUTE_NONNULL(m)
-- 
1.7.10


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]