From e086deda3c99a72efa3e4f7acbe17c31d9973bb4 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 18 Feb 2013 13:34:58 -0700 Subject: [PATCH] build: force correct gcc syntax for attribute_nonnull Gcc lets you do: int ATTRIBUTE_NONNULL(1) foo(void *param); int foo(void *param) ATTRIBUTE_NONNULL(1); int ATTRIBUTE_NONNULL(1) foo(void *param) { ... } but chokes on: int foo(void *param) ATTRIBUTE_NONNULL(1) { ... } However, since commit eefb881, we have intentionally been disabling ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that is, gcc doesn't do decent warning reporting, then compiles code that mysteriously fails if you break the contract of the attribute, which is surprisingly easy to do), leaving it on only for Coverity (which does a much better job of improved static analysis when the attribute is present). But completely eliding the macro makes it too easy to write code that uses the fourth syntax option, if you aren't using Coverity. So this patch forces us to avoid syntax errors, even when not using the attribute under gcc. It also documents WHY we disable the warning under gcc, rather than forcing you to find the commit log. * src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute, rather than nothing, when on gcc. --- src/internal.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/internal.h b/src/internal.h index ebc91c7d91..2cf4731919 100644 --- a/src/internal.h +++ b/src/internal.h @@ -181,9 +181,22 @@ # endif # endif +/* gcc's handling of attribute nonnull is less than stellar - it does + * NOT improve diagnostics, and merely allows gcc to optimize away + * null code checks even when the caller manages to pass null in spite + * of the attribute, leading to weird crashes. Coverity, on the other + * hand, knows how to do better static analysis based on knowing + * whether a parameter is nonnull. Make this attribute conditional + * based on whether we are compiling for real or for analysis, while + * still requiring correct gcc syntax when it is turned off. See also + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 */ # ifndef ATTRIBUTE_NONNULL -# if __GNUC_PREREQ (3, 3) && STATIC_ANALYSIS -# define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m))) +# if __GNUC_PREREQ (3, 3) +# if STATIC_ANALYSIS +# define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m))) +# else +# define ATTRIBUTE_NONNULL(m) __attribute__(()) +# endif # else # define ATTRIBUTE_NONNULL(m) # endif