Re: who.c vs gcc-7's improved buffer overrun check

2016-09-22 Thread Jim Meyering
On Thu, Sep 22, 2016 at 9:31 AM, Pádraig Brady  wrote:
> On 22/09/16 16:03, Jim Meyering wrote:
>> I was intrigued to see my just-built-from-git gcc fail to compile who.c.
>>
>> It's probably too soon to accept a patch like this: in case gcc's
>> static analysis is going to be improved enough that it can derive the
>> invariant that the assert makes explicit.
>
> Maybe gcc can't infer that invariant due to the undefined behavior of signed 
> integer underflow?
> I.E. if 'when' and 'boottime' were close to TYPE_MINIMUM (time_t).
> Does gcc 7 avoid the warning if you put this extra condition in the if 
> (bootime... line?
>
>   && TYPE_MINIMUM (time_t) + 24*60*60 < when
>
>> So, perhaps just for reference, for now, here's the patch.
>> If I were to push it, due diligence would require a post to the gcc
>> list about this, and (assuming we retain the patch) the addition of a
>> FIXME-in-2018 comment to see if it is still needed.
>
> The assertion looks good to me,
> though if the above works that would be preferable.

Thanks for the suggestion, but appending that conjunct did not
suppress the warning. Hence, I've added this comment and will push
soon:

+  /* FIXME-in-2018: see if this assert is still required in order
+ to suppress gcc's unwarranted -Wformat-length= warning.  */



Re: who.c vs gcc-7's improved buffer overrun check

2016-09-22 Thread Pádraig Brady
On 22/09/16 16:03, Jim Meyering wrote:
> I was intrigued to see my just-built-from-git gcc fail to compile who.c.
> 
> It's probably too soon to accept a patch like this: in case gcc's
> static analysis is going to be improved enough that it can derive the
> invariant that the assert makes explicit.

Maybe gcc can't infer that invariant due to the undefined behavior of signed 
integer underflow?
I.E. if 'when' and 'boottime' were close to TYPE_MINIMUM (time_t).
Does gcc 7 avoid the warning if you put this extra condition in the if 
(bootime... line?

  && TYPE_MINIMUM (time_t) + 24*60*60 < when

> So, perhaps just for reference, for now, here's the patch.
> If I were to push it, due diligence would require a post to the gcc
> list about this, and (assuming we retain the patch) the addition of a
> FIXME-in-2018 comment to see if it is still needed.

The assertion looks good to me,
though if the above works that would be preferable.

thanks,
Pádraig