On Wed, 23 Nov 2016, Prathamesh Kulkarni wrote:
> On 22 November 2016 at 20:23, Richard Biener <rguent...@suse.de> wrote:
> > On Tue, 22 Nov 2016, Prathamesh Kulkarni wrote:
> >
> >> On 21 November 2016 at 15:34, Richard Biener <rguent...@suse.de> wrote:
> >> > On Fri, 18 Nov 2016, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 17 November 2016 at 15:24, Richard Biener <rguent...@suse.de> wrote:
> >> >> > On Thu, 17 Nov 2016, Prathamesh Kulkarni wrote:
> >> >> >
> >> >> >> On 17 November 2016 at 14:21, Richard Biener <rguent...@suse.de>
> >> >> >> wrote:
> >> >> >> > On Thu, 17 Nov 2016, Prathamesh Kulkarni wrote:
> >> >> >> >
> >> >> >> >> Hi Richard,
> >> >> >> >> Following your suggestion in PR78154, the patch checks if stmt
> >> >> >> >> contains call to memmove (and friends) in
> >> >> >> >> gimple_stmt_nonzero_warnv_p
> >> >> >> >> and returns true in that case.
> >> >> >> >>
> >> >> >> >> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> >> >> >> >> Cross-testing on arm*-*-*, aarch64*-*-* in progress.
> >> >> >> >> Would it be OK to commit this patch in stage-3 ?
> >> >> >> >
> >> >> >> > As people noted we have returns_nonnull for this and that is
> >> >> >> > already
> >> >> >> > checked. So please make sure the builtins get this attribute
> >> >> >> > instead.
> >> >> >> OK thanks, I will add the returns_nonnull attribute to the required
> >> >> >> string builtins.
> >> >> >> I noticed some of the string builtins don't have RET1 in
> >> >> >> builtins.def:
> >> >> >> strcat, strncpy, strncat have ATTR_NOTHROW_NONNULL_LEAF.
> >> >> >> Should they instead be having ATTR_RET1_NOTHROW_NONNULL_LEAF similar
> >> >> >> to entries for memmove, strcpy ?
> >> >> >
> >> >> > Yes, I think so.
> >> >> Hi,
> >> >> In the attached patch I added returns_nonnull attribute to
> >> >> ATTR_RET1_NOTHROW_NONNULL_LEAF,
> >> >> and changed few builtins like strcat, strncpy, strncat and
> >> >> corresponding _chk builtins to use ATTR_RET1_NOTHROW_NONNULL_LEAF.
> >> >> Does the patch look correct ?
> >> >
> >> > Hmm, given you only change ATTR_RET1_NOTHROW_NONNULL_LEAF means that
> >> > the gimple_stmt_nonzero_warnv_p code is incomplete -- it should
> >> > infer returns_nonnull itself from RET1 (which is fnspec("1") basically)
> >> > and the nonnull attribute on the argument. So
> >> >
> >> > unsigned rf = gimple_call_return_flags (stmt);
> >> > if (rf & ERF_RETURNS_ARG)
> >> > {
> >> > tree arg = gimple_call_arg (stmt, rf & ERF_RETURN_ARG_MASK);
> >> > if (range of arg is ! VARYING)
> >> > use range of arg;
> >> > else if (infer_nonnull_range_by_attribute (stmt, arg))
> >> > ... nonnull ...
> >> >
> >> Hi,
> >> Thanks for the suggestions, modified gimple_stmt_nonzero_warnv_p
> >> accordingly in this version.
> >> For functions like stpcpy that return nonnull but not one of it's
> >> arguments, I added new enum ATTR_RETNONNULL_NOTHROW_LEAF.
> >> Is that OK ?
> >> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> >> Cross-testing on arm*-*-*, aarch64*-*-* in progress.
> >
> > + value_range *vr = get_value_range (arg);
> > + if ((vr && vr->type != VR_VARYING)
> > + || infer_nonnull_range_by_attribute (stmt, arg))
> > + return true;
> > + }
> >
> > actually that's not quite correct (failed to notice the function
> > doesn't return a range but whether the range is nonnull). For
> > nonnull it's just
> >
> > if (infer_nonnull_range_by_attribute (stmt, arg))
> > return true;
> >
> > in the extract_range_basic call handling we could handle
> > ERF_RETURNS_ARG by returning the range of the argument (if not varying).
> >
> > Thus the patch is ok with the above condition changed. Please refer
> > to the recently opened PR from the ChangeLog.
> Err sorry I think had looked at wrong results for bootstrap+test for
> previous patch :/
> It caused ICE for pr55890-3.c and regressed nonnull-3.c, which is fixed in
> this
> version, and changed the above condition to only check
> infer_nonnull_range_by_attribute().
> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> Cross-tested on arm*-*-*, aarch64*-*-*.
> OK to commit ?
Ok.
Richard.
> Thanks,
> Prathamesh
> >
> > Thanks,
> > Richard.
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
