On Fri, 12 Jan 2018, Prathamesh Kulkarni wrote:
> On 12 January 2018 at 05:02, Jeff Law <l...@redhat.com> wrote:
> > On 01/10/2018 10:04 PM, Prathamesh Kulkarni wrote:
> >> On 11 January 2018 at 04:50, Jeff Law <l...@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 ?
Ok.
Thanks,
Richard.
