Fix PR78154

Fix PR78154

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

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 ?

Thanks,
Prathamesh

[-- Attachment #2: pr78154-1.txt --]
[-- Type: text/plain, Size: 2606 bytes --]

2016-11-17  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	* tree-vrp.c (gimple_str_nonzero_warnv_p): New function.
	(gimple_stmt_nonzero_warnv_p): Call gimple_str_nonzero_warnv_p.

testsuite/
	* gcc.dg/tree-ssa/pr78154.c: New test-case.

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
new file mode 100644
index 0000000..d3463f4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp-slim" } */
+
+void f (void *d, const void *s, __SIZE_TYPE__ n)
+{
+  if (__builtin_memcpy (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_memmove (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_memset (d, 0, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strcpy (d, s) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strcat (d, s) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strncpy (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strncat (d, s, n) == 0)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_abort" "evrp" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index c2a4133..b563a7f 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1069,6 +1069,34 @@ gimple_assign_nonzero_warnv_p (gimple *stmt, bool *strict_overflow_p)
     }
 }
 
+/* Return true if STMT is known to contain call to a string-builtin function
+   that is known to return nonnull.  */
+
+static bool
+gimple_str_nonzero_warnv_p (gimple *stmt)
+{
+  if (!is_gimple_call (stmt))
+    return false;
+
+  tree fndecl = gimple_call_fndecl (stmt);
+  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return false;
+
+  switch (DECL_FUNCTION_CODE (fndecl))
+    {
+      case BUILT_IN_MEMMOVE:
+      case BUILT_IN_MEMCPY:
+      case BUILT_IN_MEMSET:
+      case BUILT_IN_STRCPY:
+      case BUILT_IN_STRNCPY:
+      case BUILT_IN_STRCAT:
+      case BUILT_IN_STRNCAT:
+	return true;
+      default:
+	return false;
+    }
+}
+
 /* Return true if STMT is known to compute a non-zero value.
    If the return value is based on the assumption that signed overflow is
    undefined, set *STRICT_OVERFLOW_P to true; otherwise, don't change
@@ -1097,7 +1125,7 @@ gimple_stmt_nonzero_warnv_p (gimple *stmt, bool *strict_overflow_p)
 	    lookup_attribute ("returns_nonnull",
 			      TYPE_ATTRIBUTES (gimple_call_fntype (stmt))))
 	  return true;
-	return gimple_alloca_call_p (stmt);
+	return gimple_alloca_call_p (stmt) || gimple_str_nonzero_warnv_p (stmt);
       }
     default:
       gcc_unreachable ();

Re: Fix PR78154

[Jakub Jelinek]

On Thu, Nov 17, 2016 at 12:19:37AM +0530, Prathamesh Kulkarni wrote:
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -1069,6 +1069,34 @@ gimple_assign_nonzero_warnv_p (gimple *stmt, bool *strict_overflow_p)
>      }
>  }
>  
> +/* Return true if STMT is known to contain call to a string-builtin function
> +   that is known to return nonnull.  */
> +
> +static bool
> +gimple_str_nonzero_warnv_p (gimple *stmt)
> +{
> +  if (!is_gimple_call (stmt))
> +    return false;

Shouldn't this be:
  if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
    return false;

> +
> +  tree fndecl = gimple_call_fndecl (stmt);

> +  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
> +    return false;

And drop the above 2 lines?

That we you also verify the arguments for sanity.

Otherwise I'll defer to Richard.

	Jakub

Re: Fix PR78154

[Martin Sebor]

On 11/16/2016 11:49 AM, 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.

Nice.  I think the list should also include mempcpy, stpcpy, and
stpncpy, and probably also the corresponding checking built-ins
such as __builtin___memcpy_chk.

FWIW, a more general solution to consider (possibly for GCC 8)
might be to extend attribute nonnull to apply to a functions return
value as well (e.g., use zero as the index for that), to indicate
that a pointer returned from it is not null.  That would let library
implementers annotate other functions (such as strerror)

Martin

Re: Fix PR78154

[Marc Glisse]

On Wed, 16 Nov 2016, Martin Sebor wrote:

> On 11/16/2016 11:49 AM, 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.
>
> Nice.  I think the list should also include mempcpy, stpcpy, and
> stpncpy, and probably also the corresponding checking built-ins
> such as __builtin___memcpy_chk.
>
> FWIW, a more general solution to consider (possibly for GCC 8)
> might be to extend attribute nonnull to apply to a functions return
> value as well (e.g., use zero as the index for that), to indicate
> that a pointer returned from it is not null.  That would let library
> implementers annotate other functions (such as strerror)

We already have that, under the name returns_nonnull. IIRC, people found a 
new name clearer than using position 0, when I posted the patch. Note also 
that memcpy already has both an attribute that says that it returns its 
first argument, and an attribute that says that said first argument is 
nonnull.

(I've heard some noise in C++-land about making memcpy(0,0,0) valid, but 
that may have just been noise)

-- 
Marc Glisse

Re: Fix PR78154

[Martin Sebor]

On 11/16/2016 02:21 PM, Marc Glisse wrote:
> On Wed, 16 Nov 2016, Martin Sebor wrote:
>
>> On 11/16/2016 11:49 AM, 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.
>>
>> Nice.  I think the list should also include mempcpy, stpcpy, and
>> stpncpy, and probably also the corresponding checking built-ins
>> such as __builtin___memcpy_chk.
>>
>> FWIW, a more general solution to consider (possibly for GCC 8)
>> might be to extend attribute nonnull to apply to a functions return
>> value as well (e.g., use zero as the index for that), to indicate
>> that a pointer returned from it is not null.  That would let library
>> implementers annotate other functions (such as strerror)
>
> We already have that, under the name returns_nonnull. IIRC, people found
> a new name clearer than using position 0, when I posted the patch. Note
> also that memcpy already has both an attribute that says that it returns
> its first argument, and an attribute that says that said first argument
> is nonnull.

Ah, right!  Thanks for the reminder!

__builtin_memcpy and __builtin_strcpy are both declared with
the same attribute list ATTR_RET1_NOTHROW_NONNULL_LEAF (as are
their checking versions) and that's defined like so:

   DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC,
     ATTR_LIST_STR1, ATTR_NOTHROW_NONNULL_LEAF)

ATTR_NOTHROW_NONNULL_LEAF doesn't include returns_nonull and neither
does ATTR_FNSPEC ATTR_LIST_STR1 (unless it's meant to imply that).

In any event, lookup_attribute ("returns_nonnull") fails for both
of these functions so I think the fix might be as "simple" as adding
the attribute.  Alternatively, if attribute fn spec str1 should imply
returns_nonnull when nonnull is set because it says (IIUC) that
the function returns the first (non-null) argument, the changes will
be a bit more involved and require adjusting other places besides
VRP (I see it used in fold-const.c as well similarly as in VRP).

(FWIW, I quoted "simple" above because it recently took me the better
part of an afternoon to figure out how to add attribute alloc_size to
malloc.)

>
> (I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
> that may have just been noise)

We may have read the same discussion.  It would make some things
a little easier in C++ (and remove what most people view as yet
another unnecessary gotcha in the language).

Martin

Re: Fix PR78154

[Martin Sebor]

On 11/16/2016 05:17 PM, Martin Sebor wrote:
> On 11/16/2016 02:21 PM, Marc Glisse wrote:
>> On Wed, 16 Nov 2016, Martin Sebor wrote:
>>
>>> On 11/16/2016 11:49 AM, 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.
>>>
>>> Nice.  I think the list should also include mempcpy, stpcpy, and
>>> stpncpy, and probably also the corresponding checking built-ins
>>> such as __builtin___memcpy_chk.
>>>
>>> FWIW, a more general solution to consider (possibly for GCC 8)
>>> might be to extend attribute nonnull to apply to a functions return
>>> value as well (e.g., use zero as the index for that), to indicate
>>> that a pointer returned from it is not null.  That would let library
>>> implementers annotate other functions (such as strerror)
>>
>> We already have that, under the name returns_nonnull. IIRC, people found
>> a new name clearer than using position 0, when I posted the patch. Note
>> also that memcpy already has both an attribute that says that it returns
>> its first argument, and an attribute that says that said first argument
>> is nonnull.
>
> Ah, right!  Thanks for the reminder!
>
> __builtin_memcpy and __builtin_strcpy are both declared with
> the same attribute list ATTR_RET1_NOTHROW_NONNULL_LEAF (as are
> their checking versions) and that's defined like so:
>
>   DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC,
>     ATTR_LIST_STR1, ATTR_NOTHROW_NONNULL_LEAF)
>
> ATTR_NOTHROW_NONNULL_LEAF doesn't include returns_nonull and neither
> does ATTR_FNSPEC ATTR_LIST_STR1 (unless it's meant to imply that).
>
> In any event, lookup_attribute ("returns_nonnull") fails for both
> of these functions so I think the fix might be as "simple" as adding
> the attribute.  Alternatively, if attribute fn spec str1 should imply
> returns_nonnull when nonnull is set because it says (IIUC) that
> the function returns the first (non-null) argument, the changes will
> be a bit more involved and require adjusting other places besides
> VRP (I see it used in fold-const.c as well similarly as in VRP).

I should have mentioned: when fully implemented, the test case
will pass even without VRP or without optimization.  A test for
the VRP bits will need to save the return value in a variable
and then use it (otherwise the check for memcpy(...) == 0 will
have already been optimized away by fold-const.c.

>
> (FWIW, I quoted "simple" above because it recently took me the better
> part of an afternoon to figure out how to add attribute alloc_size to
> malloc.)
>
>>
>> (I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
>> that may have just been noise)
>
> We may have read the same discussion.  It would make some things
> a little easier in C++ (and remove what most people view as yet
> another unnecessary gotcha in the language).
>
> Martin

Re: Fix PR78154

[Jeff Law]

On 11/16/2016 05:17 PM, Martin Sebor wrote:

>>
>> (I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
>> that may have just been noise)
>
> We may have read the same discussion.  It would make some things
> a little easier in C++ (and remove what most people view as yet
> another unnecessary gotcha in the language).
And that may be a reasonable thing to do.

While GCC does take advantage of the non-null attribute when trying to 
prove certain pointers must be non-null, it only does so when the magic 
flag is turned on.  There was a sense that it was too aggressive and 
that time may be necessary for folks to come to terms with what GCC was 
doing, particularly in the the memcpy (*, *, 0) case -- but I've never 
gotten the sense that happened and we've never turned that flag on by 
default.

jeff

Re: Fix PR78154

[Richard Biener]

On Wed, 16 Nov 2016, Jeff Law wrote:

> On 11/16/2016 05:17 PM, Martin Sebor wrote:
> 
> > > 
> > > (I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
> > > that may have just been noise)
> > 
> > We may have read the same discussion.  It would make some things
> > a little easier in C++ (and remove what most people view as yet
> > another unnecessary gotcha in the language).
> And that may be a reasonable thing to do.
> 
> While GCC does take advantage of the non-null attribute when trying to prove
> certain pointers must be non-null, it only does so when the magic flag is
> turned on.  There was a sense that it was too aggressive and that time may be
> necessary for folks to come to terms with what GCC was doing, particularly in
> the the memcpy (*, *, 0) case -- but I've never gotten the sense that happened
> and we've never turned that flag on by default.

We only have -f[no-]delete-null-pointer-checks and that's on by default.

So we _do_ take advantage of those.

Richard.

Re: Fix PR78154

[Richard Biener]

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.

Thanks,
Richard.

Re: Fix PR78154

[Prathamesh Kulkarni]

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 ?

Thanks,
Prathamesh
>
> Thanks,
> Richard.

Re: Fix PR78154

[Richard Biener]

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.

Richard.

Re: Fix PR78154

[Jeff Law]

On 11/17/2016 01:48 AM, Richard Biener wrote:
> On Wed, 16 Nov 2016, Jeff Law wrote:
>
>> On 11/16/2016 05:17 PM, Martin Sebor wrote:
>>
>>>>
>>>> (I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
>>>> that may have just been noise)
>>>
>>> We may have read the same discussion.  It would make some things
>>> a little easier in C++ (and remove what most people view as yet
>>> another unnecessary gotcha in the language).
>> And that may be a reasonable thing to do.
>>
>> While GCC does take advantage of the non-null attribute when trying to prove
>> certain pointers must be non-null, it only does so when the magic flag is
>> turned on.  There was a sense that it was too aggressive and that time may be
>> necessary for folks to come to terms with what GCC was doing, particularly in
>> the the memcpy (*, *, 0) case -- but I've never gotten the sense that happened
>> and we've never turned that flag on by default.
>
> We only have -f[no-]delete-null-pointer-checks and that's on by default.
>
> So we _do_ take advantage of those.
Hmm, maybe it's the path isolation I'm thinking about where we have the 
additional flag to control whether or not we use the attributes to help 
identify erroneous paths.

jeff

Re: Fix PR78154

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

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 ?

Thanks,
Prathamesh
>
> Richard.

[-- Attachment #2: pr78154-2.diff --]
[-- Type: text/plain, Size: 9958 bytes --]

diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 8dc59c9..da82da5 100644
--- a/gcc/builtin-attrs.def
+++ b/gcc/builtin-attrs.def
@@ -108,6 +108,7 @@ DEF_ATTR_IDENT (ATTR_TYPEGENERIC, "type generic")
 DEF_ATTR_IDENT (ATTR_TM_REGPARM, "*tm regparm")
 DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure")
 DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice")
+DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull")
 
 DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL)
 
@@ -195,8 +196,11 @@ DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL, ATTR_CONST, ATTR_NULL, \
 			ATTR_NOTHROW_NONNULL)
 /* Nothrow leaf functions whose pointer parameter(s) are all nonnull,
    and which return their first argument.  */
-DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC, ATTR_LIST_STR1, \
+DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF_1, ATTR_RETURNS_NONNULL, ATTR_NULL, \
 			ATTR_NOTHROW_NONNULL_LEAF)
+DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC, ATTR_LIST_STR1, \
+			ATTR_RET1_NOTHROW_NONNULL_LEAF_1)
+
 /* Nothrow const leaf functions whose pointer parameter(s) are all nonnull.  */
 DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL_LEAF, ATTR_CONST, ATTR_NULL, \
 			ATTR_NOTHROW_NONNULL_LEAF)
diff --git a/gcc/builtins.def b/gcc/builtins.def
index 219feeb..c697b0a 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -646,13 +646,13 @@ DEF_LIB_BUILTIN        (BUILT_IN_MEMCHR, "memchr", BT_FN_PTR_CONST_PTR_INT_SIZE,
 DEF_LIB_BUILTIN        (BUILT_IN_MEMCMP, "memcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMCPY, "memcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMMOVE, "memmove", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMSET, "memset", BT_FN_PTR_PTR_INT_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_RINDEX, "rindex", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRCASECMP, "strcasecmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCHR, "strchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRCMP, "strcmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCPY, "strcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
@@ -661,9 +661,9 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRDUP, "strdup", BT_FN_STRING_CONST_STRING, AT
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNDUP, "strndup", BT_FN_STRING_CONST_STRING_SIZE, ATTR_MALLOC_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRLEN, "strlen", BT_FN_SIZE_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCASECMP, "strncasecmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRNCMP, "strncmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRPBRK, "strpbrk", BT_FN_STRING_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRRCHR, "strrchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRSPN, "strspn", BT_FN_SIZE_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
@@ -904,14 +904,14 @@ DEF_BUILTIN_STUB (BUILT_IN_MEMCMP_EQ, "__builtin_memcmp_eq")
 DEF_GCC_BUILTIN	       (BUILT_IN_OBJECT_SIZE, "object_size", BT_FN_SIZE_CONST_PTR_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY_CHK, "__memcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE_CHK, "__memmove_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SNPRINTF_CHK, "__snprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_5_6)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SPRINTF_CHK, "__sprintf_chk", BT_FN_INT_STRING_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_4_5)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_VSNPRINTF_CHK, "__vsnprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_NOTHROW_5_0)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78154-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78154-1.c
new file mode 100644
index 0000000..fc51da0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78154-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-ssa" } */
+
+void f (void *d, const void *s, __SIZE_TYPE__ n)
+{
+  if (__builtin_memcpy (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_memmove (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_memset (d, 0, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strcpy (d, s) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strcat (d, s) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strncpy (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_strncat (d, s, n) == 0)
+    __builtin_abort ();
+
+  if (__builtin_stpcpy (d, s) == 0)
+    __builtin_abort ();
+
+  if (__builtin_stpncpy (d, s, n) == 0)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_abort" "ssa" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78154-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78154-2.c
new file mode 100644
index 0000000..55972a2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78154-2.c
@@ -0,0 +1,39 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp-slim" } */
+
+void f(void *d, const void *s, __SIZE_TYPE__ n)
+{
+  void *t1 = __builtin_memcpy (d, s, n);
+  if (t1 == 0)
+    __builtin_abort ();
+
+  void *t2 = __builtin_memmove (d, s, n);
+  if (t2 == 0)
+    __builtin_abort ();
+
+  void *t3 = __builtin_memset (d, 0, n);
+  if (t3 == 0)
+    __builtin_abort ();
+
+  void *t4 = __builtin_strcpy (d, s);
+  if (t4 == 0)
+    __builtin_abort ();
+
+  void *t5 = __builtin_strcat (d, s);
+  if (t5 == 0)
+    __builtin_abort ();
+
+  void *t6 = __builtin_strncat (d, s, n);
+  if (t6 == 0)
+    __builtin_abort ();
+
+  void *t7 = __builtin_stpcpy (d, s);
+  if (t7 == 0)
+    __builtin_abort ();
+
+  void *t8 = __builtin_stpncpy (d, s, n);
+  if (t8 == 0)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_abort" "evrp" } } */

Re: Fix PR78154

[Richard Biener]

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

Richard.

> Thanks,
> Prathamesh
> >
> > Richard.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: Fix PR78154

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]

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.

Thanks,
Prathamesh
> Richard.
>
>> Thanks,
>> Prathamesh
>> >
>> > Richard.
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

[-- Attachment #2: pr78154-5.txt --]
[-- Type: text/plain, Size: 10702 bytes --]

2016-11-22  Richard Biener  <rguenther@suse.de>
	    Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.rog>

	* tree-vrp.c (gimple_stmt_nonzero_warnv_p): Return true if function
	returns it's argument and the argument is nonnull.
	* builtin-attrs.def: Define ATTR_RETURNS_NONNULL,
	ATT_RETNONNULL_NOTHROW_LEAF.
	* builtins.def (BUILT_IN_MEMPCPY): Change attribute to
	ATTR_RETNONNULL_NOTHROW_LEAF.
	(BUILT_IN_STPCPY): Likewise.
	(BUILT_IN_STPNCPY): Likewise.
	(BUILT_IN_MEMPCPY_CHK): Likewise.
	(BUILT_IN_STPCPY_CHK): Likewise.
	(BUILT_IN_STPNCPY_CHK): Likewise.
	(BUILT_IN_STRCAT): Change attribute to ATTR_RET1_NOTHROW_NONNULL_LEAF.
	(BUILT_IN_STRNCAT): Likewise.
	(BUILT_IN_STRNCPY): Likewise.
	(BUILT_IN_MEMSET_CHK): Likewise.
	(BUILT_IN_STRCAT_CHK): Likewise.
	(BUILT_IN_STRCPY_CHK): Likewise.
	(BUILT_IN_STRNCAT_CHK): Likewise.
	(BUILT_IN_STRNCPY_CHK): Likewise.

testsuite/
	* gcc.dg/tree-ssa/pr78154.c: New test.
diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 8dc59c9..94d0c62 100644
--- a/gcc/builtin-attrs.def
+++ b/gcc/builtin-attrs.def
@@ -108,6 +108,7 @@ DEF_ATTR_IDENT (ATTR_TYPEGENERIC, "type generic")
 DEF_ATTR_IDENT (ATTR_TM_REGPARM, "*tm regparm")
 DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure")
 DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice")
+DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull")
 
 DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL)
 
@@ -197,6 +198,9 @@ DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL, ATTR_CONST, ATTR_NULL, \
    and which return their first argument.  */
 DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC, ATTR_LIST_STR1, \
 			ATTR_NOTHROW_NONNULL_LEAF)
+/* Nothrow leaf functions whose return value is nonnull.  */
+DEF_ATTR_TREE_LIST (ATTR_RETNONNULL_NOTHROW_LEAF, ATTR_RETURNS_NONNULL, ATTR_NULL, \
+			ATTR_NOTHROW_LEAF_LIST)
 /* Nothrow const leaf functions whose pointer parameter(s) are all nonnull.  */
 DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL_LEAF, ATTR_CONST, ATTR_NULL, \
 			ATTR_NOTHROW_NONNULL_LEAF)

diff --git a/gcc/builtins.def b/gcc/builtins.def
index 219feeb..82c987d 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -646,13 +646,13 @@ DEF_LIB_BUILTIN        (BUILT_IN_MEMCHR, "memchr", BT_FN_PTR_CONST_PTR_INT_SIZE,
 DEF_LIB_BUILTIN        (BUILT_IN_MEMCMP, "memcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMCPY, "memcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMMOVE, "memmove", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMSET, "memset", BT_FN_PTR_PTR_INT_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_RINDEX, "rindex", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRCASECMP, "strcasecmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCHR, "strchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRCMP, "strcmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCPY, "strcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
@@ -661,9 +661,9 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRDUP, "strdup", BT_FN_STRING_CONST_STRING, AT
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNDUP, "strndup", BT_FN_STRING_CONST_STRING_SIZE, ATTR_MALLOC_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRLEN, "strlen", BT_FN_SIZE_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCASECMP, "strncasecmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRNCMP, "strncmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRPBRK, "strpbrk", BT_FN_STRING_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRRCHR, "strrchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRSPN, "strspn", BT_FN_SIZE_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
@@ -904,14 +904,14 @@ DEF_BUILTIN_STUB (BUILT_IN_MEMCMP_EQ, "__builtin_memcmp_eq")
 DEF_GCC_BUILTIN	       (BUILT_IN_OBJECT_SIZE, "object_size", BT_FN_SIZE_CONST_PTR_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY_CHK, "__memcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE_CHK, "__memmove_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SNPRINTF_CHK, "__snprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_5_6)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SPRINTF_CHK, "__sprintf_chk", BT_FN_INT_STRING_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_4_5)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_VSNPRINTF_CHK, "__vsnprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_NOTHROW_5_0)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
new file mode 100644
index 0000000..d908a39
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp-slim" } */
+
+void f(void *d, const void *s, __SIZE_TYPE__ n)
+{
+  void *t1 = __builtin_memcpy (d, s, n);
+  if (t1 == 0)
+    __builtin_abort ();
+
+  void *t2 = __builtin_memmove (d, s, n);
+  if (t2 == 0)
+    __builtin_abort ();
+
+  void *t3 = __builtin_memset (d, 0, n);
+  if (t3 == 0)
+    __builtin_abort ();
+
+  void *t4 = __builtin_strcpy (d, s);
+  if (t4 == 0)
+    __builtin_abort ();
+
+  void *t5 = __builtin_strncpy (d, s, n);
+  if (t5 == 0)
+    __builtin_abort ();
+
+  void *t6 = __builtin_strcat (d, s);
+  if (t6 == 0)
+    __builtin_abort ();
+
+  void *t7 = __builtin_strncat (d, s, n);
+  if (t7 == 0)
+    __builtin_abort ();
+
+  void *t8 = __builtin_stpcpy (d, s);
+  if (t8 == 0)
+    __builtin_abort ();
+
+  void *t9 = __builtin_stpncpy (d, s, n);
+  if (t9 == 0)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_abort" "evrp" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index c2a4133..7d9bdf5 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1097,6 +1097,20 @@ gimple_stmt_nonzero_warnv_p (gimple *stmt, bool *strict_overflow_p)
 	    lookup_attribute ("returns_nonnull",
 			      TYPE_ATTRIBUTES (gimple_call_fntype (stmt))))
 	  return true;
+
+	unsigned rf = gimple_call_return_flags (as_a<gcall *> (stmt));
+	if (rf & ERF_RETURNS_ARG)
+	  {
+	    tree arg = gimple_call_arg (as_a<gcall *> (stmt), rf & ERF_RETURN_ARG_MASK);
+	    if (SSA_VAR_P (arg))
+	      {
+		value_range *vr = get_value_range (arg);
+		if ((vr && vr->type != VR_VARYING)
+		    || infer_nonnull_range_by_attribute (stmt, arg))
+		  return true;
+	      }
+	  }
+
 	return gimple_alloca_call_p (stmt);
       }
     default:

Re: Fix PR78154

[Richard Biener]

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.

Thanks,
Richard.

Re: Fix PR78154

[Prathamesh Kulkarni]

[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

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 ?

Thanks,
Prathamesh
>
> Thanks,
> Richard.

[-- Attachment #2: pr78154-7.txt --]
[-- Type: text/plain, Size: 10800 bytes --]

2016-11-23  Richard Biener  <rguenther@suse.de>
	    Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.rog>

	PR tree-optimization/78154
	* tree-vrp.c (gimple_stmt_nonzero_warnv_p): Return true if function
	returns it's argument and the argument is nonnull.
	* builtin-attrs.def: Define ATTR_RETURNS_NONNULL,
	ATT_RETNONNULL_NOTHROW_LEAF.
	* builtins.def (BUILT_IN_MEMPCPY): Change attribute to
	ATTR_RETNONNULL_NOTHROW_LEAF.
	(BUILT_IN_STPCPY): Likewise.
	(BUILT_IN_STPNCPY): Likewise.
	(BUILT_IN_MEMPCPY_CHK): Likewise.
	(BUILT_IN_STPCPY_CHK): Likewise.
	(BUILT_IN_STPNCPY_CHK): Likewise.
	(BUILT_IN_STRCAT): Change attribute to ATTR_RET1_NOTHROW_NONNULL_LEAF.
	(BUILT_IN_STRNCAT): Likewise.
	(BUILT_IN_STRNCPY): Likewise.
	(BUILT_IN_MEMSET_CHK): Likewise.
	(BUILT_IN_STRCAT_CHK): Likewise.
	(BUILT_IN_STRCPY_CHK): Likewise.
	(BUILT_IN_STRNCAT_CHK): Likewise.
	(BUILT_IN_STRNCPY_CHK): Likewise.

testsuite/
	* gcc.dg/tree-ssa/pr78154.c: New test.

diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 8dc59c9..88c9bd1 100644
--- a/gcc/builtin-attrs.def
+++ b/gcc/builtin-attrs.def
@@ -108,6 +108,7 @@ DEF_ATTR_IDENT (ATTR_TYPEGENERIC, "type generic")
 DEF_ATTR_IDENT (ATTR_TM_REGPARM, "*tm regparm")
 DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure")
 DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice")
+DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull")
 
 DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL)
 
@@ -197,6 +198,10 @@ DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL, ATTR_CONST, ATTR_NULL, \
    and which return their first argument.  */
 DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC, ATTR_LIST_STR1, \
 			ATTR_NOTHROW_NONNULL_LEAF)
+/* Nothrow leaf functions whose pointer parameter(s) are all nonnull,
+   and return value is also nonnull.  */
+DEF_ATTR_TREE_LIST (ATTR_RETNONNULL_NOTHROW_LEAF, ATTR_RETURNS_NONNULL, ATTR_NULL, \
+			ATTR_NOTHROW_NONNULL_LEAF)
 /* Nothrow const leaf functions whose pointer parameter(s) are all nonnull.  */
 DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL_LEAF, ATTR_CONST, ATTR_NULL, \
 			ATTR_NOTHROW_NONNULL_LEAF)
diff --git a/gcc/builtins.def b/gcc/builtins.def
index 219feeb..82c987d 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -646,13 +646,13 @@ DEF_LIB_BUILTIN        (BUILT_IN_MEMCHR, "memchr", BT_FN_PTR_CONST_PTR_INT_SIZE,
 DEF_LIB_BUILTIN        (BUILT_IN_MEMCMP, "memcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMCPY, "memcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMMOVE, "memmove", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_MEMSET, "memset", BT_FN_PTR_PTR_INT_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_RINDEX, "rindex", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRCASECMP, "strcasecmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCHR, "strchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRCMP, "strcmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRCPY, "strcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
@@ -661,9 +661,9 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRDUP, "strdup", BT_FN_STRING_CONST_STRING, AT
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNDUP, "strndup", BT_FN_STRING_CONST_STRING_SIZE, ATTR_MALLOC_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN_CHKP   (BUILT_IN_STRLEN, "strlen", BT_FN_SIZE_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCASECMP, "strncasecmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRNCMP, "strncmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN        (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRPBRK, "strpbrk", BT_FN_STRING_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRRCHR, "strrchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_STRSPN, "strspn", BT_FN_SIZE_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
@@ -904,14 +904,14 @@ DEF_BUILTIN_STUB (BUILT_IN_MEMCMP_EQ, "__builtin_memcmp_eq")
 DEF_GCC_BUILTIN	       (BUILT_IN_OBJECT_SIZE, "object_size", BT_FN_SIZE_CONST_PTR_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY_CHK, "__memcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE_CHK, "__memmove_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SNPRINTF_CHK, "__snprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_5_6)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_SPRINTF_CHK, "__sprintf_chk", BT_FN_INT_STRING_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_4_5)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_VSNPRINTF_CHK, "__vsnprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_NOTHROW_5_0)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
new file mode 100644
index 0000000..d908a39
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78154.c
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp-slim" } */
+
+void f(void *d, const void *s, __SIZE_TYPE__ n)
+{
+  void *t1 = __builtin_memcpy (d, s, n);
+  if (t1 == 0)
+    __builtin_abort ();
+
+  void *t2 = __builtin_memmove (d, s, n);
+  if (t2 == 0)
+    __builtin_abort ();
+
+  void *t3 = __builtin_memset (d, 0, n);
+  if (t3 == 0)
+    __builtin_abort ();
+
+  void *t4 = __builtin_strcpy (d, s);
+  if (t4 == 0)
+    __builtin_abort ();
+
+  void *t5 = __builtin_strncpy (d, s, n);
+  if (t5 == 0)
+    __builtin_abort ();
+
+  void *t6 = __builtin_strcat (d, s);
+  if (t6 == 0)
+    __builtin_abort ();
+
+  void *t7 = __builtin_strncat (d, s, n);
+  if (t7 == 0)
+    __builtin_abort ();
+
+  void *t8 = __builtin_stpcpy (d, s);
+  if (t8 == 0)
+    __builtin_abort ();
+
+  void *t9 = __builtin_stpncpy (d, s, n);
+  if (t9 == 0)
+    __builtin_abort ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_abort" "evrp" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index c2a4133..5bd4418 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1097,6 +1097,20 @@ gimple_stmt_nonzero_warnv_p (gimple *stmt, bool *strict_overflow_p)
 	    lookup_attribute ("returns_nonnull",
 			      TYPE_ATTRIBUTES (gimple_call_fntype (stmt))))
 	  return true;
+
+	gcall *call_stmt = as_a<gcall *> (stmt);
+	unsigned rf = gimple_call_return_flags (call_stmt);
+	if (rf & ERF_RETURNS_ARG)
+	  {
+	    unsigned argnum = rf & ERF_RETURN_ARG_MASK;
+	    if (argnum < gimple_call_num_args (call_stmt))
+	      {
+		tree arg = gimple_call_arg (call_stmt, argnum);
+		if (SSA_VAR_P (arg)
+		    && infer_nonnull_range_by_attribute (stmt, arg))
+		  return true;
+	      }
+	  }
 	return gimple_alloca_call_p (stmt);
       }
     default:

Re: Fix PR78154

[Richard Biener]

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)