Counter intuitively, asserts hurt gcc static dataflow analysis.

[John Carter]

Here is an example where gcc's optimizers go, ahhh, strange....

Try play with the optimizations settings on godbolt.
https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(j:1,lang:___c,source:'%23include+%3Cstdlib.h%3E%0A%0Avoid+assertFailure(+void)+__attribute__((warning(%22Compile+time+assertion+failure%22)))%3B%0A%0Aint+z(void)%3B%0A%0A%0Aint+main()%0A%7B%0A+++int+j%3B%0A%0A+++for(++j%3D0%3Bj%3C4%3Bj%2B%2B)+%7B%0A++++++if(+z())+break%3B%0A+++%7D%0A+++%0A+++if(+__builtin_constant_p(!!(j+%3C+4)))%0A+++%7B%0A++++++if(!!(j+%3C+4))%0A+++++++++assertFailure()%3B%0A+++%7D%0A+++else%0A++++++if(+!!(j+%3C+4))%0A+++++++++abort()%3B%0A+++%0A+++return+0%3B%0A%7D%0A'),l:'5',n:'0',o:'C+source+%231',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:cg81,filters:(b:'0',binary:'1',commentOnly:'0',demangle:'0',directives:'0',execute:'1',intel:'0',trim:'0'),lang:___c,libs:!(),options:'-Os+-Wall',source:1),l:'5',n:'0',o:'x86-64+gcc+8.1+(Editor+%231,+Compiler+%231)+C',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4

On -Os it behaves sane, on -O2 or higher it calls assertFailure.

#include <stdlib.h>

void assertFailure( void) __attribute__((warning("Compile time assertion
failure")));

int z(void);


int main()
{
   int j;

   for(  j=0;j<4;j++) {
      if( z()) break;
   }

   if( __builtin_constant_p(!(j < 4)))
   {
      if(!(j < 4))
         assertFailure();
   }
   else
      if( !(j < 4))
         abort();

   return 0;
}



On Thu, May 10, 2018 at 10:30 AM, John Carter <john.carter@taitradio.com>
wrote:

> I have just put something very similar to this in our code base....
>
> extern void __attribute__((error("Smart assert always failed")))
>     __smartAssertAlwaysFail(void);
>
> #define smart_assert(x) do { \
>     if (__builtin_constant_p(x)) { \
>         if (!(x)) __smartAssertAlwaysFail(); \
>     } else { \
>         assert(x); \
>     }
>
> ...and it's working quite well. Better than I expected in that it's not
> just catching things that are declared const.
>
> It catches stupid things like....
>
> if( expression) {
>     ....
>    assert( !expression);
> }
>
> It catches things that can be inlined but seen as constant at comile time.
>
> I think as gcc's optimizer get's smarter this will be quite a win.
>
> It also basically subsumes static_assert()
>
> It's not the full solution I'm looking for, but hey, it's better than
> before.
>
> Where people have done things like....
>
> switch(x) {
> case a:
> case b:
>  ....
> default:
>    assert(false);
> }
>
> Yup, it yowls about that.
>
> But I have created another noreturn function assert_not_reached() and
> replaced all assert(false) by assert_not_reached();
>
>
>
> On Wed, May 9, 2018 at 10:13 PM, David Brown <david@westcontrol.com>
> wrote:
>
>> On 09/05/18 10:35, Jonathan Wakely wrote:
>> > On 4 May 2018 at 14:34, Segher Boessenkool wrote:
>> >> On Fri, May 04, 2018 at 03:16:14PM +0200, Mason wrote:
>> >>> On 04/05/2018 01:03, John Carter wrote:
>> >>>
>> >>>> But compile with ...
>> >>>> gcc  -O3 -W -Wall -Wextra -o a a.c
>> >>>> ...now results in NO warnings!
>> >>>>
>> >>>> ie. Although gcc _knows_ the assert  _will_ trigger at run time...
>> it can't
>> >>>> tell me at compile time anymore.
>> >>>>
>> >>>> ie. Counter intuitively, adding asserts and error checks to my code
>> has
>> >>>> made me less safe.
>> >>>
>> >>> In the first version, gcc inlines the function call, which enables
>> >>> further analysis. In the second version, the assert() call makes
>> >>> gcc decide not to inline the function call, thus later analysis passes
>> >>> are no longer able to spot the out-of-bounds access.
>> >>
>> >> No, that's not it.  In the second version there *is* no out of bounds
>> >> access!
>> >
>> > Right, the assert means that if the access would have been out of
>> > bounds the program terminates. So (when NDEBUG is not defined) it's
>> > impossible to reach the array access with an index >= 4.
>> >
>> > It doesn't hurt GCC's analysis, it just changes the program, and the
>> > analysis works on the new program.
>> >
>>
>>
>> What you might want here is a smarter assert:
>>
>> extern void __attribute__((error("Smart assert always failed")))
>>     __smartAssertAlwaysFail(void);
>>
>> #define smart_assert(x) do { \
>>     if (__builtin_constant_p(x)) { \
>>         if (!(x)) __smartAssertAlwaysFail(); \
>>     } else { \
>>         assert(x); \
>>     }
>>
>>
>> I use something similar for assertions in some of my embedded code (I
>> don't use normal asserts, because there is no output or way to exit the
>> program).
>>
>> I am sure something related could be put in the normal assert macro -
>> perhaps with a warning rather than an error.
>>
>>
>
>
> --
> John Carter
> Phone : (64)(3) 358 6639
> Tait Electronics
> PO Box 1645 Christchurch
> New Zealand
>
>


-- 
John Carter
Phone : (64)(3) 358 6639
Tait Electronics
PO Box 1645 Christchurch
New Zealand

-- 
This Communication is Confidential. We only send and receive email on the

basis of the terms set out at www.taitradio.com/email_disclaimer 
<http://www.taitradio.com/email_disclaimer>