* Fix PR78154 @ 2016-11-16 18:49 Prathamesh Kulkarni 2016-11-16 18:55 ` Jakub Jelinek ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Prathamesh Kulkarni @ 2016-11-16 18:49 UTC (permalink / raw) To: gcc Patches, Richard Biener, Martin Sebor [-- 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 (); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-16 18:49 Fix PR78154 Prathamesh Kulkarni @ 2016-11-16 18:55 ` Jakub Jelinek 2016-11-16 20:27 ` Martin Sebor 2016-11-17 8:51 ` Richard Biener 2 siblings, 0 replies; 18+ messages in thread From: Jakub Jelinek @ 2016-11-16 18:55 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Richard Biener, Martin Sebor 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-16 18:49 Fix PR78154 Prathamesh Kulkarni 2016-11-16 18:55 ` Jakub Jelinek @ 2016-11-16 20:27 ` Martin Sebor 2016-11-16 21:21 ` Marc Glisse 2016-11-17 8:51 ` Richard Biener 2 siblings, 1 reply; 18+ messages in thread From: Martin Sebor @ 2016-11-16 20:27 UTC (permalink / raw) To: Prathamesh Kulkarni, gcc Patches, Richard Biener 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-16 20:27 ` Martin Sebor @ 2016-11-16 21:21 ` Marc Glisse 2016-11-17 0:17 ` Martin Sebor 0 siblings, 1 reply; 18+ messages in thread From: Marc Glisse @ 2016-11-16 21:21 UTC (permalink / raw) To: Martin Sebor; +Cc: Prathamesh Kulkarni, gcc Patches, Richard Biener 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-16 21:21 ` Marc Glisse @ 2016-11-17 0:17 ` Martin Sebor 2016-11-17 0:39 ` Martin Sebor 2016-11-17 4:57 ` Jeff Law 0 siblings, 2 replies; 18+ messages in thread From: Martin Sebor @ 2016-11-17 0:17 UTC (permalink / raw) To: gcc-patches; +Cc: Prathamesh Kulkarni, Richard Biener 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 0:17 ` Martin Sebor @ 2016-11-17 0:39 ` Martin Sebor 2016-11-17 4:57 ` Jeff Law 1 sibling, 0 replies; 18+ messages in thread From: Martin Sebor @ 2016-11-17 0:39 UTC (permalink / raw) To: gcc-patches; +Cc: Prathamesh Kulkarni, Richard Biener 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 0:17 ` Martin Sebor 2016-11-17 0:39 ` Martin Sebor @ 2016-11-17 4:57 ` Jeff Law 2016-11-17 8:48 ` Richard Biener 1 sibling, 1 reply; 18+ messages in thread From: Jeff Law @ 2016-11-17 4:57 UTC (permalink / raw) To: Martin Sebor, gcc-patches; +Cc: Prathamesh Kulkarni, Richard Biener 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 4:57 ` Jeff Law @ 2016-11-17 8:48 ` Richard Biener 2016-11-17 15:19 ` Jeff Law 0 siblings, 1 reply; 18+ messages in thread From: Richard Biener @ 2016-11-17 8:48 UTC (permalink / raw) To: Jeff Law; +Cc: Martin Sebor, gcc-patches, Prathamesh Kulkarni 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 8:48 ` Richard Biener @ 2016-11-17 15:19 ` Jeff Law 0 siblings, 0 replies; 18+ messages in thread From: Jeff Law @ 2016-11-17 15:19 UTC (permalink / raw) To: Richard Biener; +Cc: Martin Sebor, gcc-patches, Prathamesh Kulkarni 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-16 18:49 Fix PR78154 Prathamesh Kulkarni 2016-11-16 18:55 ` Jakub Jelinek 2016-11-16 20:27 ` Martin Sebor @ 2016-11-17 8:51 ` Richard Biener 2016-11-17 9:34 ` Prathamesh Kulkarni 2 siblings, 1 reply; 18+ messages in thread From: Richard Biener @ 2016-11-17 8:51 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Martin Sebor 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 8:51 ` Richard Biener @ 2016-11-17 9:34 ` Prathamesh Kulkarni 2016-11-17 9:54 ` Richard Biener 0 siblings, 1 reply; 18+ messages in thread From: Prathamesh Kulkarni @ 2016-11-17 9:34 UTC (permalink / raw) To: Richard Biener; +Cc: gcc Patches, Martin Sebor 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 9:34 ` Prathamesh Kulkarni @ 2016-11-17 9:54 ` Richard Biener 2016-11-18 13:03 ` Prathamesh Kulkarni 0 siblings, 1 reply; 18+ messages in thread From: Richard Biener @ 2016-11-17 9:54 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Martin Sebor 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-17 9:54 ` Richard Biener @ 2016-11-18 13:03 ` Prathamesh Kulkarni 2016-11-21 10:04 ` Richard Biener 0 siblings, 1 reply; 18+ messages in thread From: Prathamesh Kulkarni @ 2016-11-18 13:03 UTC (permalink / raw) To: Richard Biener; +Cc: gcc Patches, Martin Sebor [-- 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" } } */ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-18 13:03 ` Prathamesh Kulkarni @ 2016-11-21 10:04 ` Richard Biener 2016-11-22 10:14 ` Prathamesh Kulkarni 0 siblings, 1 reply; 18+ messages in thread From: Richard Biener @ 2016-11-21 10:04 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Martin Sebor 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) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-21 10:04 ` Richard Biener @ 2016-11-22 10:14 ` Prathamesh Kulkarni 2016-11-22 14:53 ` Richard Biener 0 siblings, 1 reply; 18+ messages in thread From: Prathamesh Kulkarni @ 2016-11-22 10:14 UTC (permalink / raw) To: Richard Biener; +Cc: gcc Patches, Martin Sebor [-- 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: ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-22 10:14 ` Prathamesh Kulkarni @ 2016-11-22 14:53 ` Richard Biener 2016-11-23 9:35 ` Prathamesh Kulkarni 0 siblings, 1 reply; 18+ messages in thread From: Richard Biener @ 2016-11-22 14:53 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Martin Sebor 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-22 14:53 ` Richard Biener @ 2016-11-23 9:35 ` Prathamesh Kulkarni 2016-11-23 9:42 ` Richard Biener 0 siblings, 1 reply; 18+ messages in thread From: Prathamesh Kulkarni @ 2016-11-23 9:35 UTC (permalink / raw) To: Richard Biener; +Cc: gcc Patches, Martin Sebor [-- 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: ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Fix PR78154 2016-11-23 9:35 ` Prathamesh Kulkarni @ 2016-11-23 9:42 ` Richard Biener 0 siblings, 0 replies; 18+ messages in thread From: Richard Biener @ 2016-11-23 9:42 UTC (permalink / raw) To: Prathamesh Kulkarni; +Cc: gcc Patches, Martin Sebor 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) ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-11-23 9:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-16 18:49 Fix PR78154 Prathamesh Kulkarni 2016-11-16 18:55 ` Jakub Jelinek 2016-11-16 20:27 ` Martin Sebor 2016-11-16 21:21 ` Marc Glisse 2016-11-17 0:17 ` Martin Sebor 2016-11-17 0:39 ` Martin Sebor 2016-11-17 4:57 ` Jeff Law 2016-11-17 8:48 ` Richard Biener 2016-11-17 15:19 ` Jeff Law 2016-11-17 8:51 ` Richard Biener 2016-11-17 9:34 ` Prathamesh Kulkarni 2016-11-17 9:54 ` Richard Biener 2016-11-18 13:03 ` Prathamesh Kulkarni 2016-11-21 10:04 ` Richard Biener 2016-11-22 10:14 ` Prathamesh Kulkarni 2016-11-22 14:53 ` Richard Biener 2016-11-23 9:35 ` Prathamesh Kulkarni 2016-11-23 9:42 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).