assert: Suppress pedantic warning caused by statement expression [BZ# 21242]

  On 07/04/2017 04:00 PM, Florian Weimer wrote:
> On 06/26/2017 12:56 PM, Joseph Myers wrote:
>> On Sun, 25 Jun 2017, Florian Weimer wrote:
>>
>>> I think we can use __extension__ if we also expand the expression
>>> without __extension__ in an unevaluated context.  The tricky part is to
>>> find one that is independent of GNU extensions.
>>> Perhaps this would work?
>>>
>>> #  define assert(expr)                                           \
>>>   ((void) sizeof ((expr) == 0), __extension__ ({                 \
>>>       if (expr)                                                  \
>>>         ; /* empty */                                            \
>>>       else                                                       \
>>>         __assert_fail (#expr, __FILE__, __LINE__, __FUNCTION__); \
>>>     }))
>>>
>>> sizeof suppresses the evaluation of the first occurrence of expr.  The
>>> comparison is needed because sizeof cannot be applied to function
>>> pointers and bitfields.  C11 says that expr is compared to zero, so the
>>> (expr) == 0 expression is well-formed.
>>>
>>> What do you think?  Should we make this change?
>>
>> I think that's reasonable (appropriately commented to explain why it's 
>> done that way).
> 
> This is what I came up with as a patch.
> 
> I would consider this still appropriate during the freeze.  It's
> something we'd backport to glibc 2.25, too.

Hopefully now with attachment.

Florian
  

On Tue, Jul 4, 2017 at 10:10 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/04/2017 04:00 PM, Florian Weimer wrote:
>> On 06/26/2017 12:56 PM, Joseph Myers wrote:
>>> On Sun, 25 Jun 2017, Florian Weimer wrote:
>>>
>>>> I think we can use __extension__ if we also expand the expression
>>>> without __extension__ in an unevaluated context.
...
>>
>> This is what I came up with as a patch.
>>
>> I would consider this still appropriate during the freeze.  It's
>> something we'd backport to glibc 2.25, too.
>
> Hopefully now with attachment.
...

> +/* The first occurrence of EXPR is not evaluated due to the sizeof,
> +   but will trigger any pedantic warnings masked by the __extension__
> +   for the second occurrence.  The explicit comparison against zero is
> +   required to support function pointers and bit fields in this
> +   context.  */
> #  define assert(expr)                            \
> -    ({                                    \
> +  ((void) sizeof ((expr) == 0), __extension__ ({            \

A problem occurs to me: expressions involving VLAs _are_ evaluated
inside sizeof.  Consider

   extern int f(int);
   void g(void)
   {
       const int n = 4;
       int array[n] = { 0, 0, 0, 0 }; // in C99, array is a VLA
       assert(array[f(n)] == 0);
    }

f() should be called only once, but I _think_ it will be called twice
with this definition of assert.

zw
  

On 07/05/2017 05:46 PM, Zack Weinberg wrote:
> A problem occurs to me: expressions involving VLAs _are_ evaluated
> inside sizeof.  Consider
> 
>    extern int f(int);
>    void g(void)
>    {
>        const int n = 4;
>        int array[n] = { 0, 0, 0, 0 }; // in C99, array is a VLA
>        assert(array[f(n)] == 0);
>     }
> 
> f() should be called only once, but I _think_ it will be called twice
> with this definition of assert.

The type of the sizeof argument would still be int (due to the
comparison against 0), so this doesn't actually occur.

Florian
  

On Wed, Jul 5, 2017 at 11:51 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/05/2017 05:46 PM, Zack Weinberg wrote:
>> A problem occurs to me: expressions involving VLAs _are_ evaluated
>> inside sizeof.
>
> The type of the sizeof argument would still be int (due to the
> comparison against 0), so this doesn't actually occur.

I rechecked what C99 says about sizeof and VLAs, and you're right -
the operand of sizeof is only evaluated when sizeof is _directly_
applied to a VLA.  So this is indeed safe, but I think this wrinkle
should be mentioned in the comment.  Perhaps

/* The first occurrence of EXPR is not evaluated due to the sizeof,
   but will trigger any pedantic warnings masked by the __extension__
   for the second occurrence.  The explicit comparison against zero
   ensures that sizeof is not directly applied to a function pointer or
   bit-field (which would be ill-formed) or VLA (which would be evaluated).  */

zw