This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR83648
- From: Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Jeff Law <law at redhat dot com>, Jan Hubicka <hubicka at ucw dot cz>, gcc Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 15 May 2018 11:41:53 +0530
- Subject: Re: PR83648
- References: <CAAgBjMnap+eF165TdbZrpAmU2=stko03z0OOzsHwd_Bca+MN9g@mail.gmail.com> <20180103132859.GC39213@kam.mff.cuni.cz> <CAAgBjMksL=HYaQyA2qNbf3Q=hF+CwAu2Rw+ZyJm4TvzRL3Fr9A@mail.gmail.com> <9c2a52f5-b54f-3b29-3aa6-bfb595e2b788@redhat.com> <CAAgBjMnTXXU6gVxe2n=HMMgKb_hA3NbGL7A4_KovZypSrtBhOg@mail.gmail.com> <f31b4d23-a6b5-9f85-1fe3-f18ffb68b3fb@redhat.com> <CAAgBjM=g-dBoo_9iNnhbO2=NsH62ePLGgdKSw8in23B60gAP6w@mail.gmail.com> <alpine.LSU.2.20.1801121356410.32271@zhemvz.fhfr.qr>
On 12 January 2018 at 18:26, Richard Biener <rguenther@suse.de> wrote:
> On Fri, 12 Jan 2018, Prathamesh Kulkarni wrote:
>
>> 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 ?
>
> 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.