Message ID | 20180411074805.17572-1-krebbel@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Series | Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins. | expand |
On Wed, Apr 11, 2018 at 09:48:05AM +0200, Andreas Krebbel wrote: > c-c++-common/attr-nonstring-3.c fails on IBM Z. The reason appears to be > that we provide builtin implementations for strcpy and stpcpy. The > warnings currently will only be emitted when expanding these as normal > calls. > > Bootstrapped and regression tested on x86_64 and s390x. > > Ok? > > gcc/ChangeLog: > > 2018-04-11 Andreas Krebbel <krebbel@linux.vnet.ibm.com> > > * builtins.c (expand_builtin_strcpy): Invoke > maybe_warn_nonstring_arg. > (expand_builtin_stpcpy): Likewise. Don't you then warn twice if builtin implementations for strcpy and stpcpy aren't available or can't be used, once here and once in calls.c? > --- a/gcc/builtins.c > +++ b/gcc/builtins.c > @@ -3770,6 +3770,12 @@ expand_builtin_strcpy (tree exp, rtx target) > tree dest = CALL_EXPR_ARG (exp, 0); > tree src = CALL_EXPR_ARG (exp, 1); > > + /* Check to see if the argument was declared attribute nonstring > + and if so, issue a warning since at this point it's not known > + to be nul-terminated. */ > + tree fndecl = get_callee_fndecl (exp); > + maybe_warn_nonstring_arg (fndecl, exp); > + > if (warn_stringop_overflow) > { > tree destsize = compute_objsize (dest, warn_stringop_overflow - 1); > @@ -3828,6 +3834,12 @@ expand_builtin_stpcpy (tree exp, rtx target, machine_mode mode) > tree len, lenp1; > rtx ret; > > + /* Check to see if the argument was declared attribute nonstring > + and if so, issue a warning since at this point it's not known > + to be nul-terminated. */ > + tree fndecl = get_callee_fndecl (exp); > + maybe_warn_nonstring_arg (fndecl, exp); > + > /* Ensure we get an actual string whose length can be evaluated at > compile-time, not an expression containing a string. This is > because the latter will potentially produce pessimized code > -- > 2.9.1 Jakub
On 04/11/2018 10:02 AM, Jakub Jelinek wrote: > On Wed, Apr 11, 2018 at 09:48:05AM +0200, Andreas Krebbel wrote: >> c-c++-common/attr-nonstring-3.c fails on IBM Z. The reason appears to be >> that we provide builtin implementations for strcpy and stpcpy. The >> warnings currently will only be emitted when expanding these as normal >> calls. >> >> Bootstrapped and regression tested on x86_64 and s390x. >> >> Ok? >> >> gcc/ChangeLog: >> >> 2018-04-11 Andreas Krebbel <krebbel@linux.vnet.ibm.com> >> >> * builtins.c (expand_builtin_strcpy): Invoke >> maybe_warn_nonstring_arg. >> (expand_builtin_stpcpy): Likewise. > > Don't you then warn twice if builtin implementations for strcpy and stpcpy > aren't available or can't be used, once here and once in calls.c? Looks like this could happen if the expander is present but rejects expansion. I basically copied this from the strcmp builtin which looks like possibly running into the same problem: /* Check to see if the argument was declared attribute nonstring and if so, issue a warning since at this point it's not known to be nul-terminated. */ tree fndecl = get_callee_fndecl (exp); maybe_warn_nonstring_arg (fndecl, exp); if (result) { /* Return the value in the proper mode for this function. */ machine_mode mode = TYPE_MODE (TREE_TYPE (exp)); if (GET_MODE (result) == mode) return result; if (target == 0) return convert_to_mode (mode, result, 0); convert_move (target, result, 0); return target; } /* Expand the library call ourselves using a stabilized argument list to avoid re-evaluating the function's arguments twice. */ tree fn = build_call_nofold_loc (EXPR_LOCATION (exp), fndecl, 2, arg1, arg2); gcc_assert (TREE_CODE (fn) == CALL_EXPR); CALL_EXPR_TAILCALL (fn) = CALL_EXPR_TAILCALL (exp); return expand_call (fn, target, target == const0_rtx); -Andreas-
On 04/11/2018 06:47 AM, Andreas Krebbel wrote: > On 04/11/2018 10:02 AM, Jakub Jelinek wrote: >> On Wed, Apr 11, 2018 at 09:48:05AM +0200, Andreas Krebbel wrote: >>> c-c++-common/attr-nonstring-3.c fails on IBM Z. The reason appears to be >>> that we provide builtin implementations for strcpy and stpcpy. The >>> warnings currently will only be emitted when expanding these as normal >>> calls. >>> >>> Bootstrapped and regression tested on x86_64 and s390x. >>> >>> Ok? >>> >>> gcc/ChangeLog: >>> >>> 2018-04-11 Andreas Krebbel <krebbel@linux.vnet.ibm.com> >>> >>> * builtins.c (expand_builtin_strcpy): Invoke >>> maybe_warn_nonstring_arg. >>> (expand_builtin_stpcpy): Likewise. >> >> Don't you then warn twice if builtin implementations for strcpy and stpcpy >> aren't available or can't be used, once here and once in calls.c? > > Looks like this could happen if the expander is present but rejects expansion. I basically copied > this from the strcmp builtin which looks like possibly running into the same problem: I tried to avoid the problem in the other instances of the call to maybe_warn_nonstring_arg (e.g., expand_builtin_strlen or expand_builtin_strcmp). I don't know if the expander can fail after the maybe_warn_nonstring_arg() call and so I have no tests for it. In your patch the expander failing seems more likely than in the others (in fact, on x86_64 it always fails because the call to targetm.have_movstr () in expand_movstr() returns false). That said, I see two warnings for a call to strcmp() with a nonstring argument even without the expander failing, so what I did isn't quite right either. I opened bug 85359 for it. Martin > > /* Check to see if the argument was declared attribute nonstring > and if so, issue a warning since at this point it's not known > to be nul-terminated. */ > tree fndecl = get_callee_fndecl (exp); > maybe_warn_nonstring_arg (fndecl, exp); > > if (result) > { > /* Return the value in the proper mode for this function. */ > machine_mode mode = TYPE_MODE (TREE_TYPE (exp)); > if (GET_MODE (result) == mode) > return result; > if (target == 0) > return convert_to_mode (mode, result, 0); > convert_move (target, result, 0); > return target; > } > > /* Expand the library call ourselves using a stabilized argument > list to avoid re-evaluating the function's arguments twice. */ > tree fn = build_call_nofold_loc (EXPR_LOCATION (exp), fndecl, 2, arg1, arg2); > gcc_assert (TREE_CODE (fn) == CALL_EXPR); > CALL_EXPR_TAILCALL (fn) = CALL_EXPR_TAILCALL (exp); > return expand_call (fn, target, target == const0_rtx); > > -Andreas- >
On 04/11/2018 11:20 PM, Martin Sebor wrote: > On 04/11/2018 06:47 AM, Andreas Krebbel wrote: >> On 04/11/2018 10:02 AM, Jakub Jelinek wrote: >>> On Wed, Apr 11, 2018 at 09:48:05AM +0200, Andreas Krebbel wrote: >>>> c-c++-common/attr-nonstring-3.c fails on IBM Z. The reason appears to be >>>> that we provide builtin implementations for strcpy and stpcpy. The >>>> warnings currently will only be emitted when expanding these as normal >>>> calls. >>>> >>>> Bootstrapped and regression tested on x86_64 and s390x. >>>> >>>> Ok? >>>> >>>> gcc/ChangeLog: >>>> >>>> 2018-04-11 Andreas Krebbel <krebbel@linux.vnet.ibm.com> >>>> >>>> * builtins.c (expand_builtin_strcpy): Invoke >>>> maybe_warn_nonstring_arg. >>>> (expand_builtin_stpcpy): Likewise. >>> >>> Don't you then warn twice if builtin implementations for strcpy and stpcpy >>> aren't available or can't be used, once here and once in calls.c? >> >> Looks like this could happen if the expander is present but rejects expansion. I basically copied >> this from the strcmp builtin which looks like possibly running into the same problem: > > I tried to avoid the problem in the other instances of the call > to maybe_warn_nonstring_arg (e.g., expand_builtin_strlen or > expand_builtin_strcmp). I don't know if the expander can fail > after the maybe_warn_nonstring_arg() call and so I have no > tests for it. > > In your patch the expander failing seems more likely than in > the others (in fact, on x86_64 it always fails because the call > to targetm.have_movstr () in expand_movstr() returns false). > > That said, I see two warnings for a call to strcmp() with > a nonstring argument even without the expander failing, so > what I did isn't quite right either. I opened bug 85359 for > it. I've opened BZ85369 for the strcpy / stpcpy issue. -Andreas-
diff --git a/gcc/builtins.c b/gcc/builtins.c index ababee5..83bbb70 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3770,6 +3770,12 @@ expand_builtin_strcpy (tree exp, rtx target) tree dest = CALL_EXPR_ARG (exp, 0); tree src = CALL_EXPR_ARG (exp, 1); + /* Check to see if the argument was declared attribute nonstring + and if so, issue a warning since at this point it's not known + to be nul-terminated. */ + tree fndecl = get_callee_fndecl (exp); + maybe_warn_nonstring_arg (fndecl, exp); + if (warn_stringop_overflow) { tree destsize = compute_objsize (dest, warn_stringop_overflow - 1); @@ -3828,6 +3834,12 @@ expand_builtin_stpcpy (tree exp, rtx target, machine_mode mode) tree len, lenp1; rtx ret; + /* Check to see if the argument was declared attribute nonstring + and if so, issue a warning since at this point it's not known + to be nul-terminated. */ + tree fndecl = get_callee_fndecl (exp); + maybe_warn_nonstring_arg (fndecl, exp); + /* Ensure we get an actual string whose length can be evaluated at compile-time, not an expression containing a string. This is because the latter will potentially produce pessimized code