Fix PR78154

Richard Biener rguenther@suse.de
Wed Nov 23 09:42:00 GMT 2016


On Wed, 23 Nov 2016, Prathamesh Kulkarni wrote:

> On 22 November 2016 at 20:23, Richard Biener <rguenther@suse.de> wrote:
> > On Tue, 22 Nov 2016, Prathamesh Kulkarni wrote:
> >
> >> On 21 November 2016 at 15:34, Richard Biener <rguenther@suse.de> wrote:
> >> > On Fri, 18 Nov 2016, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 17 November 2016 at 15:24, Richard Biener <rguenther@suse.de> wrote:
> >> >> > On Thu, 17 Nov 2016, Prathamesh Kulkarni wrote:
> >> >> >
> >> >> >> On 17 November 2016 at 14:21, Richard Biener <rguenther@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)



More information about the Gcc-patches mailing list