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

Fri Mar 4 14:04:46 UTC 2016

On 3.3.2016 08:59, Lukas Slebodnik wrote:
> 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

Thanks, pushed to master:
acd26eedac09749f82646ea1ac0a662109cca7a8

-- 
Petr^2 Spacek