On 12 January 2018 at 18:26, Richard Biener <rguent...@suse.de> wrote: > 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. I have committed this as r260250 after bootstrap+test on x86_64 on top of trunk. With the patch, we now emit a suggestion for malloc attribute for a function returning NULL, which may not be ideal. I was wondering for which cases should we avoid suggesting malloc attribute with -Wsuggest-attribute ?
1] Return value is NULL. 2] Return value is phi result, and all args of phi are 0. 3] Any other cases ? Thanks, Prathamesh > > Thanks, > Richard.