[PATCH] Do not warn with warn_unused_result for alloca(0).

[Martin Sebor]

On 6/12/19 5:37 AM, Jakub Jelinek wrote:
> On Wed, Jun 12, 2019 at 01:30:14PM +0200, Martin Liška wrote:
>> @@ -9447,10 +9448,19 @@ do_warn_unused_result (gimple_seq seq)
>>   	      location_t loc = gimple_location (g);
>>   
>>   	      if (fdecl)
>> -		warning_at (loc, OPT_Wunused_result,
>> -			    "ignoring return value of %qD "
>> -			    "declared with attribute %<warn_unused_result%>",
>> -			    fdecl);
>> +		{
>> +		  /* Some C libraries use alloca(0) in order to free previously
>> +		     allocated memory by alloca calls.  */
>> +		  if (gimple_maybe_alloca_call_p (g)
>> +		      && gimple_call_num_args (g) == 1
>> +		      && integer_zerop (gimple_call_arg (g, 0)))
>> +		    ;
>> +		  else
> 
> Wouldn't it be easier to negate the condition and avoid the weird ; else ?
> I.e. if (!gimple_maybe... || gimple_call_num != 1 || !integer_zerop?
> 
>> +		    warning_at (loc, OPT_Wunused_result,
>> +				"ignoring return value of %qD declared "
>> +				"with attribute %<warn_unused_result%>",
>> +				fdecl);
>> +		}
>>   	      else
>>   		warning_at (loc, OPT_Wunused_result,
>>   			    "ignoring return value of function "
> 
> Otherwise LGTM as the patch, but I'd like to hear from others whether
> it is kosher to add such a special case to the warn_unused_result attribute
> warning.  And if the agreement is yes, I think it should be documented
> somewhere that alloca (0) will not warn even when the call has such an
> attribute (probably in the description of warn_unused_result attribute).

I'm not very happy about adding another special case to alloca
(on top of not diagnosing zero allocation by -Walloc-zero).
There is no valid use case for the zero argument, whether or not
the return value is used.  When it is used it's just as dangerous
as allocating a zero-length VLA.  When it isn't used the call is
pointless entirely, and although it might be harmless, it's worth
removing from code just like any other pointless construct GCC
warns about.  So I would prefer not to suppress this warning (from
what I've read in the GDB bug it sounds like its calls to alloca(0)
are being removed).  I would also prefer to re-enable -Walloc-zero
for alloca(0).

But if there is insufficient support for this I agree that
documenting the built-ins GCC applies attribute warn_unused_result
to is a good idea.  (In fact, documenting all the attributes GCC
applies to each built-in would be helpful, but that's a much bigger
project.)

Martin