[Freeipa-devel] [PATCH 0390] Fix build with GCC 4.9+

[Lukas Slebodnik]

On (01/03/16 14:35), Petr Spacek wrote:
>On 1.3.2016 12:06, Lukas Slebodnik wrote:
>> On (25/02/16 15:57), Petr Spacek wrote:
>>> On 19.2.2016 13:55, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> Fix build with GCC 4.9+.
>>>>
>>>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>>>> attributes. This removes most of asserts() in the plugin.
>>>> GCC 6 adds warnings for these cases.
>>>>
>>>> We are disabling the unwanted condition pruning by adding
>>>> -fno-delete-null-pointer-checks argument.
>>>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>>>>
>>>> Additionally we silence warnings to prevent build failures when -Werror
>>>> is used.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>>>
>>> Updated version is attached. It contains less autotools magic because it
>>> enables attribute nonnull only under Clang static analyzer and Coverity - as a
>>> result we do not have to silence GCC warnings from -Wnonnull.
>>>
>>> Please review so I can fix build in Fedora 24.
>>>
>>> Thank you.
>>>
>>> -- 
>>> Petr^2 Spacek
>> 
>>>From 4732fe9f4e525c44b46e7ed0734ccaec94fba49e Mon Sep 17 00:00:00 2001
>>> From: Petr Spacek <pspacek at redhat.com>
>>> Date: Fri, 19 Feb 2016 13:39:27 +0100
>>> Subject: [PATCH] Fix build with GCC 4.9+.
>>>
>>> GCC 4.9+ is too aggressive when optimizing functions with nonnull
>>> attributes. This removes most of asserts() in the plugin.
>>> GCC 6 adds warnings for these cases.
>>>
>>> We are disabling the unwanted condition pruning by adding
>>> -fno-delete-null-pointer-checks argument.
>>> BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>>>
>>> Additionally we enable nonnull attribute only when the build is running under
>>> Clang static analyzer or Coverity.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>>> ---
>>> configure.ac | 13 +++++++++++++
>>> src/util.h   |  8 ++++++--
>>> 2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index a06708b1a5ee64bb64c80272c10ed1a35670c8d0..a0123ac0a62b5acd5238f028d8c42e83af4060db 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -39,6 +39,19 @@ AC_TRY_COMPILE([
>>> [CFLAGS="$SAVED_CFLAGS"
>>>  AC_MSG_RESULT([no])])
>>>
>>> +# Check if build chain supports -fno-delete-null-pointer-checks
>>> +# this flag avoids too agressive optimizations which would remove some asserts
>>> +# BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a
>>> +AC_MSG_CHECKING([for -fno-delete-null-pointer-checks compiler flag])
>>> +SAVED_CFLAGS="$CFLAGS"
>>> +CFLAGS="$CFLAGS -fno-delete-null-pointer-checks"
>>> +AC_TRY_COMPILE([
>>> +	extern int fdef(void);
>>> +],[],
>>> +[AC_MSG_RESULT([yes])],
>>> +[CFLAGS="$SAVED_CFLAGS"
>>> + AC_MSG_RESULT([no])])
>>> +
>> NACK.
>> 
>> It failes with clang.
>> 
>> configure:12982: checking for -fno-delete-null-pointer-checks compiler flag
>> configure:12999: clang -c -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -fvisibility=hidden -fno-delete-null-pointer-checks  conftest.c >&5
>> clang-3.8: warning: optimization flag '-fno-delete-null-pointer-checks' is not supported
>> clang-3.8: warning: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1'
>> configure:12999: $? = 0
>> configure:13000: result: yes
>> 
>> Reproducer:
>> autoreconf -if && CC=clang ./configure && make
>
>Thanks! I was testing this only with Clang static analyzer ...
>
>Here is updated patch.
>
>-- 
>Petr^2 Spacek

>From 6b2ac51fe4ff75c9f59499cbaa4306f70db46425 Mon Sep 17 00:00:00 2001
>From: Petr Spacek <pspacek at redhat.com>
>Date: Fri, 19 Feb 2016 13:39:27 +0100
>Subject: [PATCH] Fix build with GCC 4.9+.
>
>GCC 4.9+ is too aggressive when optimizing functions with nonnull
>attributes. This removes most of asserts() in the plugin.
>GCC 6 adds warnings for these cases.
>
>We are disabling the unwanted condition pruning by adding
>-fno-delete-null-pointer-checks argument.
>BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.
>
>Additionally we enable nonnull attribute only when the build is running under
>Clang static analyzer or Coverity.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1307346
>---
> configure.ac | 14 ++++++++++++++
> src/util.h   |  8 ++++++--
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
>diff --git a/configure.ac b/configure.ac
>index a06708b1a5ee64bb64c80272c10ed1a35670c8d0..48f5cb63c3bb5535fe1da56abe7583e15d4b5f92 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -39,6 +39,20 @@ AC_TRY_COMPILE([
> [CFLAGS="$SAVED_CFLAGS"
>  AC_MSG_RESULT([no])])
> 
>+# Check if build chain supports -fno-delete-null-pointer-checks
>+# this flag avoids too agressive optimizations which would remove some asserts
>+# BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a
>+AC_MSG_CHECKING([for -fno-delete-null-pointer-checks compiler flag])
>+SAVED_CFLAGS="$CFLAGS"
>+CFLAGS="-fno-delete-null-pointer-checks -Werror"
>+AC_TRY_COMPILE([
>+	extern int fdef(void);
>+],[],
>+[AC_MSG_RESULT([yes])
>+ CFLAGS="$SAVED_CFLAGS -fno-delete-null-pointer-checks"],
>+[CFLAGS="$SAVED_CFLAGS"
>+ AC_MSG_RESULT([no])])
>+
> # Get CFLAGS from isc-config.sh
> AC_ARG_VAR([BIND9_CFLAGS],
>            [C compiler flags for bind9, overriding isc-config.sh])
>diff --git a/src/util.h b/src/util.h
>index 9849ff9b6c38ec1c6dd143440d5b5e584b2ecd51..402503c339a5ab6ca5273cae420e743b9fc252ab 100644
>--- a/src/util.h
>+++ b/src/util.h
>@@ -103,11 +103,15 @@ extern isc_boolean_t verbose_checks; /* from settings.c */
> /* If no argument index list is given to the nonnull attribute,
>  * all pointer arguments are marked as non-null. */
> #define ATTR_NONNULLS     ATTR_NONNULL()
>-#ifdef __GNUC__
>+#if defined(__COVERITY__) || defined(__clang_analyzer__)
> #define ATTR_NONNULL(...) __attribute__((nonnull(__VA_ARGS__)))
>-#define ATTR_CHECKRESULT __attribute__((warn_unused_result))
> #else
> #define ATTR_NONNULL(...)
>+#endif
>+
>+#if defined(__GNUC__)
>+#define ATTR_CHECKRESULT __attribute__((warn_unused_result))
>+#else
> #define ATTR_CHECKRESULT
> #endif

ACK

LS