Re: [PATCH 10/12] Warn when gettimeofday is called with non-null tzp argument.

[Florian Weimer <fweimer at redhat dot com>]

* Zack Weinberg:

> On 8/21/19 12:10 PM, Florian Weimer wrote:
>> * Zack Weinberg:
>> 
>>> I didn't look into it in any detail, but the uses of __warndecl +
>>> __builtin_constant_p in bits/string2.h inlines all follow the pattern
>>>
>>>     __extern_always_inline T func (ptr)
>>>     {
>>>       if (__builtin_constant_p (ptr) && something_else_p (ptr))
>>> __issue_warning ();
>>>       ...
>>>     }
>>>
>>> that is, issue a warning if ptr is a compile-time constant with some
>>> property.  What we want for gettimeofday is just the opposite,
>>>
>>>     if (! (__builtin_constant_p (ptr) && ptr == NULL)) __issue_warning ();
>>>
>>> that is, warn whenever ptr is _not_ a compile-time NULL.  I wouldn't
>>> be surprised if GCC's earliest unreachable-code removal passes, the
>>> ones that happen early enough to prevent __attribute__((warning))
>>> diagnostics from triggering, were tuned precisely for what
>>> bits/string2.h does, and don't handle this inverse case properly.
>> 
>> I don't think this will work.  You would have to use something like
>> this:
>> 
>>   __builtin_constant_p (ptr != NULL) && ptr != NULL
>> 
>> Otherwise you will produce a warning every time someone uses the
>> gettimeofday wrapper in a function for which optimization has been
>> disabled.
>
> That is the behavior I was seeing when the wrapper was an extern inline,
> but not when it is a macro.  I'm going to do more thorough testing.

I don't think we should turn gettimeofday into a macro.  It's likely to
cause problems with some C++ wrapper libraries.

> Also, `__builtin_constant_p (ptr != NULL) && ptr != NULL` would warn
> only for compile-time non-NULL, if I'm reading it right.

Correct.

> But in this case we also want to issue a warning for any argument that
> isn't a compile-time constant.  In other words, we want to warn for
> everything _except_ compile-time NULL.  Hence `! (__builtin_constant_p
> (ptr) && ptr == NULL)`.

I think this is simply not possible because it will cause unpredictable
false positives.

Thanks,
Florian