| Message ID | f369c4e4-2c2b-4eae-3353-05cb7282af24@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 2016.12.15 at 19:27 -0700, Martin Sebor wrote: > On 12/14/2016 09:19 PM, Jeff Law wrote: > > On 12/14/2016 03:56 PM, Martin Sebor wrote: > > > The -Wnonnull warning improvement (PR c/17308 - nonnull attribute > > > not as useful as it could be) causes a couple of false positives > > > in a powerpc64le bootstrap. The attached fix suppresses them. > > > It passes bootstrap on powerpc64le but tests are still running. > > > > > > I couldn't reproduce the bootstrap comparison failure that Segher > > > mentioned in his comment on the bug. I've gone over the patch > > > again to see if I could spot what could possibly be behind it > > > but couldn't really see anything. Jeff, you mentioned attribute > > > nonnull is exploited by the null pointer optimization. Do you > > > think it could possibly have this effect in GCC? > > It's certainly possible. It would be an indicator that something in GCC > > is passing a NULL pointer into a routine where it shouldn't at runtime > > and that GCC is using its new knowledge to optimize the code assuming > > that can't happen. > > > > ie, it's an indicator of a latent bug in GCC. Depending on the > > difficulty in tracking down, you may need to revert the change. This is > > exactly the kind of scenario I was concerned about in that approval > > message. > > I couldn't reproduce the comparison error either on powerpc64-linux > or on powerpc64le-linux. > > > The change to the vec<T, va_heap, vl_ptr> is OK. Can you please verify > > that if you install just that change that ppc bootstraps are restored > > and if so, please install. > > Done. > > A profiledbootstrap exposed a few more instances of the enhanced > -Wnonnull warning. I spent some time analyzing them and decided > that three of them are justified (those in gengtype-parse.c and > gengtype.c) and the other three false positives. > > The attached patch silences them. > > The gengtype code alternates between checking for null pointers > and assuming that the same pointers are not null (and passing nulls > to nonnull functions like libiberty's lbasename). It could probably > benefit from some more cleanup but I'm out of cycles for that. > > There are two outstanding instances of -Wnonnull in the profiled- > bootstrap log that both point to the same function that I haven't > figured out yet: > > In function ‘void check_function_format(tree, int, tree_node**)’: > cc1plus: warning: argument 1 null where non-null expected [-Wnonnull] > /usr/include/string.h:398:15: note: in a call to function ‘size_t > strlen(const char*)’ declared here > > I'll deal with them next but I want to get this patch reviewed > and approved so bootstrap can be restored on the targets affected > by the vec.h warning. > > While waiting for builds to finish I also built Binutils, Busybox, > and the Linux kernel to see if the warning shows up there. It does > not. It does for me with an allmodconf. At -O2 I get three warnings, and at -O3 I get two additional warnings. Now these additional ones happen way too deep into the pipeline to be reliable. (For a reduced testcase see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8) When you as the author of the warning have difficulties in figuring out what causes these warnings, what about the average user? Therefore -fsanitize=undefined should be preferred. It gives you better info and a backtrace that makes diagnosis easy. So in my opinion -Wnonnull should warn only for trivial cases.
On 12/16/2016 01:13 AM, Markus Trippelsdorf wrote: > On 2016.12.15 at 19:27 -0700, Martin Sebor wrote: >> On 12/14/2016 09:19 PM, Jeff Law wrote: >>> On 12/14/2016 03:56 PM, Martin Sebor wrote: >>>> The -Wnonnull warning improvement (PR c/17308 - nonnull attribute >>>> not as useful as it could be) causes a couple of false positives >>>> in a powerpc64le bootstrap. The attached fix suppresses them. >>>> It passes bootstrap on powerpc64le but tests are still running. >>>> >>>> I couldn't reproduce the bootstrap comparison failure that Segher >>>> mentioned in his comment on the bug. I've gone over the patch >>>> again to see if I could spot what could possibly be behind it >>>> but couldn't really see anything. Jeff, you mentioned attribute >>>> nonnull is exploited by the null pointer optimization. Do you >>>> think it could possibly have this effect in GCC? >>> It's certainly possible. It would be an indicator that something in GCC >>> is passing a NULL pointer into a routine where it shouldn't at runtime >>> and that GCC is using its new knowledge to optimize the code assuming >>> that can't happen. >>> >>> ie, it's an indicator of a latent bug in GCC. Depending on the >>> difficulty in tracking down, you may need to revert the change. This is >>> exactly the kind of scenario I was concerned about in that approval >>> message. >> >> I couldn't reproduce the comparison error either on powerpc64-linux >> or on powerpc64le-linux. >> >>> The change to the vec<T, va_heap, vl_ptr> is OK. Can you please verify >>> that if you install just that change that ppc bootstraps are restored >>> and if so, please install. >> >> Done. >> >> A profiledbootstrap exposed a few more instances of the enhanced >> -Wnonnull warning. I spent some time analyzing them and decided >> that three of them are justified (those in gengtype-parse.c and >> gengtype.c) and the other three false positives. >> >> The attached patch silences them. >> >> The gengtype code alternates between checking for null pointers >> and assuming that the same pointers are not null (and passing nulls >> to nonnull functions like libiberty's lbasename). It could probably >> benefit from some more cleanup but I'm out of cycles for that. >> >> There are two outstanding instances of -Wnonnull in the profiled- >> bootstrap log that both point to the same function that I haven't >> figured out yet: >> >> In function ‘void check_function_format(tree, int, tree_node**)’: >> cc1plus: warning: argument 1 null where non-null expected [-Wnonnull] >> /usr/include/string.h:398:15: note: in a call to function ‘size_t >> strlen(const char*)’ declared here >> >> I'll deal with them next but I want to get this patch reviewed >> and approved so bootstrap can be restored on the targets affected >> by the vec.h warning. >> >> While waiting for builds to finish I also built Binutils, Busybox, >> and the Linux kernel to see if the warning shows up there. It does >> not. > > It does for me with an allmodconf. At -O2 I get three warnings, and at > -O3 I get two additional warnings. Now these additional ones happen way > too deep into the pipeline to be reliable. (For a reduced testcase see: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8) The warning looks quite justified in this case. The first call to sm_read_sector exits with the pointer being null and so the second call is diagnosed. That seems like a great example of when the warning is useful. > > When you as the author of the warning have difficulties in figuring out > what causes these warnings, what about the average user? I haven't encountered such difficulties. The instance I pointed to above in function check_function_format suggests there is a bug that prevents GCC from pointing to the line that triggers warning (it reads "cc1plus: warning: argument 1 null"). I just haven't had time to understand what causes it. > Therefore -fsanitize=undefined should be preferred. It gives you better > info and a backtrace that makes diagnosis easy. > > So in my opinion -Wnonnull should warn only for trivial cases. The sanitizer is a useful and more reliable tool but it has a number of limitations that put it off limits for many projects (runtime instrumentation and cost, may not be available for embedded systems, relies on running the code and exercising all paths to find bugs). Warnings have their own limitations (false positives and negatives) but they are useful as the first line of defense that doesn't suffer from the sanitizer limitations. They help catch mistakes before they get committed into the code base and affect other programmers. That's by far the cheapest way to find bugs. (Sorry if I'm stating the obvious.) The -Wnonnull implementation in GCC 6 and prior only warns on null pointer constants, like in memset(NULL, 0, n). It's exceedingly unlikely that someone will write such code by mistake, and so the warning is substantially ineffective. That's why several users over the last decade have requested it be enhanced. To be honest, I'm surprised that this is proving controversial. This is not something new. It's an improvement to an existing warning that people have asked for. It's bound to have some false positives but so far the rate is far lower than that of almost all the other warnings on the list I posted across the four projects I built. I don't claim it can't be improved but it seems pretty good as it is already. Among the 6 instances it's found in GCC three look like real bugs. Martin
On Fri, Dec 16, 2016 at 09:36:25AM -0700, Martin Sebor wrote: > > It does for me with an allmodconf. At -O2 I get three warnings, and at > > -O3 I get two additional warnings. Now these additional ones happen way > > too deep into the pipeline to be reliable. (For a reduced testcase see: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8) > > The warning looks quite justified in this case. The first call > to sm_read_sector exits with the pointer being null and so the > second call is diagnosed. That seems like a great example of > when the warning is useful. No. The first call to sm_read_sector just doesn't exit. So it is warning about dead code. > I don't claim it can't be improved but it seems pretty good as > it is already. Among the 6 instances it's found in GCC three > look like real bugs. None look like real bugs to me. Jakub
On Fri, 16 Dec 2016, Martin Sebor wrote: > I don't claim it can't be improved but it seems pretty good as > it is already. Among the 6 instances it's found in GCC three > look like real bugs. FWIW it's found at least one real bug in glibc <https://sourceware.org/bugzilla/show_bug.cgi?id=20978> - that's a case where strlen is called on a pointer that can never be non-null if the strlen call is reached. (I don't know if there are other cases in glibc, whether genuine bugs or false positives, that would appear later in the build once that bug is fixed.)
On Fri, Dec 16, 2016 at 07:00:06PM +0000, Joseph Myers wrote: > On Fri, 16 Dec 2016, Martin Sebor wrote: > > > I don't claim it can't be improved but it seems pretty good as > > it is already. Among the 6 instances it's found in GCC three > > look like real bugs. > > FWIW it's found at least one real bug in glibc > <https://sourceware.org/bugzilla/show_bug.cgi?id=20978> - that's a case > where strlen is called on a pointer that can never be non-null if the > strlen call is reached. (I don't know if there are other cases in glibc, > whether genuine bugs or false positives, that would appear later in the > build once that bug is fixed.) Thanks. Reduced to something like: int foo (const char *name) { if (name) return 6; return __builtin_strlen (name); } This is warned about both with Martin's late warning and my after ccp2 warning version. We should include it in gcc testsuite. Jakub
On 12/16/2016 12:08 PM, Jakub Jelinek wrote: > On Fri, Dec 16, 2016 at 07:00:06PM +0000, Joseph Myers wrote: >> On Fri, 16 Dec 2016, Martin Sebor wrote: >> >>> I don't claim it can't be improved but it seems pretty good as >>> it is already. Among the 6 instances it's found in GCC three >>> look like real bugs. >> >> FWIW it's found at least one real bug in glibc >> <https://sourceware.org/bugzilla/show_bug.cgi?id=20978> - that's a case >> where strlen is called on a pointer that can never be non-null if the >> strlen call is reached. (I don't know if there are other cases in glibc, >> whether genuine bugs or false positives, that would appear later in the >> build once that bug is fixed.) > > Thanks. Reduced to something like: > int > foo (const char *name) > { > if (name) > return 6; > return __builtin_strlen (name); > } > This is warned about both with Martin's late warning and my after ccp2 > warning version. We should include it in gcc testsuite. Agreed. jeff
On 12/16/2016 12:08 PM, Jakub Jelinek wrote: > On Fri, Dec 16, 2016 at 07:00:06PM +0000, Joseph Myers wrote: >> On Fri, 16 Dec 2016, Martin Sebor wrote: >> >>> I don't claim it can't be improved but it seems pretty good as >>> it is already. Among the 6 instances it's found in GCC three >>> look like real bugs. >> >> FWIW it's found at least one real bug in glibc >> <https://sourceware.org/bugzilla/show_bug.cgi?id=20978> - that's a case >> where strlen is called on a pointer that can never be non-null if the >> strlen call is reached. (I don't know if there are other cases in glibc, >> whether genuine bugs or false positives, that would appear later in the >> build once that bug is fixed.) > > Thanks. Reduced to something like: > int > foo (const char *name) > { > if (name) > return 6; > return __builtin_strlen (name); > } > This is warned about both with Martin's late warning and my after ccp2 > warning version. We should include it in gcc testsuite. I'll note this is an example of a case where Andrew's work would likely help because it allows us to ask for a range of name_XX at the return statement and it'll give us back a range that is constrained by the path(s) which reach the return statement. Contrast to the current VRP work where each SSA_NAME has a range, but that range must be valid for every context in which that SSA_NAME appears. jeff
On Fri, Dec 16, 2016 at 01:01:13PM -0700, Jeff Law wrote: > > Thanks. Reduced to something like: > > int > > foo (const char *name) > > { > > if (name) > > return 6; > > return __builtin_strlen (name); > > } > > This is warned about both with Martin's late warning and my after ccp2 > > warning version. We should include it in gcc testsuite. > I'll note this is an example of a case where Andrew's work would likely help > because it allows us to ask for a range of name_XX at the return statement > and it'll give us back a range that is constrained by the path(s) which > reach the return statement. > > Contrast to the current VRP work where each SSA_NAME has a range, but that > range must be valid for every context in which that SSA_NAME appears. Well, inside the current VRP it has separate ranges for the different paths and actually replaces the name in the strlen argument with NULL during evrp, so doesn't suffer from the current VRP limitations. Anyway, let's consider the warning from the linux kernel with the closedir, I guess it can be simplified to something along the lines of: void baz (char *) __attribute__((nonnull)); char *bar (int); int foo (void) { char *p = bar (1); int ret = 0; if (p == 0) { bar (2); bar (3); bar (4); ret = 1; goto out; } bar (5); bar (6); bar (7); bar (8); out: baz (p); if (ret) bar (10); return ret; } Here we jump thread it and with Martin's warning position warn, with my patch don't warn. But if the if (ret) bar (10); is removed, the code has the same problem that on the error path p will be NULL, but it is not going to be diagnosed. For -Wmaybe-nonnull we could e.g. look at if the argument is a PHI that has NULL at any of the edges; but that doesn't cover the above case, because p has just one def and so there will be no PHIs. And yet another testcase: void foo (const char *) __attribute__((nonnull)); void bar (int x) { foo (x ? (const char *) 0 : ""); } Here we warn only with GCC 6 or with my patch, but not with current trunk, so this case is a warning regression (if needed, I can split my patch into smaller chunks, the part that addresses this is keeping the warning in the FE and setting TREE_NO_WARNING if it warned, instead of not warning when optimize in the FE at all). Handling COND_EXPR in the argument might be more -Wmaybe-nonnull thing though. It isn't if you reach this source location, there will be always UB, but if you reach this source location along this and this path (and only careful analysis can discover if that path is actually possible at runtime), then there will be UB. Kind like we have -Wuninitialized and -Wmaybe-uninitialized. Jakub
On 12/16/2016 01:17 PM, Jakub Jelinek wrote: > On Fri, Dec 16, 2016 at 01:01:13PM -0700, Jeff Law wrote: >>> Thanks. Reduced to something like: >>> int >>> foo (const char *name) >>> { >>> if (name) >>> return 6; >>> return __builtin_strlen (name); >>> } >>> This is warned about both with Martin's late warning and my after ccp2 >>> warning version. We should include it in gcc testsuite. >> I'll note this is an example of a case where Andrew's work would likely help >> because it allows us to ask for a range of name_XX at the return statement >> and it'll give us back a range that is constrained by the path(s) which >> reach the return statement. >> >> Contrast to the current VRP work where each SSA_NAME has a range, but that >> range must be valid for every context in which that SSA_NAME appears. > > Well, inside the current VRP it has separate ranges for the different paths > and actually replaces the name in the strlen argument with NULL during evrp, > so doesn't suffer from the current VRP limitations. It'll do that sometimes, but it's not consistent and its a conscious design decision (which I may not necessarily agree with, but I'm not going to open that can-o-worms). > > Anyway, let's consider the warning from the linux kernel with the closedir, > I guess it can be simplified to something along the lines of: > void baz (char *) __attribute__((nonnull)); > char *bar (int); > > int > foo (void) > { > char *p = bar (1); > int ret = 0; > if (p == 0) > { > bar (2); > bar (3); > bar (4); > ret = 1; > goto out; > } > bar (5); > bar (6); > bar (7); > bar (8); > out: > baz (p); > if (ret) > bar (10); > return ret; > } > > Here we jump thread it and with Martin's warning position warn, with > my patch don't warn. But if the if (ret) bar (10); is removed, the > code has the same problem that on the error path p will be NULL, but it is > not going to be diagnosed. For -Wmaybe-nonnull we could e.g. look at if > the argument is a PHI that has NULL at any of the edges; but that doesn't > cover the above case, because p has just one def and so there will be no > PHIs. Yea, and so what if the warning changes if that statement is removed. That kind of change is inherent in any late running warning. But late warnings, in general, generate fewer false positives than early warnings. I don't see this as a reason to summarily reject Martin's work. This whole discussion highlights the primary reason why I stopped working on uninitialized warnings many years ago and focused strictly on the optimization side of the question. Everyone has a different view on what is acceptable and what is not for a false positive. Everyone has a different view on whether or not it is acceptable that a warning disappear when seemingly innocent changes are made to the source. Should we warn early and generate more false positives, or late, reducing false positives, but missing warnings in unreachable code. There is no single right answer and the debates are endless and frustrating as hell. Jeff
On 12/16/2016 09:46 AM, Jakub Jelinek wrote: > On Fri, Dec 16, 2016 at 09:36:25AM -0700, Martin Sebor wrote: >>> It does for me with an allmodconf. At -O2 I get three warnings, and at >>> -O3 I get two additional warnings. Now these additional ones happen way >>> too deep into the pipeline to be reliable. (For a reduced testcase see: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8) >> >> The warning looks quite justified in this case. The first call >> to sm_read_sector exits with the pointer being null and so the >> second call is diagnosed. That seems like a great example of >> when the warning is useful. > > No. The first call to sm_read_sector just doesn't exit. So it is warning > about dead code. Correct. It's a false positive. Effectively the loop is never going to terminate. > >> I don't claim it can't be improved but it seems pretty good as >> it is already. Among the 6 instances it's found in GCC three >> look like real bugs. > > None look like real bugs to me. But is the warning rate so high that we need to revert/reject the warning as implemented. That's my question. 6 across GCC doesn't sound bad across a multi-million line codebase. jeff
On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote: > > > I don't claim it can't be improved but it seems pretty good as > > > it is already. Among the 6 instances it's found in GCC three > > > look like real bugs. > > > > None look like real bugs to me. > But is the warning rate so high that we need to revert/reject the warning as > implemented. That's my question. 6 across GCC doesn't sound bad across a > multi-million line codebase. It isn't 6 across GCC, it is 6 across a single target and single set of compiler options. Other targets and other options have different sets, there is some overlap, but only partial. Jakub
On Mon, Dec 19, 2016 at 07:58:54PM +0100, Jakub Jelinek wrote: > On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote: > > > > I don't claim it can't be improved but it seems pretty good as > > > > it is already. Among the 6 instances it's found in GCC three > > > > look like real bugs. > > > > > > None look like real bugs to me. > > But is the warning rate so high that we need to revert/reject the warning as > > implemented. That's my question. 6 across GCC doesn't sound bad across a > > multi-million line codebase. > > It isn't 6 across GCC, it is 6 across a single target and single set of > compiler options. Other targets and other options have different sets, > there is some overlap, but only partial. Unrelated to where the warning is issued, it might be a good idea to use %K to emit it with inlining stack, otherwise figuring out why it warns will be harder than needed. Jakub
On 12/19/2016 12:12 PM, Jakub Jelinek wrote: > On Mon, Dec 19, 2016 at 07:58:54PM +0100, Jakub Jelinek wrote: >> On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote: >>>>> I don't claim it can't be improved but it seems pretty good as >>>>> it is already. Among the 6 instances it's found in GCC three >>>>> look like real bugs. >>>> >>>> None look like real bugs to me. >>> But is the warning rate so high that we need to revert/reject the warning as >>> implemented. That's my question. 6 across GCC doesn't sound bad across a >>> multi-million line codebase. >> >> It isn't 6 across GCC, it is 6 across a single target and single set of >> compiler options. Other targets and other options have different sets, >> there is some overlap, but only partial. > > Unrelated to where the warning is issued, it might be a good idea to use > %K to emit it with inlining stack, otherwise figuring out why it warns > will be harder than needed. I would think that would apply to any warning triggered once we've started optimizing the code. Don't you? jeff
On Mon, Dec 19, 2016 at 12:50:00PM -0700, Jeff Law wrote: > > Unrelated to where the warning is issued, it might be a good idea to use > > %K to emit it with inlining stack, otherwise figuring out why it warns > > will be harder than needed. > I would think that would apply to any warning triggered once we've started > optimizing the code. Don't you? Probably just any warning post-einline, yes. Note %K takes a tree, which we have in the expansion (the CALL_EXPR), but for gimple either we need to build some random tree with the location_t of gimple_location (stmt), or add another modifier that takes a location_t and otherwise does the similar thing as %K. Jakub
PR bootstrap/78817 - stage2 bootstrap failure in vec.h:1613:5: error: argument 1 null where non-null expected after r243661 gcc/ChangeLog: PR bootstrap/78817 * calls.c (maybe_warn_null_arg): Add a missing '='. * gengtype-parse.c (type): Guard against dereferncing a null pointer. (get_file_realbasename): Avoid passing a null pointer to a function expecting non-null. * tree-vect-data-refs.c (vect_permute_store_chain): Assert pointer is non-null. (vect_permute_load_chain): Same. * vec.h (vec<T, va_heap, vl_ptr>::safe_grow_cleared): Assert a pointer is non-null. diff --git a/gcc/calls.c b/gcc/calls.c index 8466427..463f99c 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1573,7 +1573,7 @@ maybe_warn_null_arg (tree fndecl, tree exp, tree arg, ++argpos; - location_t exploc EXPR_LOCATION (exp); + location_t exploc = EXPR_LOCATION (exp); if (warning_at (exploc, OPT_Wnonnull, "argument %u null where non-null expected", argpos)) diff --git a/gcc/gengtype-parse.c b/gcc/gengtype-parse.c index 954ac2a..b11da84 100644 --- a/gcc/gengtype-parse.c +++ b/gcc/gengtype-parse.c @@ -946,15 +946,18 @@ type (options_p *optsp, bool nested) inheritance specifications. We require single-inheritance from a non-template type. */ advance (); - const char *basename = require (ID); - /* This may be either an access specifier, or the base name. */ - if (0 == strcmp (basename, "public") - || 0 == strcmp (basename, "protected") - || 0 == strcmp (basename, "private")) - basename = require (ID); - base_class = find_structure (basename, TYPE_STRUCT); - if (!base_class) - parse_error ("unrecognized base class: %s", basename); + if (const char *basename = require (ID)) + { + /* This may be either an access specifier, or the base + name. */ + if (0 == strcmp (basename, "public") + || 0 == strcmp (basename, "protected") + || 0 == strcmp (basename, "private")) + basename = require (ID); + base_class = find_structure (basename, TYPE_STRUCT); + if (!base_class) + parse_error ("unrecognized base class: %s", basename); + } require_without_advance ('{'); } else diff --git a/gcc/gengtype.c b/gcc/gengtype.c index dcc2ff5..c63bd8e 100644 --- a/gcc/gengtype.c +++ b/gcc/gengtype.c @@ -1747,7 +1747,10 @@ open_base_files (void) static const char * get_file_realbasename (const input_file *inpf) { - return lbasename (get_input_file_name (inpf)); + if (const char *fname = get_input_file_name (inpf)) + return lbasename (fname); + + return NULL; } /* For INPF a filename, return the relative path to INPF from @@ -1756,12 +1759,13 @@ get_file_realbasename (const input_file *inpf) const char * get_file_srcdir_relative_path (const input_file *inpf) { - const char *f = get_input_file_name (inpf); - if (strlen (f) > srcdir_len - && IS_DIR_SEPARATOR (f[srcdir_len]) - && strncmp (f, srcdir, srcdir_len) == 0) - return f + srcdir_len + 1; - else + if (const char *f = get_input_file_name (inpf)) + { + if (strlen (f) > srcdir_len + && IS_DIR_SEPARATOR (f[srcdir_len]) + && strncmp (f, srcdir, srcdir_len) == 0) + return f + srcdir_len + 1; + } return NULL; } diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c index 1b9c3b3..967fa44 100644 --- a/gcc/tree-vect-data-refs.c +++ b/gcc/tree-vect-data-refs.c @@ -4954,7 +4954,8 @@ vect_permute_store_chain (vec<tree> dr_chain, } else { - /* If length is not equal to 3 then only power of 2 is supported. */ + /* If length is not equal to 3 then only positive powers of 2 are + supported. */ gcc_assert (pow2p_hwi (length)); for (i = 0, n = nelt / 2; i < n; i++) @@ -4994,6 +4995,11 @@ vect_permute_store_chain (vec<tree> dr_chain, vect_finish_stmt_generation (stmt, perm_stmt, gsi); (*result_chain)[2*j+1] = low; } + + /* The assert below is strictly redundant because RESULT_CHAIN + has a size equal to LENGTH which is asserted to be positive + above. Unfortunately, GCC doesn't see that. */ + gcc_assert (result_chain->address ()); memcpy (dr_chain.address (), result_chain->address (), length * sizeof (tree)); } @@ -5557,6 +5563,11 @@ vect_permute_load_chain (vec<tree> dr_chain, vect_finish_stmt_generation (stmt, perm_stmt, gsi); (*result_chain)[j/2+length/2] = data_ref; } + + /* The assert below is strictly redundant because RESULT_CHAIN + has a size equal to LENGTH which is asserted to be positive + above. Unfortunately, GCC doesn't see that. */ + gcc_assert (result_chain->address ()); memcpy (dr_chain.address (), result_chain->address (), length * sizeof (tree)); } diff --git a/gcc/vec.h b/gcc/vec.h index aa93411..b6b54ef 100644 --- a/gcc/vec.h +++ b/gcc/vec.h @@ -1607,10 +1608,16 @@ inline void vec<T, va_heap, vl_ptr>::safe_grow_cleared (unsigned len MEM_STAT_DECL) { unsigned oldlen = length (); - size_t sz = sizeof (T) * (len - oldlen); - safe_grow (len PASS_MEM_STAT); - if (sz != 0) - memset (&(address ()[oldlen]), 0, sz); + gcc_checking_assert (oldlen <= len); + + if (size_t sz = sizeof (T) * (len - oldlen)) + { + safe_grow (len PASS_MEM_STAT); + + T *p = address (); + gcc_assert (p != NULL); + memset (p + oldlen, 0, sz); + } }