diff mbox series

Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins.

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

Commit Message

Andreas Krebbel April 11, 2018, 7:48 a.m. UTC 
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.
---
 gcc/builtins.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jakub Jelinek April 11, 2018, 8:02 a.m. UTC | #1 
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
Andreas Krebbel April 11, 2018, 12:47 p.m. UTC | #2 
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-
Martin Sebor April 11, 2018, 9:20 p.m. UTC | #3 
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-
>
Andreas Krebbel April 12, 2018, 7:49 a.m. UTC | #4 
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 mbox series

Patch

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