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. The following is a breakdown of warnings that do show up in
those builds (including GCC's latest profiledbootstrap with the
attached patch), for reference.
Diagnostic Count Unique Files
-Wstrict-aliasing 348 4 1
-Wimplicit-fallthrough= 144 121 2
-Wformat-length= 49 47 4
-Wunused-const-variable= 12 12 1
-Wint-in-bool-context 10 10 1
-Wmaybe-uninitialized 10 6 1
-Wdangling-else 2 2 1
-Wframe-address 2 1 1
-Wnonnull 2 1 1
-Wbool-operation 1 1 1
Martin
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);
+ }
}
