Re: PR83648

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 12 January 2018 at 05:02, Jeff Law <law@redhat.com> wrote:
> On 01/10/2018 10:04 PM, Prathamesh Kulkarni wrote:
>> On 11 January 2018 at 04:50, Jeff Law <law@redhat.com> wrote:
>>> On 01/09/2018 05:57 AM, Prathamesh Kulkarni wrote:
>>>>
>>>> As Jakub pointed out for the case:
>>>> void *f()
>>>> {
>>>>   return __builtin_malloc (0);
>>>> }
>>>>
>>>> The malloc propagation would set f() to malloc.
>>>> However AFAIU, malloc(0) returns NULL (?) and the function shouldn't
>>>> be marked as malloc ?
>>> This seems like a pretty significant concern.   Given:
>>>
>>>
>>>  return  n ? 0 : __builtin_malloc (n);
>>>
>>> Is the function malloc-like enough to allow it to be marked?
>>>
>>> If not, then ISTM we have to be very conservative in what we mark.
>>>
>>> foo (n, m)
>>> {
>>>   return n ? 0 : __builtin_malloc (m);
>>> }
>>>
>>> Is that malloc-like enough to mark?
>> Not sure. Should I make it more conservative by marking it as malloc
>> only if the argument to __builtin_malloc
>> is constant or it's value-range is known not to include 0? And
>> similarly for __builtin_calloc ?
> It looks like the consensus is we don't need to worry about the cases
> above.  So unless Jakub chimes in with a solid reason, don't worry about
> them.
Thanks everyone for the clarification. The attached patch skips on 0 phi arg,
and returns false if -fno-delete-null-pointer-checks is passed.

With the patch, malloc_candidate_p returns true for
return 0;
or
ret = phi<0, 0>
return ret

which I believe is OK as far as correctness is concerned.
However as Martin points out suggesting malloc attribute for return 0
case is not ideal.
I suppose we can track the return 0 (or when value range of return
value is known not to include 0)
corner case and avoid suggesting malloc for those ?

Validation in progress.
Is this patch OK for next stage-1 ?

Thanks,
Prathamesh
>
> Jeff

Attachment: pr83648-4.diff
Description: Text document