Re: [PATCH] Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins.

[Andreas Krebbel]

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  

* 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-

Re: [PATCH] Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins.

[Martin Sebor]

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  

* 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-

Re: [PATCH] Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins.

[Andreas Krebbel]

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  
>>
>>  * 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-

Re: [PATCH] Invoke maybe_warn_nonstring_arg for strcpy/stpcpy builtins.

[Jakub Jelinek]

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  
> 
>   * 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