* [PATCH] warn for more impossible null pointer tests
@ 2021-09-01 2:08 Martin Sebor
2021-09-01 17:35 ` Jeff Law
2021-09-01 19:21 ` Jason Merrill
0 siblings, 2 replies; 24+ messages in thread
From: Martin Sebor @ 2021-09-01 2:08 UTC (permalink / raw)
To: gcc-patches, Jason Merrill, Joseph S. Myers
[-- Attachment #1: Type: text/plain, Size: 2925 bytes --]
A Coverity run recently uncovered a latent bug in GCC that GCC should
be able to detect itself: comparing the address of a declared object
for equality to null, similar to:
int f (void)
{
int a[2][2];
return &a == 0;
}
GCC issues -Waddress for this code, but the bug Coverity found was
actually closer to the following:
int f (void)
{
int a[2][2];
return a[0] == 0;
}
where the hapless author (yours truly) meant to compare the value
of a[0][0] (as in r12-3268).
This variant is not diagnosed even though the bug in it is the same
and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
diagnosed either, though that's a less likely mistake to make).
The attached patch enhances -Waddress to detect this variant along
with a number of other similar instances of the problem, including
comparing the address of array members to null.
Besides these, the patch also issues -Waddress for null equality
tests of pointer-plus expressions such as in:
int g (int i)
{
return a[0] + i == 0;
}
and in C++ more instances of pointers to members.
Testing on x86_64-linux, besides a few benign issues in GCC sources
a regression test, run shows a failure in gcc.dg/Waddress.c. That's
a test added after GCC for some reason stopped warning for one of
the basic cases that other tools warn about (comparing an array to
null). I suspect the change was unintentional because GCC still
warns for other very similar expressions. The reporter who also
submitted the test in pr36299 argued that the warning wasn't
helpful because tests for arrays sometimes come from macros, and
the test was committed after it was noted that GCC no longer warned
for the reporter's simple case. While it's certainly true that
the warning can be triggered by the null equality tests in macros
(the patch exposed two such instances in GCC) they are easy to
avoid (the patch adds a an additional escape hatch). At the same
time, as is evident from the Coverity bug report and from the two
issues the enhancement exposes in the FORTRAN front end (even if
benign), issuing the warning in these cases does help find bugs
or mistaken assumptions. With that, I've changed the test to
expect the restored -Waddress warning instead.
Testing with Glibc exposed a couple of harmless comparisons of
arrays a large macro in vfprintf-internal.c. I'll submit a fix
to avoid the -Waddress instances if/when this enhancement is
approved.
Testing with Binutils/GDB also turned up a couple of pointless
comparison of arrays to null and a couple of uses in macros that
can be trivially suppressed.
Martin
PS Clang issues a warning for some of the same null pointer tests
the patch diagnoses, including gcc.dg/Waddress.c, except under at
least three different options: some under -Wpointer-bool-conversion,
others under -Wtautological-pointer-compare, and others still under
-Wtautological-compare.
[-- Attachment #2: gcc-102103.diff --]
[-- Type: text/x-patch, Size: 24702 bytes --]
Enhance -Waddress to detect more suspicious expressions.
Resolves:
PR c/102103 - missing warning comparing array address to null
gcc/ChangeLog:
* doc/invoke.texi (-Waddress): Update.
gcc/c-family/ChangeLog:
* c-common.c (decl_with_nonnull_addr_p): Handle members.
gcc/c/ChangeLog:
* c-typeck.c (maybe_warn_for_null_address): New function.
(build_binary_op): Call it.
gcc/cp/ChangeLog:
* typeck.c (warn_for_null_address): Enhance.
(cp_build_binary_op): Call it also for member pointers.
gcc/fortran/ChangeLog:
* gcc/fortran/array.c: Remove an unnecessary test.
* gcc/fortran/trans-array.c: Same.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-array-ptr10.C: Suppress a valid warning.
* g++.dg/warn/Wreturn-local-addr-6.C: Correct a cast.
* gcc.dg/Waddress.c: Expect a warning.
* c-c++-common/Waddress-3.c: New test.
* c-c++-common/Waddress-4.c: New test.
* g++.dg/warn/Waddress-5.C: New test.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 017e41537ac..ca3544bd066 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3393,14 +3393,16 @@ c_wrap_maybe_const (tree expr, bool non_const)
return expr;
}
-/* Return whether EXPR is a declaration whose address can never be
- NULL. */
+/* Return whether EXPR is a declaration whose address can never be NULL.
+ The address of the first struct member could be NULL only if it were
+ accessed through a NULL pointer, and such an access would be invalid. */
bool
decl_with_nonnull_addr_p (const_tree expr)
{
return (DECL_P (expr)
- && (TREE_CODE (expr) == PARM_DECL
+ && (TREE_CODE (expr) == FIELD_DECL
+ || TREE_CODE (expr) == PARM_DECL
|| TREE_CODE (expr) == LABEL_DECL
|| !DECL_WEAK (expr)));
}
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index d9f26d67bd3..d6aa4fe9263 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -11541,6 +11541,78 @@ build_vec_cmp (tree_code code, tree type,
return build3 (VEC_COND_EXPR, type, cmp, minus_one_vec, zero_vec);
}
+/* Possibly warn about an address of OP never being NULL in a comparison
+ operation CODE involving null. */
+
+static void
+maybe_warn_for_null_address (location_t loc, tree op, tree_code code)
+{
+ if (!warn_address || warning_suppressed_p (op, OPT_Waddress))
+ return;
+
+ if (TREE_CODE (op) == NOP_EXPR)
+ {
+ /* Allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (op));
+ if (VOID_TYPE_P (type))
+ return;
+ op = TREE_OPERAND (op, 0);
+ }
+
+ if (TREE_CODE (op) == POINTER_PLUS_EXPR)
+ {
+ /* Allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (op));
+ if (VOID_TYPE_P (type))
+ return;
+
+ /* Adding any value to a null pointer, including zero, is undefined
+ in C. This includes the expression &p[0] where p is the null
+ pointer, although &p[0] will have been folded to p by this point
+ and so not diagnosed. */
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+
+ return;
+ }
+
+ if (TREE_CODE (op) != ADDR_EXPR)
+ return;
+
+ op = TREE_OPERAND (op, 0);
+ while (TREE_CODE (op) == ARRAY_REF
+ || TREE_CODE (op) == COMPONENT_REF)
+ {
+ unsigned opno = TREE_CODE (op) == COMPONENT_REF;
+ op = TREE_OPERAND (op, opno);
+ }
+
+ if (!decl_with_nonnull_addr_p (op)
+ || from_macro_expansion_at (loc))
+ return;
+
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the address of %qD will never be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the address of %qD will never be NULL",
+ op);
+
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
+}
+
/* Build a binary-operation expression without default conversions.
CODE is the kind of expression to build.
LOCATION is the operator's location.
@@ -12176,44 +12248,12 @@ build_binary_op (location_t location, enum tree_code code,
short_compare = 1;
else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
{
- if (TREE_CODE (op0) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- }
+ maybe_warn_for_null_address (location, op0, code);
result_type = type0;
}
else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
{
- if (TREE_CODE (op1) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- }
+ maybe_warn_for_null_address (location, op1, code);
result_type = type1;
}
else if (code0 == POINTER_TYPE && code1 == POINTER_TYPE)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a46c6d2340d..50ee43a2ebe 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4617,28 +4617,76 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
if (!warn_address
|| (complain & tf_warning) == 0
|| c_inhibit_evaluation_warnings != 0
- || warning_suppressed_p (op, OPT_Waddress))
+ || warning_suppressed_p (op, OPT_Waddress)
+ || processing_template_decl != 0)
return;
tree cop = fold_for_warn (op);
- if (TREE_CODE (cop) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (cop, 0))
- && !warning_suppressed_p (cop, OPT_Waddress))
- warning_at (location, OPT_Waddress, "the address of %qD will never "
- "be NULL", TREE_OPERAND (cop, 0));
+ if (TREE_CODE (op) == PTRMEM_CST)
+ {
+ /* The address of a nonstatic data member is never null. */
+ warning_at (location, OPT_Waddress,
+ "the address %qE will never be NULL",
+ op);
+ return;
+ }
+
+ if (TREE_CODE (cop) == NOP_EXPR)
+ {
+ /* Allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (cop));
+ if (VOID_TYPE_P (type))
+ return;
+ STRIP_NOPS (cop);
+ }
+
+ bool warned = false;
+ if (TREE_CODE (cop) == ADDR_EXPR)
+ {
+ cop = TREE_OPERAND (cop, 0);
+ while (TREE_CODE (cop) == ARRAY_REF
+ || TREE_CODE (cop) == COMPONENT_REF)
+ {
+ unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
+ cop = TREE_OPERAND (cop, opno);
+ }
+
+ if (decl_with_nonnull_addr_p (cop)
+ && !warning_suppressed_p (cop, OPT_Waddress))
+ warned = warning_at (location, OPT_Waddress,
+ "the address of %qD will never be NULL", cop);
+ }
+ else if (TREE_CODE (cop) == POINTER_PLUS_EXPR)
+ {
+ /* Same as above, allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (cop));
+ if (VOID_TYPE_P (type))
+ return;
+
+ /* Adding zero to the null pointer is well-defined in C++. When
+ the offset is unknown (i.e., not a constant) warn anyway since
+ it's less likely that the pointer operand is null than not. */
+ tree off = TREE_OPERAND (cop, 1);
+ if (!integer_zerop (off))
+ warning_at (location, OPT_Waddress, "comparing the result of pointer "
+ "addition %qE and NULL", cop);
+ return;
+ }
if (CONVERT_EXPR_P (op)
&& TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
{
- tree inner_op = op;
- STRIP_NOPS (inner_op);
+ STRIP_NOPS (op);
- if (DECL_P (inner_op))
- warning_at (location, OPT_Waddress,
- "the compiler can assume that the address of "
- "%qD will never be NULL", inner_op);
+ if (DECL_P (op))
+ warned = warning_at (location, OPT_Waddress,
+ "the compiler can assume that the address of "
+ "%qD will never be NULL", op);
}
+
+ if (warned && DECL_P (op))
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
}
/* Warn about [expr.arith.conv]/2: If one operand is of enumeration type and
@@ -5428,6 +5476,8 @@ cp_build_binary_op (const op_location_t &location,
op1 = cp_convert (TREE_TYPE (op0), op1, complain);
}
result_type = TREE_TYPE (op0);
+
+ warn_for_null_address (location, orig_op0, complain);
}
else if (TYPE_PTRMEMFUNC_P (type1) && null_ptr_cst_p (orig_op0))
return cp_build_binary_op (location, code, op1, op0, complain);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b83bd902cec..9ffc7f2a2b4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8547,17 +8547,40 @@ by @option{-Wall}.
@item -Waddress
@opindex Waddress
@opindex Wno-address
-Warn about suspicious uses of memory addresses. These include using
-the address of a function in a conditional expression, such as
-@code{void func(void); if (func)}, and comparisons against the memory
-address of a string literal, such as @code{if (x == "abc")}. Such
-uses typically indicate a programmer error: the address of a function
-always evaluates to true, so their use in a conditional usually
-indicate that the programmer forgot the parentheses in a function
-call; and comparisons against string literals result in unspecified
-behavior and are not portable in C, so they usually indicate that the
-programmer intended to use @code{strcmp}. This warning is enabled by
-@option{-Wall}.
+Warn about suspicious uses of address expressions. These include comparing
+the address of a function or a declared object to the null pointer constant
+such as in
+@smallexample
+void f (void);
+void g (void)
+@{
+ if (!func) // warning: expression evaluates to false
+ abort ();
+@}
+@end smallexample
+comparisons of a pointer to a string literal, such as in
+@smallexample
+void f (const char *x)
+@{
+ if (x == "abc") // warning: expression evaluates to false
+ puts ("equal");
+@}
+@end smallexample
+and tests of the results of pointer addition or subtraction for equality
+to null, such as in
+@smallexample
+void f (const int *p, int i)
+@{
+ return p + i == NULL;
+@}
+@end smallexample
+Such uses typically indicate a programmer error: the address of most
+functions and objects necessarily evaluates to true (the exception are
+weak symbols), so their use in a conditional might indicate missing
+parentheses in a function call or a missing dereference in an array
+expression. Comparisons against string literals result in unspecified
+behavior and are not portable, and suggest the intent was to call
+@code{strcmp}. @option{-Waddress} warning is enabled by @option{-Wall}.
@item -Wno-address-of-packed-member
@opindex Waddress-of-packed-member
diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index b858bada18a..341290f91b8 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -2578,7 +2578,7 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t *result)
}
}
- if (array->shape && array->shape[dimen])
+ if (array->shape)
{
mpz_init_set (*result, array->shape[dimen]);
return true;
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0d013defdbb..5147509fbe7 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5104,7 +5104,6 @@ set_loop_bounds (gfc_loopinfo *loop)
if (info->shape)
{
- gcc_assert (info->shape[dim]);
/* The frontend has worked out the size for us. */
if (!loopspec[n]
|| !specinfo->shape
diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 31d4bf4e5d0..fa6b81e806f 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -3685,8 +3685,8 @@ write_types (outf_p output_header, type_p structures,
output_mangled_typename (output_header, s);
oprintf (output_header, "(X) do { \\\n");
oprintf (output_header,
- " if (X != NULL) gt_%sx_%s (X);\\\n", wtd->prefix,
- s_id_for_tag);
+ " if ((void *)(X) != NULL) gt_%sx_%s (X);\\\n",
+ wtd->prefix, s_id_for_tag);
oprintf (output_header, " } while (0)\n");
for (opt = s->u.s.opt; opt; opt = opt->next)
diff --git a/gcc/poly-int.h b/gcc/poly-int.h
index f47f9e436a8..94e7b701f64 100644
--- a/gcc/poly-int.h
+++ b/gcc/poly-int.h
@@ -324,10 +324,10 @@ struct poly_result<T1, T2, 2>
routine can take the address of RES rather than the address of
a temporary.
- The dummy comparison against a null C * is just a way of checking
+ The dummy self-comparison against C * is just a way of checking
that C gives the right type. */
#define POLY_SET_COEFF(C, RES, I, VALUE) \
- ((void) (&(RES).coeffs[0] == (C *) 0), \
+ ((void) (&(RES).coeffs[0] == (C *) (void *) &(RES).coeffs[0]), \
wi::int_traits<C>::precision_type == wi::FLEXIBLE_PRECISION \
? (void) ((RES).coeffs[I] = VALUE) \
: (void) ((RES).coeffs[I].~C (), new (&(RES).coeffs[I]) C (VALUE)))
diff --git a/gcc/testsuite/c-c++-common/Waddress-3.c b/gcc/testsuite/c-c++-common/Waddress-3.c
new file mode 100644
index 00000000000..2390f6722b8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-3.c
@@ -0,0 +1,226 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+struct S { void *p, *a1[2], *a2[2][2]; } s, *p;
+
+extern void *a1[2], *a2[2][2], *ax[];
+
+int x;
+
+void test_array_eq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)a1 == 0)
+ ++x;
+
+ if (a1 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a1) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a1[0] == 0)
+ ++x;
+
+ if (0 == (void *)&a1[0])
+ ++x;
+
+ if (0 == &a1[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a1[i] == 0)
+ ++x;
+
+ if (0 == (void *)&a1[i])
+ ++x;
+
+ if (0 == &a1[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[0] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a1[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[i] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[0][0] == 0)
+ ++x;
+
+ if (0 == &a2[0][0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&ax == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &ax) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&ax[0] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == ax[0])
+ ++x;
+}
+
+
+void test_array_neq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)a1)
+ ++x;
+
+ if (a1) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a1 != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a1[0])
+ ++x;
+
+ if (&a1[0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a1[i])
+ ++x;
+
+ if (&a1[i] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a2 != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a1[0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a2[i] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2[0][0])
+ ++x;
+
+ if (&a2[0][0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+}
+
+
+void test_member_array_eq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)s.a1 == 0)
+ ++x;
+
+ if (s.a1 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a1) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a1[0] == 0)
+ ++x;
+
+ if (0 == &a1[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a1[i] == 0)
+ ++x;
+
+ if (0 == &a1[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[0] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a1[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[i] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[0][0] == 0)
+ ++x;
+
+ if (0 == &a2[0][0]) // { dg-warning "-Waddress" }
+ ++x;
+}
+
+
+void test_member_array_neq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)s.a1)
+ ++x;
+
+ if (s.a1) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&s.a1 != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a1[0])
+ ++x;
+
+ if (&s.a1[0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a1[i])
+ ++x;
+
+ if (&s.a1[i] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&s.a2 != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[0]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&s.a1[0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[i]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&s.a2[i] != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (s.a2[0][0])
+ ++x;
+
+ if (&s.a2[0][0] != 0) // { dg-warning "-Waddress" }
+ ++x;
+}
diff --git a/gcc/testsuite/c-c++-common/Waddress-4.c b/gcc/testsuite/c-c++-common/Waddress-4.c
new file mode 100644
index 00000000000..6aa94f4c375
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-4.c
@@ -0,0 +1,109 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+extern char *ax[], *a2[][2];
+
+extern int x;
+
+void test_ax_plus_eq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)(ax + 0) == 0)
+ ++x;
+
+ if (ax + 0 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&ax[0] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (ax - 1 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &ax[-1]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if ((void *)(&ax[0] + 2) == 0)
+ ++x;
+
+ if (&ax[0] + 2 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (ax + 3 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &ax[-4]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (ax - i == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&ax[i] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &ax[1] + i) // { dg-warning "-Waddress" }
+ ++x;
+}
+
+void test_a2_plus_eq_0 (int i)
+{
+ // Verify that a cast to void* suppresses the warning.
+ if ((void *)(a2 + 0) == 0)
+ ++x;
+
+ if (a2 + 0 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a2[0] == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2 - 1 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2[-1]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2 + 2 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &a2[-2]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (a2 - i == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&a2[i] == 0) // { dg-warning "-Waddress" }
+ ++x;
+}
+
+
+void test_p_plus_eq_0 (int *p, int i)
+{
+ /* P + 0 and equivalently &P[0] are folded to p before the warning
+ has a chance to trigger. */
+
+ if (p + 0 == 0) // { dg-warning "-Waddress" "pr??????" { xfail *-*-*} }
+ ++x;
+
+ if (&p[0] == 0) // { dg-warning "-Waddress" "pr??????" { xfail *-*-*} }
+ ++x;
+
+ if (p - 1 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &p[-1]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (p + 2 == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (0 == &p[-2]) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (p - i == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&p[i] == 0) // { dg-warning "-Waddress" }
+ ++x;
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
index 5224bb14234..63295230d51 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
@@ -85,8 +85,11 @@ extern __attribute__ ((weak)) int i;
constexpr int *p1 = &i + 1;
#pragma GCC diagnostic push
+// Suppress warning: ordered comparison of pointer with integer zero.
#pragma GCC diagnostic ignored "-Wextra"
-// Suppress warning: ordered comparison of pointer with integer zero
+// Also suppress -Waddress for comparisons of constant addresses to
+// to null.
+#pragma GCC diagnostic ignored "-Waddress"
constexpr bool b0 = p1; // { dg-error "not a constant expression" }
constexpr bool b1 = p1 == 0; // { dg-error "not a constant expression" }
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-5.C b/gcc/testsuite/g++.dg/warn/Waddress-5.C
new file mode 100644
index 00000000000..5278cda2fab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-5.C
@@ -0,0 +1,75 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+struct A
+{
+ void f ();
+ virtual void vf ();
+ virtual void pvf () = 0;
+
+ void sf ();
+
+ int *p;
+ int a[2];
+};
+
+int x;
+
+void warn_memptr ()
+{
+ // Exercise warnings for addresses of nonstatic member functions.
+ if (&A::f == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&A::vf) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&A::pvf != 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ // Exercise warnings for addresses of static member functions.
+ if (&A::sf == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&A::sf) // { dg-warning "-Waddress" }
+ ++x;
+
+ // Exercise warnings for addresses of nonstatic data members.
+ if (&A::p == 0) // { dg-warning "-Waddress" }
+ ++x;
+
+ if (&A::a == 0) // { dg-warning "-Waddress" }
+ ++x;
+}
+
+
+// Verify that no warnings are issued for an uninstantiated template.
+
+template <int>
+struct B
+{
+ // This is why.
+ struct F { void* operator& () const { return 0; } } f;
+};
+
+template <int N>
+void nowarn_template ()
+{
+ if (&B<N>::f == 0)
+ ++x;
+
+ /* Like in the case above, the address-of operator could be a member
+ of B<N>::vf that returns zero. */
+ if (&B<N>::vf)
+ ++x;
+
+ if (&B<N>::pvf != 0)
+ ++x;
+
+ if (&B<N>::p == 0)
+ ++x;
+
+ if (&B<N>::a == 0)
+ ++x;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
index bfe14457547..fae8b7e766f 100644
--- a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
+++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
@@ -9,7 +9,7 @@ const intptr_t&
return_addr_label_as_intref (void)
{
label:
- if ((const intptr_t*)&&label == 0)
+ if ((const intptr_t)&&label == 0)
__builtin_exit (1);
return *(const intptr_t*)&&label; // { dg-warning "\\\[-Wreturn-local-addr]" } */
@@ -19,7 +19,7 @@ const intptr_t&
return_addr_local_as_intref (void)
{
int a[1];
- if ((const intptr_t*)a == 0)
+ if ((const intptr_t)a == 0)
__builtin_exit (1);
return (const intptr_t&)a; // { dg-warning "\\\[-Wreturn-local-addr]" } */
diff --git a/gcc/testsuite/gcc.dg/Waddress.c b/gcc/testsuite/gcc.dg/Waddress.c
index 146b1a932df..b26e7b1f329 100644
--- a/gcc/testsuite/gcc.dg/Waddress.c
+++ b/gcc/testsuite/gcc.dg/Waddress.c
@@ -6,5 +6,5 @@ int
foo(void)
{
char a[1];
- return a == 0;
+ return a == 0; // { dg-warning "-Waddress" }
}
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 2:08 [PATCH] warn for more impossible null pointer tests Martin Sebor
@ 2021-09-01 17:35 ` Jeff Law
2021-09-01 18:57 ` Koning, Paul
2021-09-01 19:21 ` Jason Merrill
1 sibling, 1 reply; 24+ messages in thread
From: Jeff Law @ 2021-09-01 17:35 UTC (permalink / raw)
To: gcc-patches
On 8/31/2021 8:08 PM, Martin Sebor via Gcc-patches wrote:
> A Coverity run recently uncovered a latent bug in GCC that GCC should
> be able to detect itself: comparing the address of a declared object
> for equality to null, similar to:
>
> int f (void)
> {
> int a[2][2];
> return &a == 0;
> }
>
> GCC issues -Waddress for this code, but the bug Coverity found was
> actually closer to the following:
>
> int f (void)
> {
> int a[2][2];
> return a[0] == 0;
> }
>
> where the hapless author (yours truly) meant to compare the value
> of a[0][0] (as in r12-3268).
>
> This variant is not diagnosed even though the bug in it is the same
> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
> diagnosed either, though that's a less likely mistake to make).
>
> The attached patch enhances -Waddress to detect this variant along
> with a number of other similar instances of the problem, including
> comparing the address of array members to null.
>
> Besides these, the patch also issues -Waddress for null equality
> tests of pointer-plus expressions such as in:
>
> int g (int i)
> {
> return a[0] + i == 0;
> }
>
> and in C++ more instances of pointers to members.
>
> Testing on x86_64-linux, besides a few benign issues in GCC sources
> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
> a test added after GCC for some reason stopped warning for one of
> the basic cases that other tools warn about (comparing an array to
> null). I suspect the change was unintentional because GCC still
> warns for other very similar expressions. The reporter who also
> submitted the test in pr36299 argued that the warning wasn't
> helpful because tests for arrays sometimes come from macros, and
> the test was committed after it was noted that GCC no longer warned
> for the reporter's simple case. While it's certainly true that
> the warning can be triggered by the null equality tests in macros
> (the patch exposed two such instances in GCC) they are easy to
> avoid (the patch adds a an additional escape hatch). At the same
> time, as is evident from the Coverity bug report and from the two
> issues the enhancement exposes in the FORTRAN front end (even if
> benign), issuing the warning in these cases does help find bugs
> or mistaken assumptions. With that, I've changed the test to
> expect the restored -Waddress warning instead.
>
> Testing with Glibc exposed a couple of harmless comparisons of
> arrays a large macro in vfprintf-internal.c. I'll submit a fix
> to avoid the -Waddress instances if/when this enhancement is
> approved.
>
> Testing with Binutils/GDB also turned up a couple of pointless
> comparison of arrays to null and a couple of uses in macros that
> can be trivially suppressed.
>
> Martin
>
> PS Clang issues a warning for some of the same null pointer tests
> the patch diagnoses, including gcc.dg/Waddress.c, except under at
> least three different options: some under -Wpointer-bool-conversion,
> others under -Wtautological-pointer-compare, and others still under
> -Wtautological-compare.
>
>
> gcc-102103.diff
>
> Enhance -Waddress to detect more suspicious expressions.
>
> Resolves:
> PR c/102103 - missing warning comparing array address to null
>
>
> gcc/ChangeLog:
>
> * doc/invoke.texi (-Waddress): Update.
>
> gcc/c-family/ChangeLog:
>
> * c-common.c (decl_with_nonnull_addr_p): Handle members.
>
> gcc/c/ChangeLog:
>
> * c-typeck.c (maybe_warn_for_null_address): New function.
> (build_binary_op): Call it.
>
> gcc/cp/ChangeLog:
>
> * typeck.c (warn_for_null_address): Enhance.
> (cp_build_binary_op): Call it also for member pointers.
>
> gcc/fortran/ChangeLog:
>
> * gcc/fortran/array.c: Remove an unnecessary test.
> * gcc/fortran/trans-array.c: Same.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-array-ptr10.C: Suppress a valid warning.
> * g++.dg/warn/Wreturn-local-addr-6.C: Correct a cast.
> * gcc.dg/Waddress.c: Expect a warning.
> * c-c++-common/Waddress-3.c: New test.
> * c-c++-common/Waddress-4.c: New test.
> * g++.dg/warn/Waddress-5.C: New test.
Generally OK. There's some C++ front-end bits that Jason ought to take
a quick looksie at. Second, how does this interact with targets that
allow objects at address 0? We have a few targets like that and that
makes me wonder if we should be suppressing some, if not all, of these
warnings for targets that turn on -fno-delete-null-pointer-checks?
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 17:35 ` Jeff Law
@ 2021-09-01 18:57 ` Koning, Paul
2021-09-01 19:08 ` Jeff Law
0 siblings, 1 reply; 24+ messages in thread
From: Koning, Paul @ 2021-09-01 18:57 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches
> On Sep 1, 2021, at 1:35 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> Generally OK. There's some C++ front-end bits that Jason ought to take a quick looksie at. Second, how does this interact with targets that allow objects at address 0? We have a few targets like that and that makes me wonder if we should be suppressing some, if not all, of these warnings for targets that turn on -fno-delete-null-pointer-checks?
But in C, the pointer constant 0 represents the null (invalid) pointer, not the actual address zero necessarily.
If a target supports objects at address zero, how does it represent the pointer value 0 (which we usually refer to as NULL)? Is the issue simply ignored? It seems to me it is in pdp11, which I would guess is one of the targets for which objects at address 0 make sense.
paul
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 18:57 ` Koning, Paul
@ 2021-09-01 19:08 ` Jeff Law
2021-09-01 19:28 ` Koning, Paul
0 siblings, 1 reply; 24+ messages in thread
From: Jeff Law @ 2021-09-01 19:08 UTC (permalink / raw)
To: Koning, Paul; +Cc: GCC Patches
On 9/1/2021 12:57 PM, Koning, Paul wrote:
>
>> On Sep 1, 2021, at 1:35 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>
>> Generally OK. There's some C++ front-end bits that Jason ought to take a quick looksie at. Second, how does this interact with targets that allow objects at address 0? We have a few targets like that and that makes me wonder if we should be suppressing some, if not all, of these warnings for targets that turn on -fno-delete-null-pointer-checks?
> But in C, the pointer constant 0 represents the null (invalid) pointer, not the actual address zero necessarily.
>
> If a target supports objects at address zero, how does it represent the pointer value 0 (which we usually refer to as NULL)? Is the issue simply ignored? It seems to me it is in pdp11, which I would guess is one of the targets for which objects at address 0 make sense.
The issue is ignored to the best of my knowledge.
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 2:08 [PATCH] warn for more impossible null pointer tests Martin Sebor
2021-09-01 17:35 ` Jeff Law
@ 2021-09-01 19:21 ` Jason Merrill
2021-09-01 20:33 ` Martin Sebor
1 sibling, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2021-09-01 19:21 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers
On 8/31/21 10:08 PM, Martin Sebor wrote:
> A Coverity run recently uncovered a latent bug in GCC that GCC should
> be able to detect itself: comparing the address of a declared object
> for equality to null, similar to:
>
> int f (void)
> {
> int a[2][2];
> return &a == 0;
> }
>
> GCC issues -Waddress for this code, but the bug Coverity found was
> actually closer to the following:
>
> int f (void)
> {
> int a[2][2];
> return a[0] == 0;
> }
>
> where the hapless author (yours truly) meant to compare the value
> of a[0][0] (as in r12-3268).
>
> This variant is not diagnosed even though the bug in it is the same
> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
> diagnosed either, though that's a less likely mistake to make).
>
> The attached patch enhances -Waddress to detect this variant along
> with a number of other similar instances of the problem, including
> comparing the address of array members to null.
>
> Besides these, the patch also issues -Waddress for null equality
> tests of pointer-plus expressions such as in:
>
> int g (int i)
> {
> return a[0] + i == 0;
> }
>
> and in C++ more instances of pointers to members.
>
> Testing on x86_64-linux, besides a few benign issues in GCC sources
> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
> a test added after GCC for some reason stopped warning for one of
> the basic cases that other tools warn about (comparing an array to
> null). I suspect the change was unintentional because GCC still
> warns for other very similar expressions. The reporter who also
> submitted the test in pr36299 argued that the warning wasn't
> helpful because tests for arrays sometimes come from macros, and
> the test was committed after it was noted that GCC no longer warned
> for the reporter's simple case. While it's certainly true that
> the warning can be triggered by the null equality tests in macros
> (the patch exposed two such instances in GCC) they are easy to
> avoid (the patch adds a an additional escape hatch). At the same
> time, as is evident from the Coverity bug report and from the two
> issues the enhancement exposes in the FORTRAN front end (even if
> benign), issuing the warning in these cases does help find bugs
> or mistaken assumptions. With that, I've changed the test to
> expect the restored -Waddress warning instead.
>
> Testing with Glibc exposed a couple of harmless comparisons of
> arrays a large macro in vfprintf-internal.c. I'll submit a fix
> to avoid the -Waddress instances if/when this enhancement is
> approved.
>
> Testing with Binutils/GDB also turned up a couple of pointless
> comparison of arrays to null and a couple of uses in macros that
> can be trivially suppressed.
>
> Martin
>
> PS Clang issues a warning for some of the same null pointer tests
> the patch diagnoses, including gcc.dg/Waddress.c, except under at
> least three different options: some under -Wpointer-bool-conversion,
> others under -Wtautological-pointer-compare, and others still under
> -Wtautological-compare.
> + while (TREE_CODE (cop) == ARRAY_REF
> + || TREE_CODE (cop) == COMPONENT_REF)
> + {
> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
> + cop = TREE_OPERAND (cop, opno);
> + }
1) Maybe 'while (handled_component_p (cop))'?
2) Why handle COMPONENT_REF differently? Operand 1 is the FIELD_DECL,
which doesn't have an address of its own.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 19:08 ` Jeff Law
@ 2021-09-01 19:28 ` Koning, Paul
2021-09-01 19:35 ` Iain Sandoe
0 siblings, 1 reply; 24+ messages in thread
From: Koning, Paul @ 2021-09-01 19:28 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches
> On Sep 1, 2021, at 3:08 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 9/1/2021 12:57 PM, Koning, Paul wrote:
>>
>>> On Sep 1, 2021, at 1:35 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>
>>> Generally OK. There's some C++ front-end bits that Jason ought to take a quick looksie at. Second, how does this interact with targets that allow objects at address 0? We have a few targets like that and that makes me wonder if we should be suppressing some, if not all, of these warnings for targets that turn on -fno-delete-null-pointer-checks?
>> But in C, the pointer constant 0 represents the null (invalid) pointer, not the actual address zero necessarily.
>>
>> If a target supports objects at address zero, how does it represent the pointer value 0 (which we usually refer to as NULL)? Is the issue simply ignored? It seems to me it is in pdp11, which I would guess is one of the targets for which objects at address 0 make sense.
> The issue is ignored to the best of my knowledge.
If so, then I would think that ignoring it for this patch as well is reasonable. If in a given target a pointer that C thinks of as NULL is in fact a valid object pointer, then all sorts of optimizations are incorrect. If the target really cares, it can use a different representation for the null pointer. (Does GCC give us a way to do that?) For example, pdp11 could use the all-ones bit pattern to represent an invalid pointer.
paul
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 19:28 ` Koning, Paul
@ 2021-09-01 19:35 ` Iain Sandoe
2021-09-01 19:58 ` Andreas Schwab
2021-09-01 20:36 ` Koning, Paul
0 siblings, 2 replies; 24+ messages in thread
From: Iain Sandoe @ 2021-09-01 19:35 UTC (permalink / raw)
To: Koning, Paul; +Cc: Jeff Law, GCC Patches
Hi Paul,
> On 1 Sep 2021, at 20:28, Koning, Paul via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>> On Sep 1, 2021, at 3:08 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> On 9/1/2021 12:57 PM, Koning, Paul wrote:
>>>
>>>> On Sep 1, 2021, at 1:35 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>> Generally OK. There's some C++ front-end bits that Jason ought to take a quick looksie at. Second, how does this interact with targets that allow objects at address 0? We have a few targets like that and that makes me wonder if we should be suppressing some, if not all, of these warnings for targets that turn on -fno-delete-null-pointer-checks?
>>> But in C, the pointer constant 0 represents the null (invalid) pointer, not the actual address zero necessarily.
>>>
>>> If a target supports objects at address zero, how does it represent the pointer value 0 (which we usually refer to as NULL)? Is the issue simply ignored? It seems to me it is in pdp11, which I would guess is one of the targets for which objects at address 0 make sense.
>> The issue is ignored to the best of my knowledge.
>
> If so, then I would think that ignoring it for this patch as well is reasonable. If in a given target a pointer that C thinks of as NULL is in fact a valid object pointer, then all sorts of optimizations are incorrect. If the target really cares, it can use a different representation for the null pointer. (Does GCC give us a way to do that?) For example, pdp11 could use the all-ones bit pattern to represent an invalid pointer.
regardless of whether GCC supports it or not - trying to use a non-0 NULL pointer is likely to break massive amounts of code in the wild.
It might, OTOH, be possible to use a non-0 special value to represent the valid 0 address-use (providing that there is somewhere in the address space you can steal that from).
I wonder what things like m68k do that have vector tables at 0 …
Iain
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 19:35 ` Iain Sandoe
@ 2021-09-01 19:58 ` Andreas Schwab
2021-09-01 20:36 ` Koning, Paul
1 sibling, 0 replies; 24+ messages in thread
From: Andreas Schwab @ 2021-09-01 19:58 UTC (permalink / raw)
To: Iain Sandoe via Gcc-patches; +Cc: Koning, Paul, Iain Sandoe
On Sep 01 2021, Iain Sandoe via Gcc-patches wrote:
> I wonder what things like m68k do that have vector tables at 0 …
Vector 0 is the reset stack pointer, if that isn't ROM you have a
problem. On the Atari, the MCU redirects physical addresses 0-7 to the
system ROM. Then there is the VBR on m68010+.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 19:21 ` Jason Merrill
@ 2021-09-01 20:33 ` Martin Sebor
2021-09-01 21:39 ` Jason Merrill
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-01 20:33 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers
On 9/1/21 1:21 PM, Jason Merrill wrote:
> On 8/31/21 10:08 PM, Martin Sebor wrote:
>> A Coverity run recently uncovered a latent bug in GCC that GCC should
>> be able to detect itself: comparing the address of a declared object
>> for equality to null, similar to:
>>
>> int f (void)
>> {
>> int a[2][2];
>> return &a == 0;
>> }
>>
>> GCC issues -Waddress for this code, but the bug Coverity found was
>> actually closer to the following:
>>
>> int f (void)
>> {
>> int a[2][2];
>> return a[0] == 0;
>> }
>>
>> where the hapless author (yours truly) meant to compare the value
>> of a[0][0] (as in r12-3268).
>>
>> This variant is not diagnosed even though the bug in it is the same
>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>> diagnosed either, though that's a less likely mistake to make).
>>
>> The attached patch enhances -Waddress to detect this variant along
>> with a number of other similar instances of the problem, including
>> comparing the address of array members to null.
>>
>> Besides these, the patch also issues -Waddress for null equality
>> tests of pointer-plus expressions such as in:
>>
>> int g (int i)
>> {
>> return a[0] + i == 0;
>> }
>>
>> and in C++ more instances of pointers to members.
>>
>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>> a test added after GCC for some reason stopped warning for one of
>> the basic cases that other tools warn about (comparing an array to
>> null). I suspect the change was unintentional because GCC still
>> warns for other very similar expressions. The reporter who also
>> submitted the test in pr36299 argued that the warning wasn't
>> helpful because tests for arrays sometimes come from macros, and
>> the test was committed after it was noted that GCC no longer warned
>> for the reporter's simple case. While it's certainly true that
>> the warning can be triggered by the null equality tests in macros
>> (the patch exposed two such instances in GCC) they are easy to
>> avoid (the patch adds a an additional escape hatch). At the same
>> time, as is evident from the Coverity bug report and from the two
>> issues the enhancement exposes in the FORTRAN front end (even if
>> benign), issuing the warning in these cases does help find bugs
>> or mistaken assumptions. With that, I've changed the test to
>> expect the restored -Waddress warning instead.
>>
>> Testing with Glibc exposed a couple of harmless comparisons of
>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>> to avoid the -Waddress instances if/when this enhancement is
>> approved.
>>
>> Testing with Binutils/GDB also turned up a couple of pointless
>> comparison of arrays to null and a couple of uses in macros that
>> can be trivially suppressed.
>>
>> Martin
>>
>> PS Clang issues a warning for some of the same null pointer tests
>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>> least three different options: some under -Wpointer-bool-conversion,
>> others under -Wtautological-pointer-compare, and others still under
>> -Wtautological-compare.
>
>> + while (TREE_CODE (cop) == ARRAY_REF
>> + || TREE_CODE (cop) == COMPONENT_REF)
>> + {
>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>> + cop = TREE_OPERAND (cop, opno);
>> + }
>
> 1) Maybe 'while (handled_component_p (cop))'?
> 2) Why handle COMPONENT_REF differently? Operand 1 is the FIELD_DECL,
> which doesn't have an address of its own.
This is because the address of a field is never null, regardless of
what the P in in &P->M points to. (With the caveat mentioned in
the comment further up about the pointer used to access the member
being nonnull.) So this is diagnosed:
extern struct { int m; } *p;
bool b = &p->m == 0;
Using handled_component_p() in a loop would prevent that.
For array_refs, the loop gets us the decl to mention in the warning.
But this should work too and looks cleaner:
cop = TREE_OPERAND (cop, 0);
/* Get the outermost array. */
while (TREE_CODE (cop) == ARRAY_REF)
cop = TREE_OPERAND (cop, 0);
/* Get the member (its address is never null). */
if (TREE_CODE (cop) == COMPONENT_REF)
cop = TREE_OPERAND (cop, 1);
Do you prefer the above instead?
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 19:35 ` Iain Sandoe
2021-09-01 19:58 ` Andreas Schwab
@ 2021-09-01 20:36 ` Koning, Paul
1 sibling, 0 replies; 24+ messages in thread
From: Koning, Paul @ 2021-09-01 20:36 UTC (permalink / raw)
To: Iain Sandoe; +Cc: Jeff Law, GCC Patches
> On Sep 1, 2021, at 3:35 PM, Iain Sandoe <idsandoe@googlemail.com> wrote:
>
>
> [EXTERNAL EMAIL]
>
> Hi Paul,
>
>> ...
>> If so, then I would think that ignoring it for this patch as well is reasonable. If in a given target a pointer that C thinks of as NULL is in fact a valid object pointer, then all sorts of optimizations are incorrect. If the target really cares, it can use a different representation for the null pointer. (Does GCC give us a way to do that?) For example, pdp11 could use the all-ones bit pattern to represent an invalid pointer.
>
> regardless of whether GCC supports it or not - trying to use a non-0 NULL pointer is likely to break massive amounts of code in the wild.
It depends on what you mean by "non-0 NULL pointer". The constant written as 0 in pointer context doesn't represent the all-zeroes bit pattern but rather whatever is a null pointer on that target. Most code would not notice that. The two places I can think of where this would break is (a) if you cast a pointer to int or look at it via a pointer/int union and expect to see integer zero, and (b) if you initialize pointers by using bzero. The former seems rather unlikely, the latter is somewhat common. Can GCC detect the bzero case? It would make a good check for -Wpedantic on the usual platforms that use all zero bits as NULL.
> It might, OTOH, be possible to use a non-0 special value to represent the valid 0 address-use (providing that there is somewhere in the address space you can steal that from).
That would be really ugly, because every pointer reference would have to do the address translation at run time.
paul
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 20:33 ` Martin Sebor
@ 2021-09-01 21:39 ` Jason Merrill
2021-09-01 22:27 ` Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2021-09-01 21:39 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers
On 9/1/21 4:33 PM, Martin Sebor wrote:
> On 9/1/21 1:21 PM, Jason Merrill wrote:
>> On 8/31/21 10:08 PM, Martin Sebor wrote:
>>> A Coverity run recently uncovered a latent bug in GCC that GCC should
>>> be able to detect itself: comparing the address of a declared object
>>> for equality to null, similar to:
>>>
>>> int f (void)
>>> {
>>> int a[2][2];
>>> return &a == 0;
>>> }
>>>
>>> GCC issues -Waddress for this code, but the bug Coverity found was
>>> actually closer to the following:
>>>
>>> int f (void)
>>> {
>>> int a[2][2];
>>> return a[0] == 0;
>>> }
>>>
>>> where the hapless author (yours truly) meant to compare the value
>>> of a[0][0] (as in r12-3268).
>>>
>>> This variant is not diagnosed even though the bug in it is the same
>>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>>> diagnosed either, though that's a less likely mistake to make).
>>>
>>> The attached patch enhances -Waddress to detect this variant along
>>> with a number of other similar instances of the problem, including
>>> comparing the address of array members to null.
>>>
>>> Besides these, the patch also issues -Waddress for null equality
>>> tests of pointer-plus expressions such as in:
>>>
>>> int g (int i)
>>> {
>>> return a[0] + i == 0;
>>> }
>>>
>>> and in C++ more instances of pointers to members.
>>>
>>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>>> a test added after GCC for some reason stopped warning for one of
>>> the basic cases that other tools warn about (comparing an array to
>>> null). I suspect the change was unintentional because GCC still
>>> warns for other very similar expressions. The reporter who also
>>> submitted the test in pr36299 argued that the warning wasn't
>>> helpful because tests for arrays sometimes come from macros, and
>>> the test was committed after it was noted that GCC no longer warned
>>> for the reporter's simple case. While it's certainly true that
>>> the warning can be triggered by the null equality tests in macros
>>> (the patch exposed two such instances in GCC) they are easy to
>>> avoid (the patch adds a an additional escape hatch). At the same
>>> time, as is evident from the Coverity bug report and from the two
>>> issues the enhancement exposes in the FORTRAN front end (even if
>>> benign), issuing the warning in these cases does help find bugs
>>> or mistaken assumptions. With that, I've changed the test to
>>> expect the restored -Waddress warning instead.
>>>
>>> Testing with Glibc exposed a couple of harmless comparisons of
>>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>>> to avoid the -Waddress instances if/when this enhancement is
>>> approved.
>>>
>>> Testing with Binutils/GDB also turned up a couple of pointless
>>> comparison of arrays to null and a couple of uses in macros that
>>> can be trivially suppressed.
>>>
>>> Martin
>>>
>>> PS Clang issues a warning for some of the same null pointer tests
>>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>>> least three different options: some under -Wpointer-bool-conversion,
>>> others under -Wtautological-pointer-compare, and others still under
>>> -Wtautological-compare.
>>
>>> + while (TREE_CODE (cop) == ARRAY_REF
>>> + || TREE_CODE (cop) == COMPONENT_REF)
>>> + {
>>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>>> + cop = TREE_OPERAND (cop, opno);
>>> + }
>>
>> 1) Maybe 'while (handled_component_p (cop))'?
>> 2) Why handle COMPONENT_REF differently? Operand 1 is the FIELD_DECL,
>> which doesn't have an address of its own.
>
> This is because the address of a field is never null, regardless of
> what the P in in &P->M points to.
True, though I'd change "invalid" to "undefined" in the comment for
decl_with_nonnull_addr_p.
> (With the caveat mentioned in
> the comment further up about the pointer used to access the member
> being nonnull.) So this is diagnosed:
>
> extern struct { int m; } *p;
> bool b = &p->m == 0;
>
> Using handled_component_p() in a loop would prevent that.
Would it? p isn't declared weak.
> For array_refs, the loop gets us the decl to mention in the warning.
> But this should work too and looks cleaner:
>
> cop = TREE_OPERAND (cop, 0);
>
> /* Get the outermost array. */
> while (TREE_CODE (cop) == ARRAY_REF)
> cop = TREE_OPERAND (cop, 0);
>
> /* Get the member (its address is never null). */
> if (TREE_CODE (cop) == COMPONENT_REF)
> cop = TREE_OPERAND (cop, 1);
>
> Do you prefer the above instead?
Sure. OK with that change and the comment tweak above.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 21:39 ` Jason Merrill
@ 2021-09-01 22:27 ` Martin Sebor
2021-09-02 13:43 ` Jason Merrill
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-01 22:27 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers
On 9/1/21 3:39 PM, Jason Merrill wrote:
> On 9/1/21 4:33 PM, Martin Sebor wrote:
>> On 9/1/21 1:21 PM, Jason Merrill wrote:
>>> On 8/31/21 10:08 PM, Martin Sebor wrote:
>>>> A Coverity run recently uncovered a latent bug in GCC that GCC should
>>>> be able to detect itself: comparing the address of a declared object
>>>> for equality to null, similar to:
>>>>
>>>> int f (void)
>>>> {
>>>> int a[2][2];
>>>> return &a == 0;
>>>> }
>>>>
>>>> GCC issues -Waddress for this code, but the bug Coverity found was
>>>> actually closer to the following:
>>>>
>>>> int f (void)
>>>> {
>>>> int a[2][2];
>>>> return a[0] == 0;
>>>> }
>>>>
>>>> where the hapless author (yours truly) meant to compare the value
>>>> of a[0][0] (as in r12-3268).
>>>>
>>>> This variant is not diagnosed even though the bug in it is the same
>>>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>>>> diagnosed either, though that's a less likely mistake to make).
>>>>
>>>> The attached patch enhances -Waddress to detect this variant along
>>>> with a number of other similar instances of the problem, including
>>>> comparing the address of array members to null.
>>>>
>>>> Besides these, the patch also issues -Waddress for null equality
>>>> tests of pointer-plus expressions such as in:
>>>>
>>>> int g (int i)
>>>> {
>>>> return a[0] + i == 0;
>>>> }
>>>>
>>>> and in C++ more instances of pointers to members.
>>>>
>>>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>>>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>>>> a test added after GCC for some reason stopped warning for one of
>>>> the basic cases that other tools warn about (comparing an array to
>>>> null). I suspect the change was unintentional because GCC still
>>>> warns for other very similar expressions. The reporter who also
>>>> submitted the test in pr36299 argued that the warning wasn't
>>>> helpful because tests for arrays sometimes come from macros, and
>>>> the test was committed after it was noted that GCC no longer warned
>>>> for the reporter's simple case. While it's certainly true that
>>>> the warning can be triggered by the null equality tests in macros
>>>> (the patch exposed two such instances in GCC) they are easy to
>>>> avoid (the patch adds a an additional escape hatch). At the same
>>>> time, as is evident from the Coverity bug report and from the two
>>>> issues the enhancement exposes in the FORTRAN front end (even if
>>>> benign), issuing the warning in these cases does help find bugs
>>>> or mistaken assumptions. With that, I've changed the test to
>>>> expect the restored -Waddress warning instead.
>>>>
>>>> Testing with Glibc exposed a couple of harmless comparisons of
>>>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>>>> to avoid the -Waddress instances if/when this enhancement is
>>>> approved.
>>>>
>>>> Testing with Binutils/GDB also turned up a couple of pointless
>>>> comparison of arrays to null and a couple of uses in macros that
>>>> can be trivially suppressed.
>>>>
>>>> Martin
>>>>
>>>> PS Clang issues a warning for some of the same null pointer tests
>>>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>>>> least three different options: some under -Wpointer-bool-conversion,
>>>> others under -Wtautological-pointer-compare, and others still under
>>>> -Wtautological-compare.
>>>
>>>> + while (TREE_CODE (cop) == ARRAY_REF
>>>> + || TREE_CODE (cop) == COMPONENT_REF)
>>>> + {
>>>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>>>> + cop = TREE_OPERAND (cop, opno);
>>>> + }
>>>
>>> 1) Maybe 'while (handled_component_p (cop))'?
>>> 2) Why handle COMPONENT_REF differently? Operand 1 is the
>>> FIELD_DECL, which doesn't have an address of its own.
>>
>> This is because the address of a field is never null, regardless of
>> what the P in in &P->M points to.
>
> True, though I'd change "invalid" to "undefined" in the comment for
> decl_with_nonnull_addr_p.
>
>> (With the caveat mentioned in
>> the comment further up about the pointer used to access the member
>> being nonnull.) So this is diagnosed:
>>
>> extern struct { int m; } *p;
>> bool b = &p->m == 0;
>>
>> Using handled_component_p() in a loop would prevent that.
>
> Would it? p isn't declared weak.
Maybe I misunderstood. This loop:
while (handled_component_p (cop))
cop = TREE_OPERAND (cop, 0);
would unwrap the COMPONENT_REF from cop and terminate with it set
to INDIRECT_REF for which decl_with_nonnull_addr_p() would return
false. But if you meant to keep the body as is and just change
the condition, that would work. If you think that's better,
e.g., because it would handle more cases, I'm all for it.
>
>> For array_refs, the loop gets us the decl to mention in the warning.
>> But this should work too and looks cleaner:
>>
>> cop = TREE_OPERAND (cop, 0);
>>
>> /* Get the outermost array. */
>> while (TREE_CODE (cop) == ARRAY_REF)
>> cop = TREE_OPERAND (cop, 0);
>>
>> /* Get the member (its address is never null). */
>> if (TREE_CODE (cop) == COMPONENT_REF)
>> cop = TREE_OPERAND (cop, 1);
>>
>> Do you prefer the above instead?
>
> Sure. OK with that change and the comment tweak above.
Thanks. Are you approving the whole patch (including the C FE
changes) or just the C++ part?
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-01 22:27 ` Martin Sebor
@ 2021-09-02 13:43 ` Jason Merrill
2021-09-02 14:39 ` Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2021-09-02 13:43 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers
On 9/1/21 6:27 PM, Martin Sebor wrote:
> On 9/1/21 3:39 PM, Jason Merrill wrote:
>> On 9/1/21 4:33 PM, Martin Sebor wrote:
>>> On 9/1/21 1:21 PM, Jason Merrill wrote:
>>>> On 8/31/21 10:08 PM, Martin Sebor wrote:
>>>>> A Coverity run recently uncovered a latent bug in GCC that GCC should
>>>>> be able to detect itself: comparing the address of a declared object
>>>>> for equality to null, similar to:
>>>>>
>>>>> int f (void)
>>>>> {
>>>>> int a[2][2];
>>>>> return &a == 0;
>>>>> }
>>>>>
>>>>> GCC issues -Waddress for this code, but the bug Coverity found was
>>>>> actually closer to the following:
>>>>>
>>>>> int f (void)
>>>>> {
>>>>> int a[2][2];
>>>>> return a[0] == 0;
>>>>> }
>>>>>
>>>>> where the hapless author (yours truly) meant to compare the value
>>>>> of a[0][0] (as in r12-3268).
>>>>>
>>>>> This variant is not diagnosed even though the bug in it is the same
>>>>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>>>>> diagnosed either, though that's a less likely mistake to make).
>>>>>
>>>>> The attached patch enhances -Waddress to detect this variant along
>>>>> with a number of other similar instances of the problem, including
>>>>> comparing the address of array members to null.
>>>>>
>>>>> Besides these, the patch also issues -Waddress for null equality
>>>>> tests of pointer-plus expressions such as in:
>>>>>
>>>>> int g (int i)
>>>>> {
>>>>> return a[0] + i == 0;
>>>>> }
>>>>>
>>>>> and in C++ more instances of pointers to members.
>>>>>
>>>>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>>>>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>>>>> a test added after GCC for some reason stopped warning for one of
>>>>> the basic cases that other tools warn about (comparing an array to
>>>>> null). I suspect the change was unintentional because GCC still
>>>>> warns for other very similar expressions. The reporter who also
>>>>> submitted the test in pr36299 argued that the warning wasn't
>>>>> helpful because tests for arrays sometimes come from macros, and
>>>>> the test was committed after it was noted that GCC no longer warned
>>>>> for the reporter's simple case. While it's certainly true that
>>>>> the warning can be triggered by the null equality tests in macros
>>>>> (the patch exposed two such instances in GCC) they are easy to
>>>>> avoid (the patch adds a an additional escape hatch). At the same
>>>>> time, as is evident from the Coverity bug report and from the two
>>>>> issues the enhancement exposes in the FORTRAN front end (even if
>>>>> benign), issuing the warning in these cases does help find bugs
>>>>> or mistaken assumptions. With that, I've changed the test to
>>>>> expect the restored -Waddress warning instead.
>>>>>
>>>>> Testing with Glibc exposed a couple of harmless comparisons of
>>>>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>>>>> to avoid the -Waddress instances if/when this enhancement is
>>>>> approved.
>>>>>
>>>>> Testing with Binutils/GDB also turned up a couple of pointless
>>>>> comparison of arrays to null and a couple of uses in macros that
>>>>> can be trivially suppressed.
>>>>>
>>>>> Martin
>>>>>
>>>>> PS Clang issues a warning for some of the same null pointer tests
>>>>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>>>>> least three different options: some under -Wpointer-bool-conversion,
>>>>> others under -Wtautological-pointer-compare, and others still under
>>>>> -Wtautological-compare.
>>>>
>>>>> + while (TREE_CODE (cop) == ARRAY_REF
>>>>> + || TREE_CODE (cop) == COMPONENT_REF)
>>>>> + {
>>>>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>>>>> + cop = TREE_OPERAND (cop, opno);
>>>>> + }
>>>>
>>>> 1) Maybe 'while (handled_component_p (cop))'?
>>>> 2) Why handle COMPONENT_REF differently? Operand 1 is the
>>>> FIELD_DECL, which doesn't have an address of its own.
>>>
>>> This is because the address of a field is never null, regardless of
>>> what the P in in &P->M points to.
>>
>> True, though I'd change "invalid" to "undefined" in the comment for
>> decl_with_nonnull_addr_p.
>>
>>> (With the caveat mentioned in
>>> the comment further up about the pointer used to access the member
>>> being nonnull.) So this is diagnosed:
>>>
>>> extern struct { int m; } *p;
>>> bool b = &p->m == 0;
>>>
>>> Using handled_component_p() in a loop would prevent that.
>>
>> Would it? p isn't declared weak.
>
> Maybe I misunderstood. This loop:
>
> while (handled_component_p (cop))
> cop = TREE_OPERAND (cop, 0);
>
> would unwrap the COMPONENT_REF from cop and terminate with it set
> to INDIRECT_REF for which decl_with_nonnull_addr_p() would return
> false. But if you meant to keep the body as is and just change
> the condition, that would work. If you think that's better,
> e.g., because it would handle more cases, I'm all for it.
It's worth considering the other codes in handled_component_p, at least.
BIT_FIELD_REF we shouldn't see an address of. I don't think the C++
front end produces ARRAY_RANGE_REF. I'd think REALPART_EXPR and
IMAGPART_EXPR would be like COMPONENT_REF. I guess we would want to
look through VIEW_CONVERT_EXPR.
>>> For array_refs, the loop gets us the decl to mention in the warning.
>>> But this should work too and looks cleaner:
>>>
>>> cop = TREE_OPERAND (cop, 0);
>>>
>>> /* Get the outermost array. */
>>> while (TREE_CODE (cop) == ARRAY_REF)
>>> cop = TREE_OPERAND (cop, 0);
>>>
>>> /* Get the member (its address is never null). */
>>> if (TREE_CODE (cop) == COMPONENT_REF)
>>> cop = TREE_OPERAND (cop, 1);
>>>
>>> Do you prefer the above instead?
>>
>> Sure. OK with that change and the comment tweak above.
>
> Thanks. Are you approving the whole patch (including the C FE
> changes) or just the C++ part?
My impression was that Jeff approved the rest of the patch, so I was
only really looking at the C++ part.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests
2021-09-02 13:43 ` Jason Merrill
@ 2021-09-02 14:39 ` Martin Sebor
2021-09-02 23:53 ` [PATCH] warn for more impossible null pointer tests [PR102103] Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-02 14:39 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers
On 9/2/21 7:43 AM, Jason Merrill wrote:
> On 9/1/21 6:27 PM, Martin Sebor wrote:
>> On 9/1/21 3:39 PM, Jason Merrill wrote:
>>> On 9/1/21 4:33 PM, Martin Sebor wrote:
>>>> On 9/1/21 1:21 PM, Jason Merrill wrote:
>>>>> On 8/31/21 10:08 PM, Martin Sebor wrote:
>>>>>> A Coverity run recently uncovered a latent bug in GCC that GCC should
>>>>>> be able to detect itself: comparing the address of a declared object
>>>>>> for equality to null, similar to:
>>>>>>
>>>>>> int f (void)
>>>>>> {
>>>>>> int a[2][2];
>>>>>> return &a == 0;
>>>>>> }
>>>>>>
>>>>>> GCC issues -Waddress for this code, but the bug Coverity found was
>>>>>> actually closer to the following:
>>>>>>
>>>>>> int f (void)
>>>>>> {
>>>>>> int a[2][2];
>>>>>> return a[0] == 0;
>>>>>> }
>>>>>>
>>>>>> where the hapless author (yours truly) meant to compare the value
>>>>>> of a[0][0] (as in r12-3268).
>>>>>>
>>>>>> This variant is not diagnosed even though the bug in it is the same
>>>>>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>>>>>> diagnosed either, though that's a less likely mistake to make).
>>>>>>
>>>>>> The attached patch enhances -Waddress to detect this variant along
>>>>>> with a number of other similar instances of the problem, including
>>>>>> comparing the address of array members to null.
>>>>>>
>>>>>> Besides these, the patch also issues -Waddress for null equality
>>>>>> tests of pointer-plus expressions such as in:
>>>>>>
>>>>>> int g (int i)
>>>>>> {
>>>>>> return a[0] + i == 0;
>>>>>> }
>>>>>>
>>>>>> and in C++ more instances of pointers to members.
>>>>>>
>>>>>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>>>>>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>>>>>> a test added after GCC for some reason stopped warning for one of
>>>>>> the basic cases that other tools warn about (comparing an array to
>>>>>> null). I suspect the change was unintentional because GCC still
>>>>>> warns for other very similar expressions. The reporter who also
>>>>>> submitted the test in pr36299 argued that the warning wasn't
>>>>>> helpful because tests for arrays sometimes come from macros, and
>>>>>> the test was committed after it was noted that GCC no longer warned
>>>>>> for the reporter's simple case. While it's certainly true that
>>>>>> the warning can be triggered by the null equality tests in macros
>>>>>> (the patch exposed two such instances in GCC) they are easy to
>>>>>> avoid (the patch adds a an additional escape hatch). At the same
>>>>>> time, as is evident from the Coverity bug report and from the two
>>>>>> issues the enhancement exposes in the FORTRAN front end (even if
>>>>>> benign), issuing the warning in these cases does help find bugs
>>>>>> or mistaken assumptions. With that, I've changed the test to
>>>>>> expect the restored -Waddress warning instead.
>>>>>>
>>>>>> Testing with Glibc exposed a couple of harmless comparisons of
>>>>>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>>>>>> to avoid the -Waddress instances if/when this enhancement is
>>>>>> approved.
>>>>>>
>>>>>> Testing with Binutils/GDB also turned up a couple of pointless
>>>>>> comparison of arrays to null and a couple of uses in macros that
>>>>>> can be trivially suppressed.
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>> PS Clang issues a warning for some of the same null pointer tests
>>>>>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>>>>>> least three different options: some under -Wpointer-bool-conversion,
>>>>>> others under -Wtautological-pointer-compare, and others still under
>>>>>> -Wtautological-compare.
>>>>>
>>>>>> + while (TREE_CODE (cop) == ARRAY_REF
>>>>>> + || TREE_CODE (cop) == COMPONENT_REF)
>>>>>> + {
>>>>>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>>>>>> + cop = TREE_OPERAND (cop, opno);
>>>>>> + }
>>>>>
>>>>> 1) Maybe 'while (handled_component_p (cop))'?
>>>>> 2) Why handle COMPONENT_REF differently? Operand 1 is the
>>>>> FIELD_DECL, which doesn't have an address of its own.
>>>>
>>>> This is because the address of a field is never null, regardless of
>>>> what the P in in &P->M points to.
>>>
>>> True, though I'd change "invalid" to "undefined" in the comment for
>>> decl_with_nonnull_addr_p.
>>>
>>>> (With the caveat mentioned in
>>>> the comment further up about the pointer used to access the member
>>>> being nonnull.) So this is diagnosed:
>>>>
>>>> extern struct { int m; } *p;
>>>> bool b = &p->m == 0;
>>>>
>>>> Using handled_component_p() in a loop would prevent that.
>>>
>>> Would it? p isn't declared weak.
>>
>> Maybe I misunderstood. This loop:
>>
>> while (handled_component_p (cop))
>> cop = TREE_OPERAND (cop, 0);
>>
>> would unwrap the COMPONENT_REF from cop and terminate with it set
>> to INDIRECT_REF for which decl_with_nonnull_addr_p() would return
>> false. But if you meant to keep the body as is and just change
>> the condition, that would work. If you think that's better,
>> e.g., because it would handle more cases, I'm all for it.
>
> It's worth considering the other codes in handled_component_p, at least.
> BIT_FIELD_REF we shouldn't see an address of. I don't think the C++
> front end produces ARRAY_RANGE_REF. I'd think REALPART_EXPR and
> IMAGPART_EXPR would be like COMPONENT_REF. I guess we would want to
> look through VIEW_CONVERT_EXPR.
Okay, let me update the patch then and repost before committing.
>
>>>> For array_refs, the loop gets us the decl to mention in the warning.
>>>> But this should work too and looks cleaner:
>>>>
>>>> cop = TREE_OPERAND (cop, 0);
>>>>
>>>> /* Get the outermost array. */
>>>> while (TREE_CODE (cop) == ARRAY_REF)
>>>> cop = TREE_OPERAND (cop, 0);
>>>>
>>>> /* Get the member (its address is never null). */
>>>> if (TREE_CODE (cop) == COMPONENT_REF)
>>>> cop = TREE_OPERAND (cop, 1);
>>>>
>>>> Do you prefer the above instead?
>>>
>>> Sure. OK with that change and the comment tweak above.
>>
>> Thanks. Are you approving the whole patch (including the C FE
>> changes) or just the C++ part?
>
> My impression was that Jeff approved the rest of the patch, so I was
> only really looking at the C++ part.
That's odd (and worrisome). I didn't get that email for some
strange reason but I just found it in the archive. Let me also
respond to Jeff's question in my follow up.
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-02 14:39 ` Martin Sebor
@ 2021-09-02 23:53 ` Martin Sebor
2021-09-08 20:06 ` Jason Merrill
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-02 23:53 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 8638 bytes --]
Attached is an updated patch with Jason's suggested change to use
handled_component_p(), retested on x86_64-linux and with Glibc.
Adding more tests led to more changes but hopefully also a better
end result.
I've changed the warning suppression from a cast to void* to one
to intptr_t, in part because that's what Clang does, and in part
because I couldn't get it to work consistently between C and C++
(the C front end seems to introduce a NOP_EXPR in some cases even
when there is no cast in the source). In hindsight, a cast to
void* doesn't seem like the most intuitive way to avoid this sort
of warning.
Jeff, I didn't get your reply to my first post for some reason so
let me copy your question here and answer it below:
how does this interact with targets that allow objects at address
0? We have a few targets like that and that makes me wonder if
we should be suppressing some, if not all, of these warnings for
targets that turn on -fno-delete-null-pointer-checks?
I didn't see any code related to -Waddress that tries to handle those
targets. I have very little experience with them but I do know that
AVR makes it possible to pin an object to a fixed address by means
of attribute address. The test case below triggers -Waddress (both
with and without my changes):
__attribute__ ((address (0))) int x;
int f (void)
{
if (&x == 0) return -1; // -Waddress
int *p = &x;
int t = *p;
if (!p) __builtin_abort (); // folded to false
return t;
}
But the optimized code doesn't have the second test so I'm not sure
that the address attribute here does what I think it does (I'd
expect the test to be folded to true). If you have a better test
case or other targets for me to try I can look into it some more.
Martin
On 9/2/21 8:39 AM, Martin Sebor wrote:
> On 9/2/21 7:43 AM, Jason Merrill wrote:
>> On 9/1/21 6:27 PM, Martin Sebor wrote:
>>> On 9/1/21 3:39 PM, Jason Merrill wrote:
>>>> On 9/1/21 4:33 PM, Martin Sebor wrote:
>>>>> On 9/1/21 1:21 PM, Jason Merrill wrote:
>>>>>> On 8/31/21 10:08 PM, Martin Sebor wrote:
>>>>>>> A Coverity run recently uncovered a latent bug in GCC that GCC
>>>>>>> should
>>>>>>> be able to detect itself: comparing the address of a declared object
>>>>>>> for equality to null, similar to:
>>>>>>>
>>>>>>> int f (void)
>>>>>>> {
>>>>>>> int a[2][2];
>>>>>>> return &a == 0;
>>>>>>> }
>>>>>>>
>>>>>>> GCC issues -Waddress for this code, but the bug Coverity found was
>>>>>>> actually closer to the following:
>>>>>>>
>>>>>>> int f (void)
>>>>>>> {
>>>>>>> int a[2][2];
>>>>>>> return a[0] == 0;
>>>>>>> }
>>>>>>>
>>>>>>> where the hapless author (yours truly) meant to compare the value
>>>>>>> of a[0][0] (as in r12-3268).
>>>>>>>
>>>>>>> This variant is not diagnosed even though the bug in it is the same
>>>>>>> and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
>>>>>>> diagnosed either, though that's a less likely mistake to make).
>>>>>>>
>>>>>>> The attached patch enhances -Waddress to detect this variant along
>>>>>>> with a number of other similar instances of the problem, including
>>>>>>> comparing the address of array members to null.
>>>>>>>
>>>>>>> Besides these, the patch also issues -Waddress for null equality
>>>>>>> tests of pointer-plus expressions such as in:
>>>>>>>
>>>>>>> int g (int i)
>>>>>>> {
>>>>>>> return a[0] + i == 0;
>>>>>>> }
>>>>>>>
>>>>>>> and in C++ more instances of pointers to members.
>>>>>>>
>>>>>>> Testing on x86_64-linux, besides a few benign issues in GCC sources
>>>>>>> a regression test, run shows a failure in gcc.dg/Waddress.c. That's
>>>>>>> a test added after GCC for some reason stopped warning for one of
>>>>>>> the basic cases that other tools warn about (comparing an array to
>>>>>>> null). I suspect the change was unintentional because GCC still
>>>>>>> warns for other very similar expressions. The reporter who also
>>>>>>> submitted the test in pr36299 argued that the warning wasn't
>>>>>>> helpful because tests for arrays sometimes come from macros, and
>>>>>>> the test was committed after it was noted that GCC no longer warned
>>>>>>> for the reporter's simple case. While it's certainly true that
>>>>>>> the warning can be triggered by the null equality tests in macros
>>>>>>> (the patch exposed two such instances in GCC) they are easy to
>>>>>>> avoid (the patch adds a an additional escape hatch). At the same
>>>>>>> time, as is evident from the Coverity bug report and from the two
>>>>>>> issues the enhancement exposes in the FORTRAN front end (even if
>>>>>>> benign), issuing the warning in these cases does help find bugs
>>>>>>> or mistaken assumptions. With that, I've changed the test to
>>>>>>> expect the restored -Waddress warning instead.
>>>>>>>
>>>>>>> Testing with Glibc exposed a couple of harmless comparisons of
>>>>>>> arrays a large macro in vfprintf-internal.c. I'll submit a fix
>>>>>>> to avoid the -Waddress instances if/when this enhancement is
>>>>>>> approved.
>>>>>>>
>>>>>>> Testing with Binutils/GDB also turned up a couple of pointless
>>>>>>> comparison of arrays to null and a couple of uses in macros that
>>>>>>> can be trivially suppressed.
>>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>> PS Clang issues a warning for some of the same null pointer tests
>>>>>>> the patch diagnoses, including gcc.dg/Waddress.c, except under at
>>>>>>> least three different options: some under -Wpointer-bool-conversion,
>>>>>>> others under -Wtautological-pointer-compare, and others still under
>>>>>>> -Wtautological-compare.
>>>>>>
>>>>>>> + while (TREE_CODE (cop) == ARRAY_REF
>>>>>>> + || TREE_CODE (cop) == COMPONENT_REF)
>>>>>>> + {
>>>>>>> + unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
>>>>>>> + cop = TREE_OPERAND (cop, opno);
>>>>>>> + }
>>>>>>
>>>>>> 1) Maybe 'while (handled_component_p (cop))'?
>>>>>> 2) Why handle COMPONENT_REF differently? Operand 1 is the
>>>>>> FIELD_DECL, which doesn't have an address of its own.
>>>>>
>>>>> This is because the address of a field is never null, regardless of
>>>>> what the P in in &P->M points to.
>>>>
>>>> True, though I'd change "invalid" to "undefined" in the comment for
>>>> decl_with_nonnull_addr_p.
>>>>
>>>>> (With the caveat mentioned in
>>>>> the comment further up about the pointer used to access the member
>>>>> being nonnull.) So this is diagnosed:
>>>>>
>>>>> extern struct { int m; } *p;
>>>>> bool b = &p->m == 0;
>>>>>
>>>>> Using handled_component_p() in a loop would prevent that.
>>>>
>>>> Would it? p isn't declared weak.
>>>
>>> Maybe I misunderstood. This loop:
>>>
>>> while (handled_component_p (cop))
>>> cop = TREE_OPERAND (cop, 0);
>>>
>>> would unwrap the COMPONENT_REF from cop and terminate with it set
>>> to INDIRECT_REF for which decl_with_nonnull_addr_p() would return
>>> false. But if you meant to keep the body as is and just change
>>> the condition, that would work. If you think that's better,
>>> e.g., because it would handle more cases, I'm all for it.
>>
>> It's worth considering the other codes in handled_component_p, at
>> least. BIT_FIELD_REF we shouldn't see an address of. I don't think
>> the C++ front end produces ARRAY_RANGE_REF. I'd think REALPART_EXPR
>> and IMAGPART_EXPR would be like COMPONENT_REF. I guess we would want
>> to look through VIEW_CONVERT_EXPR.
>
> Okay, let me update the patch then and repost before committing.
>
>>
>>>>> For array_refs, the loop gets us the decl to mention in the warning.
>>>>> But this should work too and looks cleaner:
>>>>>
>>>>> cop = TREE_OPERAND (cop, 0);
>>>>>
>>>>> /* Get the outermost array. */
>>>>> while (TREE_CODE (cop) == ARRAY_REF)
>>>>> cop = TREE_OPERAND (cop, 0);
>>>>>
>>>>> /* Get the member (its address is never null). */
>>>>> if (TREE_CODE (cop) == COMPONENT_REF)
>>>>> cop = TREE_OPERAND (cop, 1);
>>>>>
>>>>> Do you prefer the above instead?
>>>>
>>>> Sure. OK with that change and the comment tweak above.
>>>
>>> Thanks. Are you approving the whole patch (including the C FE
>>> changes) or just the C++ part?
>>
>> My impression was that Jeff approved the rest of the patch, so I was
>> only really looking at the C++ part.
>
> That's odd (and worrisome). I didn't get that email for some
> strange reason but I just found it in the archive. Let me also
> respond to Jeff's question in my follow up.
>
> Martin
[-- Attachment #2: gcc-102103.diff --]
[-- Type: text/x-patch, Size: 35088 bytes --]
Enhance -Waddress to detect more suspicious expressions [PR102103].
Resolves:
PR c/102103 - missing warning comparing array address to null
gcc/ChangeLog:
* doc/invoke.texi (-Waddress): Update.
* gcc/gengtype.c (write_types): Avoid -Waddress.
gcc/c-family/ChangeLog:
* c-common.c (decl_with_nonnull_addr_p): Handle members.
Check and perform warning suppression.
(c_common_truthvalue_conversion): Enhance warning suppression.
gcc/c/ChangeLog:
* c-typeck.c (maybe_warn_for_null_address): New function.
(build_binary_op): Call it.
gcc/cp/ChangeLog:
* typeck.c (warn_for_null_address): Enhance.
(cp_build_binary_op): Call it also for member pointers.
gcc/fortran/ChangeLog:
* gcc/fortran/array.c: Remove an unnecessary test.
* gcc/fortran/trans-array.c: Same.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-array-ptr10.C: Suppress a valid warning.
* g++.dg/warn/Wreturn-local-addr-6.C: Correct a cast.
* gcc.dg/Waddress.c: Expect a warning.
* c-c++-common/Waddress-3.c: New test.
* c-c++-common/Waddress-4.c: New test.
* g++.dg/warn/Waddress-5.C: New test.
* g++.dg/warn/Waddress-6.C: New test.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 017e41537ac..50d9559175e 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3393,14 +3393,16 @@ c_wrap_maybe_const (tree expr, bool non_const)
return expr;
}
-/* Return whether EXPR is a declaration whose address can never be
- NULL. */
+/* Return whether EXPR is a declaration whose address can never be NULL.
+ The address of the first struct member could be NULL only if it were
+ accessed through a NULL pointer, and such an access would be invalid. */
bool
decl_with_nonnull_addr_p (const_tree expr)
{
return (DECL_P (expr)
- && (TREE_CODE (expr) == PARM_DECL
+ && (TREE_CODE (expr) == FIELD_DECL
+ || TREE_CODE (expr) == PARM_DECL
|| TREE_CODE (expr) == LABEL_DECL
|| !DECL_WEAK (expr)));
}
@@ -3488,13 +3490,17 @@ c_common_truthvalue_conversion (location_t location, tree expr)
case ADDR_EXPR:
{
tree inner = TREE_OPERAND (expr, 0);
- if (decl_with_nonnull_addr_p (inner))
+ if (decl_with_nonnull_addr_p (inner)
+ /* Check both EXPR and INNER for suppression. */
+ && !warning_suppressed_p (expr, OPT_Waddress)
+ && !warning_suppressed_p (inner, OPT_Waddress))
{
- /* Common Ada programmer's mistake. */
+ /* Common Ada programmer's mistake. */
warning_at (location,
OPT_Waddress,
"the address of %qD will always evaluate as %<true%>",
inner);
+ suppress_warning (inner, OPT_Waddress);
return truthvalue_true_node;
}
break;
@@ -3627,8 +3633,17 @@ c_common_truthvalue_conversion (location_t location, tree expr)
break;
/* If this isn't narrowing the argument, we can ignore it. */
if (TYPE_PRECISION (totype) >= TYPE_PRECISION (fromtype))
- return c_common_truthvalue_conversion (location,
- TREE_OPERAND (expr, 0));
+ {
+ tree op0 = TREE_OPERAND (expr, 0);
+ if ((TREE_CODE (fromtype) == POINTER_TYPE
+ && TREE_CODE (totype) == INTEGER_TYPE)
+ || warning_suppressed_p (expr, OPT_Waddress))
+ /* Suppress -Waddress for casts to intptr_t, propagating
+ any suppression from the enclosing expression to its
+ operand. */
+ suppress_warning (op0, OPT_Waddress);
+ return c_common_truthvalue_conversion (location, op0);
+ }
}
break;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5849c5ab23d..0e9cf1063c0 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -11545,6 +11545,110 @@ build_vec_cmp (tree_code code, tree type,
return build3 (VEC_COND_EXPR, type, cmp, minus_one_vec, zero_vec);
}
+/* Possibly warn about an address of OP never being NULL in a comparison
+ operation CODE involving null. */
+
+static void
+maybe_warn_for_null_address (location_t loc, tree op, tree_code code)
+{
+ if (!warn_address || warning_suppressed_p (op, OPT_Waddress))
+ return;
+
+ if (TREE_CODE (op) == NOP_EXPR)
+ {
+ /* Allow casts to intptr_t to suppress the warning. */
+ tree type = TREE_TYPE (op);
+ if (TREE_CODE (type) == INTEGER_TYPE)
+ return;
+ op = TREE_OPERAND (op, 0);
+ }
+
+ if (TREE_CODE (op) == POINTER_PLUS_EXPR)
+ {
+ /* Allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (op));
+ if (VOID_TYPE_P (type))
+ return;
+
+ /* Adding any value to a null pointer, including zero, is undefined
+ in C. This includes the expression &p[0] where p is the null
+ pointer, although &p[0] will have been folded to p by this point
+ and so not diagnosed. */
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+
+ return;
+ }
+
+ if (TREE_CODE (op) != ADDR_EXPR)
+ return;
+
+ op = TREE_OPERAND (op, 0);
+
+ if (TREE_CODE (op) == IMAGPART_EXPR
+ || TREE_CODE (op) == REALPART_EXPR)
+ {
+ /* The address of either complex part may not be null. */
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the address %qE will never be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the address %qE will never be NULL",
+ op);
+ return;
+ }
+
+ /* Set to true in the loop below if OP dereferences is operand.
+ In such a case the ultimate target need not be a decl for
+ the null [in]equality test to be constant. */
+ bool deref = false;
+
+ /* Get the outermost array or object, or member. */
+ while (handled_component_p (op))
+ {
+ if (TREE_CODE (op) == COMPONENT_REF)
+ {
+ /* Get the member (its address is never null). */
+ op = TREE_OPERAND (op, 1);
+ break;
+ }
+
+ /* Get the outer array/object to refer to in the warning. */
+ op = TREE_OPERAND (op, 0);
+ deref = true;
+ }
+
+ if ((!deref && !decl_with_nonnull_addr_p (op))
+ || from_macro_expansion_at (loc))
+ return;
+
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the address of %qE will never be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the address of %qE will never be NULL",
+ op);
+
+ if (DECL_P (op))
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
+}
+
/* Build a binary-operation expression without default conversions.
CODE is the kind of expression to build.
LOCATION is the operator's location.
@@ -12180,44 +12284,12 @@ build_binary_op (location_t location, enum tree_code code,
short_compare = 1;
else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
{
- if (TREE_CODE (op0) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- }
+ maybe_warn_for_null_address (location, op0, code);
result_type = type0;
}
else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
{
- if (TREE_CODE (op1) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- }
+ maybe_warn_for_null_address (location, op1, code);
result_type = type1;
}
else if (code0 == POINTER_TYPE && code1 == POINTER_TYPE)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cc61b509f8a..293c292d9b2 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
if (!warn_address
|| (complain & tf_warning) == 0
|| c_inhibit_evaluation_warnings != 0
- || warning_suppressed_p (op, OPT_Waddress))
+ || warning_suppressed_p (op, OPT_Waddress)
+ || processing_template_decl != 0)
return;
tree cop = fold_for_warn (op);
- if (TREE_CODE (cop) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (cop, 0))
- && !warning_suppressed_p (cop, OPT_Waddress))
- warning_at (location, OPT_Waddress, "the address of %qD will never "
- "be NULL", TREE_OPERAND (cop, 0));
+ if (TREE_CODE (cop) == NON_LVALUE_EXPR)
+ /* Unwrap the expression for C++ 98. */
+ cop = TREE_OPERAND (cop, 0);
- if (CONVERT_EXPR_P (op)
+ if (TREE_CODE (cop) == PTRMEM_CST)
+ {
+ /* The address of a nonstatic data member is never null. */
+ warning_at (location, OPT_Waddress,
+ "the address %qE will never be NULL",
+ cop);
+ return;
+ }
+
+ if (TREE_CODE (cop) == NOP_EXPR)
+ {
+ /* Allow casts to intptr_t to suppress the warning. */
+ tree type = TREE_TYPE (cop);
+ if (TREE_CODE (type) == INTEGER_TYPE)
+ return;
+
+ STRIP_NOPS (cop);
+ }
+
+ bool warned = false;
+ if (TREE_CODE (cop) == ADDR_EXPR)
+ {
+ cop = TREE_OPERAND (cop, 0);
+
+ /* Set to true in the loop below if OP dereferences its operand.
+ In such a case the ultimate target need not be a decl for
+ the null [in]equality test to be necessarily constant. */
+ bool deref = false;
+
+ /* Get the outermost array or object, or member. */
+ while (handled_component_p (cop))
+ {
+ if (TREE_CODE (cop) == COMPONENT_REF)
+ {
+ /* Get the member (its address is never null). */
+ cop = TREE_OPERAND (cop, 1);
+ break;
+ }
+
+ /* Get the outer array/object to refer to in the warning. */
+ cop = TREE_OPERAND (cop, 0);
+ deref = true;
+ }
+
+ if ((!deref && !decl_with_nonnull_addr_p (cop))
+ || from_macro_expansion_at (location)
+ || warning_suppressed_p (cop, OPT_Waddress))
+ return;
+
+ warned = warning_at (location, OPT_Waddress,
+ "the address of %qD will never be NULL", cop);
+ op = cop;
+ }
+ else if (TREE_CODE (cop) == POINTER_PLUS_EXPR)
+ {
+ /* Adding zero to the null pointer is well-defined in C++. When
+ the offset is unknown (i.e., not a constant) warn anyway since
+ it's less likely that the pointer operand is null than not. */
+ tree off = TREE_OPERAND (cop, 1);
+ if (!integer_zerop (off)
+ && !warning_suppressed_p (cop, OPT_Waddress))
+ warning_at (location, OPT_Waddress, "comparing the result of pointer "
+ "addition %qE and NULL", cop);
+ return;
+ }
+ else if (CONVERT_EXPR_P (op)
&& TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
{
- tree inner_op = op;
- STRIP_NOPS (inner_op);
+ STRIP_NOPS (op);
- if (DECL_P (inner_op))
- warning_at (location, OPT_Waddress,
- "the compiler can assume that the address of "
- "%qD will never be NULL", inner_op);
+ if (DECL_P (op))
+ warned = warning_at (location, OPT_Waddress,
+ "the compiler can assume that the address of "
+ "%qD will never be NULL", op);
}
+
+ if (warned && DECL_P (op))
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
}
/* Warn about [expr.arith.conv]/2: If one operand is of enumeration type and
@@ -5433,6 +5499,8 @@ cp_build_binary_op (const op_location_t &location,
op1 = cp_convert (TREE_TYPE (op0), op1, complain);
}
result_type = TREE_TYPE (op0);
+
+ warn_for_null_address (location, orig_op0, complain);
}
else if (TYPE_PTRMEMFUNC_P (type1) && null_ptr_cst_p (orig_op0))
return cp_build_binary_op (location, code, op1, op0, complain);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8969bac664d..cba5fefb027 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8547,17 +8547,43 @@ by @option{-Wall}.
@item -Waddress
@opindex Waddress
@opindex Wno-address
-Warn about suspicious uses of memory addresses. These include using
-the address of a function in a conditional expression, such as
-@code{void func(void); if (func)}, and comparisons against the memory
-address of a string literal, such as @code{if (x == "abc")}. Such
-uses typically indicate a programmer error: the address of a function
-always evaluates to true, so their use in a conditional usually
-indicate that the programmer forgot the parentheses in a function
-call; and comparisons against string literals result in unspecified
-behavior and are not portable in C, so they usually indicate that the
-programmer intended to use @code{strcmp}. This warning is enabled by
-@option{-Wall}.
+Warn about suspicious uses of address expressions. These include comparing
+the address of a function or a declared object to the null pointer constant
+such as in
+@smallexample
+void f (void);
+void g (void)
+@{
+ if (!func) // warning: expression evaluates to false
+ abort ();
+@}
+@end smallexample
+comparisons of a pointer to a string literal, such as in
+@smallexample
+void f (const char *x)
+@{
+ if (x == "abc") // warning: expression evaluates to false
+ puts ("equal");
+@}
+@end smallexample
+and tests of the results of pointer addition or subtraction for equality
+to null, such as in
+@smallexample
+void f (const int *p, int i)
+@{
+ return p + i == NULL;
+@}
+@end smallexample
+Such uses typically indicate a programmer error: the address of most
+functions and objects necessarily evaluates to true (the exception are
+weak symbols), so their use in a conditional might indicate missing
+parentheses in a function call or a missing dereference in an array
+expression. The subset of the warning for object pointers can be
+suppressed by casting the pointer operand to an integer type such
+as @code{inptr_t} or @code{uinptr_t}.
+Comparisons against string literals result in unspecified behavior
+and are not portable, and suggest the intent was to call @code{strcmp}.
+@option{-Waddress} warning is enabled by @option{-Wall}.
@item -Wno-address-of-packed-member
@opindex Waddress-of-packed-member
diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index b858bada18a..341290f91b8 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -2578,7 +2578,7 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t *result)
}
}
- if (array->shape && array->shape[dimen])
+ if (array->shape)
{
mpz_init_set (*result, array->shape[dimen]);
return true;
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0d013defdbb..5147509fbe7 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5104,7 +5104,6 @@ set_loop_bounds (gfc_loopinfo *loop)
if (info->shape)
{
- gcc_assert (info->shape[dim]);
/* The frontend has worked out the size for us. */
if (!loopspec[n]
|| !specinfo->shape
diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 31d4bf4e5d0..a77cfd92bfa 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -3685,8 +3685,8 @@ write_types (outf_p output_header, type_p structures,
output_mangled_typename (output_header, s);
oprintf (output_header, "(X) do { \\\n");
oprintf (output_header,
- " if (X != NULL) gt_%sx_%s (X);\\\n", wtd->prefix,
- s_id_for_tag);
+ " if ((intptr_t)(X) != 0) gt_%sx_%s (X);\\\n",
+ wtd->prefix, s_id_for_tag);
oprintf (output_header, " } while (0)\n");
for (opt = s->u.s.opt; opt; opt = opt->next)
diff --git a/gcc/poly-int.h b/gcc/poly-int.h
index f47f9e436a8..94e7b701f64 100644
--- a/gcc/poly-int.h
+++ b/gcc/poly-int.h
@@ -324,10 +324,10 @@ struct poly_result<T1, T2, 2>
routine can take the address of RES rather than the address of
a temporary.
- The dummy comparison against a null C * is just a way of checking
+ The dummy self-comparison against C * is just a way of checking
that C gives the right type. */
#define POLY_SET_COEFF(C, RES, I, VALUE) \
- ((void) (&(RES).coeffs[0] == (C *) 0), \
+ ((void) (&(RES).coeffs[0] == (C *) (void *) &(RES).coeffs[0]), \
wi::int_traits<C>::precision_type == wi::FLEXIBLE_PRECISION \
? (void) ((RES).coeffs[I] = VALUE) \
: (void) ((RES).coeffs[I].~C (), new (&(RES).coeffs[I]) C (VALUE)))
diff --git a/gcc/testsuite/c-c++-common/Waddress-3.c b/gcc/testsuite/c-c++-common/Waddress-3.c
new file mode 100644
index 00000000000..9a13a444045
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-3.c
@@ -0,0 +1,125 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+struct S { void *p, *a1[2], *a2[2][2]; } s, *p;
+
+extern const void *a1[2];
+extern void *a2[2][2], *ax[];
+
+void T (bool);
+
+void test_array_eq_0 (int i)
+{
+ // Verify that casts intptr_t suppress the warning.
+ T ((intptr_t)a1 == 0);
+ T ((uintptr_t)a1 == 0);
+ T (a1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1); // { dg-warning "-Waddress" }
+ // Verify that casts to other pointer types don't suppress it.
+ T ((void *)a1 == 0); // { dg-warning "-Waddress" }
+ T ((char *)a1 == 0); // { dg-warning "-Waddress" }
+ T (a1[0] == 0);
+ T (0 == (intptr_t)&a1[0]);
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (a1[i] == 0);
+ T (0 == (uintptr_t)&a1[i]);
+ T (0 == &a1[i]); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)a2 == 0);
+ T (a2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2); // { dg-warning "-Waddress" }
+ T (a2[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (a2[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[i]); // { dg-warning "-Waddress" }
+ T (a2[0][0] == 0);
+ T (0 == &a2[0][0]); // { dg-warning "-Waddress" }
+ T (&ax == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax); // { dg-warning "-Waddress" }
+ T (&ax[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == ax[0]);
+}
+
+
+void test_array_neq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((uintptr_t)a1);
+
+ T (a1); // { dg-warning "-Waddress" }
+ T ((void *)a1); // { dg-warning "-Waddress" }
+ T (&a1 != 0); // { dg-warning "-Waddress" }
+ T (a1[0]);
+ T (&a1[0] != 0); // { dg-warning "-Waddress" }
+ T (a1[i]);
+ T (&a1[i] != 0); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)a2);
+ T (a2); // { dg-warning "-Waddress" }
+ T ((void *)a2); // { dg-warning "-Waddress" }
+ T ((char *)a2); // { dg-warning "-Waddress" }
+ T (&a2 != 0); // { dg-warning "-Waddress" }
+ T (a2[0]); // { dg-warning "-Waddress" }
+ T (&a1[0] != 0); // { dg-warning "-Waddress" }
+ T (a2[i]); // { dg-warning "-Waddress" }
+ T (&a2[i] != 0); // { dg-warning "-Waddress" }
+ T (a2[0][0]);
+ T (&a2[0][0] != 0); // { dg-warning "-Waddress" }
+}
+
+
+void test_member_array_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)s.a1 == 0);
+ T (s.a1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1); // { dg-warning "-Waddress" }
+ T (s.a1[0] == 0);
+ T ((void*)s.a1); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (s.a1[i] == 0);
+ T (0 == &a1[i]); // { dg-warning "-Waddress" }
+
+ T ((uintptr_t)s.a2 == 0);
+ T (s.a2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2); // { dg-warning "-Waddress" }
+ T ((void *)s.a2 == 0);// { dg-warning "-Waddress" }
+ T (s.a2[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (s.a2[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[i]); // { dg-warning "-Waddress" }
+ T (s.a2[0][0] == 0);
+ T (0 == &a2[0][0]); // { dg-warning "-Waddress" }
+}
+
+
+void test_member_array_neq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((uintptr_t)s.a1);
+ T (s.a1); // { dg-warning "-Waddress" }
+ T (&s.a1 != 0); // { dg-warning "-Waddress" }
+ T ((void *)&s.a1[0]); // { dg-warning "-Waddress" }
+ T (s.a1[0]);
+ T (&s.a1[0] != 0); // { dg-warning "-Waddress" }
+ T (s.a1[i]);
+ T (&s.a1[i] != 0); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)s.a2);
+ T (s.a2); // { dg-warning "-Waddress" }
+ T (&s.a2 != 0); // { dg-warning "-Waddress" }
+ T (s.a2[0]); // { dg-warning "-Waddress" }
+ T (&s.a1[0] != 0); // { dg-warning "-Waddress" }
+ T (s.a2[i]); // { dg-warning "-Waddress" }
+ T (&s.a2[i] != 0); // { dg-warning "-Waddress" }
+ T (s.a2[0][0]);
+ T (&s.a2[0][0] != 0); // { dg-warning "-Waddress" }
+}
diff --git a/gcc/testsuite/c-c++-common/Waddress-4.c b/gcc/testsuite/c-c++-common/Waddress-4.c
new file mode 100644
index 00000000000..7b32503f84e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-4.c
@@ -0,0 +1,106 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __INTPTR_TYPE__ uintptr_t;
+
+extern char *ax[], *a2[][2];
+
+void T (int);
+
+void test_ax_plus_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)(ax + 0) == 0);
+ T ((uintptr_t)(ax + 1) == 0);
+
+ T (ax + 0 == 0); // { dg-warning "-Waddress" }
+ T (&ax[0] == 0); // { dg-warning "-Waddress" }
+ T (ax - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[-1]); // { dg-warning "-Waddress" }
+ T ((void *)(&ax[0] + 2) == 0); // { dg-warning "-Waddress" }
+ T (&ax[0] + 2 == 0); // { dg-warning "-Waddress" }
+ T (ax + 3 == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[-4]); // { dg-warning "-Waddress" }
+ T (ax - i == 0); // { dg-warning "-Waddress" }
+ T (&ax[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[1] + i); // { dg-warning "-Waddress" }
+}
+
+void test_a2_plus_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)(a2 + 0) == 0);
+ T ((uintptr_t)(a2 + 1) == 0);
+
+ T (a2 + 0 == 0); // { dg-warning "-Waddress" }
+ // Verify that a cast to another pointer type doesn't suppress it.
+ T ((void*)(a2 + 0) == 0); // { dg-warning "-Waddress" }
+ T ((char*)a2 + 1 == 0); // { dg-warning "-Waddress" }
+ T (&a2[0] == 0); // { dg-warning "-Waddress" }
+ T (a2 - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[-1]); // { dg-warning "-Waddress" }
+ T (a2 + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[-2]); // { dg-warning "-Waddress" }
+ T (a2 - i == 0); // { dg-warning "-Waddress" }
+ T (&a2[i] == 0); // { dg-warning "-Waddress" }
+}
+
+// Exercise a pointer.
+void test_p_plus_eq_0 (int *p, int i)
+{
+ /* P + 0 and equivalently &P[0] are invalid for a null P but they're
+ folded to p before the warning has a chance to trigger. */
+ T (p + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&p[0] == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+
+ T (p - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &p[-1]); // { dg-warning "-Waddress" }
+ T (p + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &p[-2]); // { dg-warning "-Waddress" }
+ T (p - i == 0); // { dg-warning "-Waddress" }
+ T (&p[i] == 0); // { dg-warning "-Waddress" }
+}
+
+// Exercise pointer to array.
+void test_pa_plus_eq_0 (int (*p)[], int (*p2)[][2], int i)
+{
+ // The array pointer may be null.
+ T (*p == 0);
+ /* &**P is equivalent to *P and might be the result od macro expansion.
+ Verify it doesn't cause a warning. */
+ T (&**p == 0);
+
+ /* *P + 0 is invalid but folded to *P before the warning has a chance
+ to trigger. */
+ T (*p + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+
+ T (&(*p)[0] == 0); // { dg-warning "-Waddress" }
+ T (*p - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p)[-1]); // { dg-warning "-Waddress" }
+ T (*p + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p)[-2]); // { dg-warning "-Waddress" }
+ T (*p - i == 0); // { dg-warning "-Waddress" }
+ T (&(*p)[i] == 0); // { dg-warning "-Waddress" }
+
+
+ /* Similar to the above but for a pointer to a two-dimensional array,
+ referring to the higher-level element (i.e., an array itself). */
+ T (*p2 == 0);
+ T (**p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&**p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&***p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&**p2 == 0);
+
+ T (*p2 + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&(*p2)[0] == 0); // { dg-warning "-Waddress" }
+ T (&(*p2)[0][1] == 0); // { dg-warning "-Waddress" }
+ T (*p2 - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[-1]); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[1][2]); // { dg-warning "-Waddress" }
+ T (*p2 + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[-2]); // { dg-warning "-Waddress" }
+ T (*p2 - i == 0); // { dg-warning "-Waddress" }
+ T (&(*p2)[i] == 0); // { dg-warning "-Waddress" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
index 5224bb14234..63295230d51 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
@@ -85,8 +85,11 @@ extern __attribute__ ((weak)) int i;
constexpr int *p1 = &i + 1;
#pragma GCC diagnostic push
+// Suppress warning: ordered comparison of pointer with integer zero.
#pragma GCC diagnostic ignored "-Wextra"
-// Suppress warning: ordered comparison of pointer with integer zero
+// Also suppress -Waddress for comparisons of constant addresses to
+// to null.
+#pragma GCC diagnostic ignored "-Waddress"
constexpr bool b0 = p1; // { dg-error "not a constant expression" }
constexpr bool b1 = p1 == 0; // { dg-error "not a constant expression" }
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-5.C b/gcc/testsuite/g++.dg/warn/Waddress-5.C
new file mode 100644
index 00000000000..3390cda563b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-5.C
@@ -0,0 +1,87 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+#if __cplusplus < 201103L
+# define nullptr __null
+#endif
+
+struct A
+{
+ void f ();
+ virtual void vf ();
+ virtual void pvf () = 0;
+
+ void sf ();
+
+ int *p;
+ int a[2];
+};
+
+void T (bool);
+
+void warn_memptr_if ()
+{
+ // Exercise warnings for addresses of nonstatic member functions.
+ if (&A::f == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::vf) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::pvf != 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ // Exercise warnings for addresses of static member functions.
+ if (&A::sf == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::sf) // { dg-warning "-Waddress" }
+ T (0);
+
+ // Exercise warnings for addresses of nonstatic data members.
+ if (&A::p == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::a == nullptr) // { dg-warning "-Waddress" }
+ T (0);
+}
+
+void warn_memptr_bool ()
+{
+ // Exercise warnings for addresses of nonstatic member functions.
+ T (&A::f == 0); // { dg-warning "-Waddress" }
+ T (&A::vf); // { dg-warning "-Waddress" }
+ T (&A::pvf != 0); // { dg-warning "-Waddress" }
+
+ // Exercise warnings for addresses of static member functions.
+ T (&A::sf == 0); // { dg-warning "-Waddress" }
+ T (&A::sf); // { dg-warning "-Waddress" }
+
+ // Exercise warnings for addresses of nonstatic data members.
+ T (&A::p == 0); // { dg-warning "-Waddress" }
+ T (&A::a == nullptr); // { dg-warning "-Waddress" }
+}
+
+
+// Verify that no warnings are issued for an uninstantiated template.
+
+template <int>
+struct B
+{
+ // This is why.
+ struct F { void* operator& () const { return 0; } } f;
+};
+
+template <int N>
+void nowarn_template ()
+{
+ T (&B<N>::f == 0);
+
+ /* Like in the case above, the address-of operator could be a member
+ of B<N>::vf that returns zero. */
+ T (&B<N>::vf);
+ T (&B<N>::pvf != 0);
+ T (&B<N>::p == 0);
+ T (&B<N>::a == 0);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-6.C b/gcc/testsuite/g++.dg/warn/Waddress-6.C
new file mode 100644
index 00000000000..b4f6b4f4b52
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-6.C
@@ -0,0 +1,79 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ Verify -Waddress for member arrays of structs and notes.
+ { dg-options "-Wall" } */
+
+#if __cplusplus < 201103L
+# define nullptr __null
+#endif
+
+void T (bool);
+
+struct A
+{
+ int n;
+ int ia[]; // { dg-message "'A::ia' declared here" }
+};
+
+struct B
+{
+ A a[3]; // { dg-message "'B::a' declared here" }
+};
+
+struct C
+{
+ B b[3]; // { dg-message "'C::b' declared here" }
+};
+
+struct D
+{
+ C c[3]; // { dg-message "'D::c' declared here" }
+};
+
+
+void test_waddress_1d ()
+{
+ D d[2]; // { dg-message "'d' declared here" }
+
+ T (d); // { dg-warning "address of 'd'" }
+ T (d == nullptr); // { dg-warning "address of 'd'" }
+ T (&d); // { dg-warning "address of 'd'" }
+ T (d->c); // { dg-warning "address of 'D::c'" }
+ T (d->c != nullptr); // { dg-warning "address of 'D::c'" }
+ T (d->c->b); // { dg-warning "address of 'C::b'" }
+ T (d->c[1].b->a); // { dg-warning "address of 'B::a'" }
+ T (d->c->b[2].a->ia); // { dg-warning "address of 'A::ia'" }
+
+ if (d->c->b[2].a[1].ia) // { dg-warning "address of 'A::ia'" }
+ T (0);
+
+ if (bool b = d->c->b[1].a) // { dg-warning "address of 'A::ia'" }
+ T (b);
+
+ /* The following is represented as a declaration of P followed
+ by an if statement and so it isn't diagnosed. It's not clear
+ that it should be since the pointer is then used.
+ void *p = d->c->b[2].a;
+ if (p) ...
+ */
+ if (void *p = d->c->b[2].a) // { dg-warning "address of 'A::ia'" "" { xfail *-*-* } }
+ T (p);
+}
+
+
+void test_waddress_2d (int i)
+{
+ D d[2][3]; // { dg-message "'d' declared here" }
+
+ T (d); // { dg-warning "address of 'd'" }
+ T (d == nullptr); // { dg-warning "address of 'd'" }
+ T (&d); // { dg-warning "address of 'd'" }
+ T (*d); // { dg-warning "address of 'd'" }
+ T (d[1] != nullptr); // { dg-warning "address of 'd'" }
+ T (&d[1]->c); // { dg-warning "address of 'D::c'" }
+ T (d[1]->c); // { dg-warning "address of 'D::c'" }
+ T (d[1]->c == nullptr); // { dg-warning "address of 'D::c'" }
+ T (d[i]->c[1].b); // { dg-warning "address of 'C::b'" }
+ T ((*(d + i))->c->b->a); // { dg-warning "address of 'B::a'" }
+ T (d[1][2].c->b->a->ia); // { dg-warning "address of 'A::ia'" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
index bfe14457547..fae8b7e766f 100644
--- a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
+++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
@@ -9,7 +9,7 @@ const intptr_t&
return_addr_label_as_intref (void)
{
label:
- if ((const intptr_t*)&&label == 0)
+ if ((const intptr_t)&&label == 0)
__builtin_exit (1);
return *(const intptr_t*)&&label; // { dg-warning "\\\[-Wreturn-local-addr]" } */
@@ -19,7 +19,7 @@ const intptr_t&
return_addr_local_as_intref (void)
{
int a[1];
- if ((const intptr_t*)a == 0)
+ if ((const intptr_t)a == 0)
__builtin_exit (1);
return (const intptr_t&)a; // { dg-warning "\\\[-Wreturn-local-addr]" } */
diff --git a/gcc/testsuite/gcc.dg/Waddress-3.c b/gcc/testsuite/gcc.dg/Waddress-3.c
new file mode 100644
index 00000000000..1bf0050b000
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Waddress-3.c
@@ -0,0 +1,36 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef _Complex float Cflt;
+
+extern Cflt cf, cfa[], cfa2[][2];
+
+Cflt *pcf (void);
+
+void T (int);
+
+void test_complex (Cflt *p, int i)
+{
+ T (&__real__ cf == 0); // { dg-warning "-Waddress" }
+ T (&__imag__ cf == 0); // { dg-warning "-Waddress" }
+
+ T (0 != &__real__ cf); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ cf); // { dg-warning "-Waddress" }
+
+ T (&__real__ cfa[0] == 0); // { dg-warning "-Waddress" }
+ T (&__imag__ cfa[1] == 0); // { dg-warning "-Waddress" }
+
+ T (0 != &__real__ cfa2[i][i]); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ cfa2[i][i]); // { dg-warning "-Waddress" }
+
+ T (0 == &__real__ *p); // { dg-warning "-Waddress" }
+ T (0 == &__imag__ *p); // { dg-warning "-Waddress" }
+
+ T (0 == &__real__ p[i]); // { dg-warning "-Waddress" }
+ T (0 == &__imag__ p[i]); // { dg-warning "-Waddress" }
+
+ T (&__real__ *pcf () == 0); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ *pcf ()); // { dg-warning "-Waddress" }
+}
+
diff --git a/gcc/testsuite/gcc.dg/Waddress.c b/gcc/testsuite/gcc.dg/Waddress.c
index 146b1a932df..b26e7b1f329 100644
--- a/gcc/testsuite/gcc.dg/Waddress.c
+++ b/gcc/testsuite/gcc.dg/Waddress.c
@@ -6,5 +6,5 @@ int
foo(void)
{
char a[1];
- return a == 0;
+ return a == 0; // { dg-warning "-Waddress" }
}
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-02 23:53 ` [PATCH] warn for more impossible null pointer tests [PR102103] Martin Sebor
@ 2021-09-08 20:06 ` Jason Merrill
2021-09-17 16:02 ` Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2021-09-08 20:06 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers, Jeff Law
On 9/2/21 7:53 PM, Martin Sebor wrote:
> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
> if (!warn_address
> || (complain & tf_warning) == 0
> || c_inhibit_evaluation_warnings != 0
> - || warning_suppressed_p (op, OPT_Waddress))
> + || warning_suppressed_p (op, OPT_Waddress)
> + || processing_template_decl != 0)
Completely suppressing this warning in templates seems like a
regression; I'd think we could recognize many relevant cases before
instantiation. You just can't assume that ADDR_EXPR has the default
meaning if it has unknown type (i.e. because op0 is type-dependent).
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-08 20:06 ` Jason Merrill
@ 2021-09-17 16:02 ` Martin Sebor
2021-09-21 21:40 ` Jason Merrill
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-17 16:02 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers, Jeff Law
[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]
On 9/8/21 2:06 PM, Jason Merrill wrote:
> On 9/2/21 7:53 PM, Martin Sebor wrote:
>> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location,
>> tree op, tsubst_flags_t complain)
>> if (!warn_address
>> || (complain & tf_warning) == 0
>> || c_inhibit_evaluation_warnings != 0
>> - || warning_suppressed_p (op, OPT_Waddress))
>> + || warning_suppressed_p (op, OPT_Waddress)
>> + || processing_template_decl != 0)
>
> Completely suppressing this warning in templates seems like a
> regression; I'd think we could recognize many relevant cases before
> instantiation. You just can't assume that ADDR_EXPR has the default
> meaning if it has unknown type (i.e. because op0 is type-dependent).
I added the suppression to keep g++.dg/warn/pr101219.C from failing
but in hindsight I should have questioned the reasoning behind
the "no warning emitted here (no instantiation)" comment in the test.
I agree that it would be helpful to diagnose the type-independent
subset of the problem even in uninstantiated templates. Current
trunk doesn't (it never has), but with my patch and the suppression
above removed it does. I've updated the tests to expect it.
Please see the attached revision.
Martin
PS There are still more opportunities to issue -Waddress in templates
that this patch doesn't handle, e.g.,:
template <class T> bool f (T *p) { return &p == 0; }
Handling this will take more surgery.
PPS It seems that most other warnings (and even some errors, like
-Wnarrowing) are suppressed in uninstantiated templates as well,
even for non-dependent expressions. In the few test cases I looked
at Clang does better. It sounds like you'd like to see improvements
in this direction not just for -Waddress but in general. Just for
the avoidance of doubt, can you confirm that for future reference?
>
> Jason
>
[-- Attachment #2: gcc-102103.diff --]
[-- Type: text/x-patch, Size: 36507 bytes --]
Enhance -Waddress to detect more suspicious expressions [PR102103].
Resolves:
PR c/102103 - missing warning comparing array address to null
gcc/ChangeLog:
* doc/invoke.texi (-Waddress): Update.
* gcc/gengtype.c (write_types): Avoid -Waddress.
gcc/c-family/ChangeLog:
* c-common.c (decl_with_nonnull_addr_p): Handle members.
Check and perform warning suppression.
(c_common_truthvalue_conversion): Enhance warning suppression.
gcc/c/ChangeLog:
* c-typeck.c (maybe_warn_for_null_address): New function.
(build_binary_op): Call it.
gcc/cp/ChangeLog:
* typeck.c (warn_for_null_address): Enhance.
(cp_build_binary_op): Call it also for member pointers.
gcc/fortran/ChangeLog:
* gcc/fortran/array.c: Remove an unnecessary test.
* gcc/fortran/trans-array.c: Same.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-array-ptr10.C: Suppress a valid warning.
* g++.dg/warn/Wreturn-local-addr-6.C: Correct a cast.
* gcc.dg/Waddress.c: Expect a warning.
* c-c++-common/Waddress-3.c: New test.
* c-c++-common/Waddress-4.c: New test.
* g++.dg/warn/Waddress-5.C: New test.
* g++.dg/warn/Waddress-6.C: New test.
* g++.dg/warn/pr101219.C: Expect a warning.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c6757f093ac..249da7c7f0f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3393,14 +3393,16 @@ c_wrap_maybe_const (tree expr, bool non_const)
return expr;
}
-/* Return whether EXPR is a declaration whose address can never be
- NULL. */
+/* Return whether EXPR is a declaration whose address can never be NULL.
+ The address of the first struct member could be NULL only if it were
+ accessed through a NULL pointer, and such an access would be invalid. */
bool
decl_with_nonnull_addr_p (const_tree expr)
{
return (DECL_P (expr)
- && (TREE_CODE (expr) == PARM_DECL
+ && (TREE_CODE (expr) == FIELD_DECL
+ || TREE_CODE (expr) == PARM_DECL
|| TREE_CODE (expr) == LABEL_DECL
|| !DECL_WEAK (expr)));
}
@@ -3488,13 +3490,17 @@ c_common_truthvalue_conversion (location_t location, tree expr)
case ADDR_EXPR:
{
tree inner = TREE_OPERAND (expr, 0);
- if (decl_with_nonnull_addr_p (inner))
+ if (decl_with_nonnull_addr_p (inner)
+ /* Check both EXPR and INNER for suppression. */
+ && !warning_suppressed_p (expr, OPT_Waddress)
+ && !warning_suppressed_p (inner, OPT_Waddress))
{
- /* Common Ada programmer's mistake. */
+ /* Common Ada programmer's mistake. */
warning_at (location,
OPT_Waddress,
"the address of %qD will always evaluate as %<true%>",
inner);
+ suppress_warning (inner, OPT_Waddress);
return truthvalue_true_node;
}
break;
@@ -3627,8 +3633,17 @@ c_common_truthvalue_conversion (location_t location, tree expr)
break;
/* If this isn't narrowing the argument, we can ignore it. */
if (TYPE_PRECISION (totype) >= TYPE_PRECISION (fromtype))
- return c_common_truthvalue_conversion (location,
- TREE_OPERAND (expr, 0));
+ {
+ tree op0 = TREE_OPERAND (expr, 0);
+ if ((TREE_CODE (fromtype) == POINTER_TYPE
+ && TREE_CODE (totype) == INTEGER_TYPE)
+ || warning_suppressed_p (expr, OPT_Waddress))
+ /* Suppress -Waddress for casts to intptr_t, propagating
+ any suppression from the enclosing expression to its
+ operand. */
+ suppress_warning (op0, OPT_Waddress);
+ return c_common_truthvalue_conversion (location, op0);
+ }
}
break;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 49d1bb067a0..a440bc9a75a 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -11545,6 +11545,110 @@ build_vec_cmp (tree_code code, tree type,
return build3 (VEC_COND_EXPR, type, cmp, minus_one_vec, zero_vec);
}
+/* Possibly warn about an address of OP never being NULL in a comparison
+ operation CODE involving null. */
+
+static void
+maybe_warn_for_null_address (location_t loc, tree op, tree_code code)
+{
+ if (!warn_address || warning_suppressed_p (op, OPT_Waddress))
+ return;
+
+ if (TREE_CODE (op) == NOP_EXPR)
+ {
+ /* Allow casts to intptr_t to suppress the warning. */
+ tree type = TREE_TYPE (op);
+ if (TREE_CODE (type) == INTEGER_TYPE)
+ return;
+ op = TREE_OPERAND (op, 0);
+ }
+
+ if (TREE_CODE (op) == POINTER_PLUS_EXPR)
+ {
+ /* Allow a cast to void* to suppress the warning. */
+ tree type = TREE_TYPE (TREE_TYPE (op));
+ if (VOID_TYPE_P (type))
+ return;
+
+ /* Adding any value to a null pointer, including zero, is undefined
+ in C. This includes the expression &p[0] where p is the null
+ pointer, although &p[0] will have been folded to p by this point
+ and so not diagnosed. */
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the pointer operand in %qE must not be NULL",
+ op);
+
+ return;
+ }
+
+ if (TREE_CODE (op) != ADDR_EXPR)
+ return;
+
+ op = TREE_OPERAND (op, 0);
+
+ if (TREE_CODE (op) == IMAGPART_EXPR
+ || TREE_CODE (op) == REALPART_EXPR)
+ {
+ /* The address of either complex part may not be null. */
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the address %qE will never be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the address %qE will never be NULL",
+ op);
+ return;
+ }
+
+ /* Set to true in the loop below if OP dereferences is operand.
+ In such a case the ultimate target need not be a decl for
+ the null [in]equality test to be constant. */
+ bool deref = false;
+
+ /* Get the outermost array or object, or member. */
+ while (handled_component_p (op))
+ {
+ if (TREE_CODE (op) == COMPONENT_REF)
+ {
+ /* Get the member (its address is never null). */
+ op = TREE_OPERAND (op, 1);
+ break;
+ }
+
+ /* Get the outer array/object to refer to in the warning. */
+ op = TREE_OPERAND (op, 0);
+ deref = true;
+ }
+
+ if ((!deref && !decl_with_nonnull_addr_p (op))
+ || from_macro_expansion_at (loc))
+ return;
+
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<false%> "
+ "for the address of %qE will never be NULL",
+ op);
+ else
+ warning_at (loc, OPT_Waddress,
+ "the comparison will always evaluate as %<true%> "
+ "for the address of %qE will never be NULL",
+ op);
+
+ if (DECL_P (op))
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
+}
+
/* Build a binary-operation expression without default conversions.
CODE is the kind of expression to build.
LOCATION is the operator's location.
@@ -12180,44 +12284,12 @@ build_binary_op (location_t location, enum tree_code code,
short_compare = 1;
else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
{
- if (TREE_CODE (op0) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op0, 0));
- }
+ maybe_warn_for_null_address (location, op0, code);
result_type = type0;
}
else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
{
- if (TREE_CODE (op1) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0))
- && !from_macro_expansion_at (location))
- {
- if (code == EQ_EXPR)
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<false%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- else
- warning_at (location,
- OPT_Waddress,
- "the comparison will always evaluate as %<true%> "
- "for the address of %qD will never be NULL",
- TREE_OPERAND (op1, 0));
- }
+ maybe_warn_for_null_address (location, op1, code);
result_type = type1;
}
else if (code0 == POINTER_TYPE && code1 == POINTER_TYPE)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a2398dbe660..3cb25ec627c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4603,25 +4603,93 @@ warn_for_null_address (location_t location, tree op, tsubst_flags_t complain)
|| warning_suppressed_p (op, OPT_Waddress))
return;
+ if (TREE_CODE (op) == NON_DEPENDENT_EXPR)
+ op = TREE_OPERAND (op, 0);
+
tree cop = fold_for_warn (op);
- if (TREE_CODE (cop) == ADDR_EXPR
- && decl_with_nonnull_addr_p (TREE_OPERAND (cop, 0))
- && !warning_suppressed_p (cop, OPT_Waddress))
- warning_at (location, OPT_Waddress, "the address of %qD will never "
- "be NULL", TREE_OPERAND (cop, 0));
+ if (TREE_CODE (cop) == NON_LVALUE_EXPR)
+ /* Unwrap the expression for C++ 98. */
+ cop = TREE_OPERAND (cop, 0);
- if (CONVERT_EXPR_P (op)
+ if (TREE_CODE (cop) == PTRMEM_CST)
+ {
+ /* The address of a nonstatic data member is never null. */
+ warning_at (location, OPT_Waddress,
+ "the address %qE will never be NULL",
+ cop);
+ return;
+ }
+
+ if (TREE_CODE (cop) == NOP_EXPR)
+ {
+ /* Allow casts to intptr_t to suppress the warning. */
+ tree type = TREE_TYPE (cop);
+ if (TREE_CODE (type) == INTEGER_TYPE)
+ return;
+
+ STRIP_NOPS (cop);
+ }
+
+ bool warned = false;
+ if (TREE_CODE (cop) == ADDR_EXPR)
+ {
+ cop = TREE_OPERAND (cop, 0);
+
+ /* Set to true in the loop below if OP dereferences its operand.
+ In such a case the ultimate target need not be a decl for
+ the null [in]equality test to be necessarily constant. */
+ bool deref = false;
+
+ /* Get the outermost array or object, or member. */
+ while (handled_component_p (cop))
+ {
+ if (TREE_CODE (cop) == COMPONENT_REF)
+ {
+ /* Get the member (its address is never null). */
+ cop = TREE_OPERAND (cop, 1);
+ break;
+ }
+
+ /* Get the outer array/object to refer to in the warning. */
+ cop = TREE_OPERAND (cop, 0);
+ deref = true;
+ }
+
+ if ((!deref && !decl_with_nonnull_addr_p (cop))
+ || from_macro_expansion_at (location)
+ || warning_suppressed_p (cop, OPT_Waddress))
+ return;
+
+ warned = warning_at (location, OPT_Waddress,
+ "the address of %qD will never be NULL", cop);
+ op = cop;
+ }
+ else if (TREE_CODE (cop) == POINTER_PLUS_EXPR)
+ {
+ /* Adding zero to the null pointer is well-defined in C++. When
+ the offset is unknown (i.e., not a constant) warn anyway since
+ it's less likely that the pointer operand is null than not. */
+ tree off = TREE_OPERAND (cop, 1);
+ if (!integer_zerop (off)
+ && !warning_suppressed_p (cop, OPT_Waddress))
+ warning_at (location, OPT_Waddress, "comparing the result of pointer "
+ "addition %qE and NULL", cop);
+ return;
+ }
+ else if (CONVERT_EXPR_P (op)
&& TYPE_REF_P (TREE_TYPE (TREE_OPERAND (op, 0))))
{
- tree inner_op = op;
- STRIP_NOPS (inner_op);
+ STRIP_NOPS (op);
- if (DECL_P (inner_op))
- warning_at (location, OPT_Waddress,
- "the compiler can assume that the address of "
- "%qD will never be NULL", inner_op);
+ if (DECL_P (op))
+ warned = warning_at (location, OPT_Waddress,
+ "the compiler can assume that the address of "
+ "%qD will never be NULL", op);
}
+
+ if (warned && DECL_P (op))
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
}
/* Warn about [expr.arith.conv]/2: If one operand is of enumeration type and
@@ -5411,6 +5479,8 @@ cp_build_binary_op (const op_location_t &location,
op1 = cp_convert (TREE_TYPE (op0), op1, complain);
}
result_type = TREE_TYPE (op0);
+
+ warn_for_null_address (location, orig_op0, complain);
}
else if (TYPE_PTRMEMFUNC_P (type1) && null_ptr_cst_p (orig_op0))
return cp_build_binary_op (location, code, op1, op0, complain);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 78cfc100ac2..534d14d1e18 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8549,17 +8549,43 @@ by @option{-Wall}.
@item -Waddress
@opindex Waddress
@opindex Wno-address
-Warn about suspicious uses of memory addresses. These include using
-the address of a function in a conditional expression, such as
-@code{void func(void); if (func)}, and comparisons against the memory
-address of a string literal, such as @code{if (x == "abc")}. Such
-uses typically indicate a programmer error: the address of a function
-always evaluates to true, so their use in a conditional usually
-indicate that the programmer forgot the parentheses in a function
-call; and comparisons against string literals result in unspecified
-behavior and are not portable in C, so they usually indicate that the
-programmer intended to use @code{strcmp}. This warning is enabled by
-@option{-Wall}.
+Warn about suspicious uses of address expressions. These include comparing
+the address of a function or a declared object to the null pointer constant
+such as in
+@smallexample
+void f (void);
+void g (void)
+@{
+ if (!func) // warning: expression evaluates to false
+ abort ();
+@}
+@end smallexample
+comparisons of a pointer to a string literal, such as in
+@smallexample
+void f (const char *x)
+@{
+ if (x == "abc") // warning: expression evaluates to false
+ puts ("equal");
+@}
+@end smallexample
+and tests of the results of pointer addition or subtraction for equality
+to null, such as in
+@smallexample
+void f (const int *p, int i)
+@{
+ return p + i == NULL;
+@}
+@end smallexample
+Such uses typically indicate a programmer error: the address of most
+functions and objects necessarily evaluates to true (the exception are
+weak symbols), so their use in a conditional might indicate missing
+parentheses in a function call or a missing dereference in an array
+expression. The subset of the warning for object pointers can be
+suppressed by casting the pointer operand to an integer type such
+as @code{inptr_t} or @code{uinptr_t}.
+Comparisons against string literals result in unspecified behavior
+and are not portable, and suggest the intent was to call @code{strcmp}.
+@option{-Waddress} warning is enabled by @option{-Wall}.
@item -Wno-address-of-packed-member
@opindex Waddress-of-packed-member
diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index b858bada18a..341290f91b8 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -2578,7 +2578,7 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t *result)
}
}
- if (array->shape && array->shape[dimen])
+ if (array->shape)
{
mpz_init_set (*result, array->shape[dimen]);
return true;
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0d013defdbb..5147509fbe7 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5104,7 +5104,6 @@ set_loop_bounds (gfc_loopinfo *loop)
if (info->shape)
{
- gcc_assert (info->shape[dim]);
/* The frontend has worked out the size for us. */
if (!loopspec[n]
|| !specinfo->shape
diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 31d4bf4e5d0..a77cfd92bfa 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -3685,8 +3685,8 @@ write_types (outf_p output_header, type_p structures,
output_mangled_typename (output_header, s);
oprintf (output_header, "(X) do { \\\n");
oprintf (output_header,
- " if (X != NULL) gt_%sx_%s (X);\\\n", wtd->prefix,
- s_id_for_tag);
+ " if ((intptr_t)(X) != 0) gt_%sx_%s (X);\\\n",
+ wtd->prefix, s_id_for_tag);
oprintf (output_header, " } while (0)\n");
for (opt = s->u.s.opt; opt; opt = opt->next)
diff --git a/gcc/poly-int.h b/gcc/poly-int.h
index f47f9e436a8..94e7b701f64 100644
--- a/gcc/poly-int.h
+++ b/gcc/poly-int.h
@@ -324,10 +324,10 @@ struct poly_result<T1, T2, 2>
routine can take the address of RES rather than the address of
a temporary.
- The dummy comparison against a null C * is just a way of checking
+ The dummy self-comparison against C * is just a way of checking
that C gives the right type. */
#define POLY_SET_COEFF(C, RES, I, VALUE) \
- ((void) (&(RES).coeffs[0] == (C *) 0), \
+ ((void) (&(RES).coeffs[0] == (C *) (void *) &(RES).coeffs[0]), \
wi::int_traits<C>::precision_type == wi::FLEXIBLE_PRECISION \
? (void) ((RES).coeffs[I] = VALUE) \
: (void) ((RES).coeffs[I].~C (), new (&(RES).coeffs[I]) C (VALUE)))
diff --git a/gcc/testsuite/c-c++-common/Waddress-3.c b/gcc/testsuite/c-c++-common/Waddress-3.c
new file mode 100644
index 00000000000..9a13a444045
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-3.c
@@ -0,0 +1,125 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+struct S { void *p, *a1[2], *a2[2][2]; } s, *p;
+
+extern const void *a1[2];
+extern void *a2[2][2], *ax[];
+
+void T (bool);
+
+void test_array_eq_0 (int i)
+{
+ // Verify that casts intptr_t suppress the warning.
+ T ((intptr_t)a1 == 0);
+ T ((uintptr_t)a1 == 0);
+ T (a1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1); // { dg-warning "-Waddress" }
+ // Verify that casts to other pointer types don't suppress it.
+ T ((void *)a1 == 0); // { dg-warning "-Waddress" }
+ T ((char *)a1 == 0); // { dg-warning "-Waddress" }
+ T (a1[0] == 0);
+ T (0 == (intptr_t)&a1[0]);
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (a1[i] == 0);
+ T (0 == (uintptr_t)&a1[i]);
+ T (0 == &a1[i]); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)a2 == 0);
+ T (a2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2); // { dg-warning "-Waddress" }
+ T (a2[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (a2[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[i]); // { dg-warning "-Waddress" }
+ T (a2[0][0] == 0);
+ T (0 == &a2[0][0]); // { dg-warning "-Waddress" }
+ T (&ax == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax); // { dg-warning "-Waddress" }
+ T (&ax[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == ax[0]);
+}
+
+
+void test_array_neq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((uintptr_t)a1);
+
+ T (a1); // { dg-warning "-Waddress" }
+ T ((void *)a1); // { dg-warning "-Waddress" }
+ T (&a1 != 0); // { dg-warning "-Waddress" }
+ T (a1[0]);
+ T (&a1[0] != 0); // { dg-warning "-Waddress" }
+ T (a1[i]);
+ T (&a1[i] != 0); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)a2);
+ T (a2); // { dg-warning "-Waddress" }
+ T ((void *)a2); // { dg-warning "-Waddress" }
+ T ((char *)a2); // { dg-warning "-Waddress" }
+ T (&a2 != 0); // { dg-warning "-Waddress" }
+ T (a2[0]); // { dg-warning "-Waddress" }
+ T (&a1[0] != 0); // { dg-warning "-Waddress" }
+ T (a2[i]); // { dg-warning "-Waddress" }
+ T (&a2[i] != 0); // { dg-warning "-Waddress" }
+ T (a2[0][0]);
+ T (&a2[0][0] != 0); // { dg-warning "-Waddress" }
+}
+
+
+void test_member_array_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)s.a1 == 0);
+ T (s.a1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1); // { dg-warning "-Waddress" }
+ T (s.a1[0] == 0);
+ T ((void*)s.a1); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (s.a1[i] == 0);
+ T (0 == &a1[i]); // { dg-warning "-Waddress" }
+
+ T ((uintptr_t)s.a2 == 0);
+ T (s.a2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2); // { dg-warning "-Waddress" }
+ T ((void *)s.a2 == 0);// { dg-warning "-Waddress" }
+ T (s.a2[0] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a1[0]); // { dg-warning "-Waddress" }
+ T (s.a2[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[i]); // { dg-warning "-Waddress" }
+ T (s.a2[0][0] == 0);
+ T (0 == &a2[0][0]); // { dg-warning "-Waddress" }
+}
+
+
+void test_member_array_neq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((uintptr_t)s.a1);
+ T (s.a1); // { dg-warning "-Waddress" }
+ T (&s.a1 != 0); // { dg-warning "-Waddress" }
+ T ((void *)&s.a1[0]); // { dg-warning "-Waddress" }
+ T (s.a1[0]);
+ T (&s.a1[0] != 0); // { dg-warning "-Waddress" }
+ T (s.a1[i]);
+ T (&s.a1[i] != 0); // { dg-warning "-Waddress" }
+
+ T ((intptr_t)s.a2);
+ T (s.a2); // { dg-warning "-Waddress" }
+ T (&s.a2 != 0); // { dg-warning "-Waddress" }
+ T (s.a2[0]); // { dg-warning "-Waddress" }
+ T (&s.a1[0] != 0); // { dg-warning "-Waddress" }
+ T (s.a2[i]); // { dg-warning "-Waddress" }
+ T (&s.a2[i] != 0); // { dg-warning "-Waddress" }
+ T (s.a2[0][0]);
+ T (&s.a2[0][0] != 0); // { dg-warning "-Waddress" }
+}
diff --git a/gcc/testsuite/c-c++-common/Waddress-4.c b/gcc/testsuite/c-c++-common/Waddress-4.c
new file mode 100644
index 00000000000..7b32503f84e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-4.c
@@ -0,0 +1,106 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __INTPTR_TYPE__ uintptr_t;
+
+extern char *ax[], *a2[][2];
+
+void T (int);
+
+void test_ax_plus_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)(ax + 0) == 0);
+ T ((uintptr_t)(ax + 1) == 0);
+
+ T (ax + 0 == 0); // { dg-warning "-Waddress" }
+ T (&ax[0] == 0); // { dg-warning "-Waddress" }
+ T (ax - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[-1]); // { dg-warning "-Waddress" }
+ T ((void *)(&ax[0] + 2) == 0); // { dg-warning "-Waddress" }
+ T (&ax[0] + 2 == 0); // { dg-warning "-Waddress" }
+ T (ax + 3 == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[-4]); // { dg-warning "-Waddress" }
+ T (ax - i == 0); // { dg-warning "-Waddress" }
+ T (&ax[i] == 0); // { dg-warning "-Waddress" }
+ T (0 == &ax[1] + i); // { dg-warning "-Waddress" }
+}
+
+void test_a2_plus_eq_0 (int i)
+{
+ // Verify that casts to intptr_t suppress the warning.
+ T ((intptr_t)(a2 + 0) == 0);
+ T ((uintptr_t)(a2 + 1) == 0);
+
+ T (a2 + 0 == 0); // { dg-warning "-Waddress" }
+ // Verify that a cast to another pointer type doesn't suppress it.
+ T ((void*)(a2 + 0) == 0); // { dg-warning "-Waddress" }
+ T ((char*)a2 + 1 == 0); // { dg-warning "-Waddress" }
+ T (&a2[0] == 0); // { dg-warning "-Waddress" }
+ T (a2 - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[-1]); // { dg-warning "-Waddress" }
+ T (a2 + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &a2[-2]); // { dg-warning "-Waddress" }
+ T (a2 - i == 0); // { dg-warning "-Waddress" }
+ T (&a2[i] == 0); // { dg-warning "-Waddress" }
+}
+
+// Exercise a pointer.
+void test_p_plus_eq_0 (int *p, int i)
+{
+ /* P + 0 and equivalently &P[0] are invalid for a null P but they're
+ folded to p before the warning has a chance to trigger. */
+ T (p + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&p[0] == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+
+ T (p - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &p[-1]); // { dg-warning "-Waddress" }
+ T (p + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &p[-2]); // { dg-warning "-Waddress" }
+ T (p - i == 0); // { dg-warning "-Waddress" }
+ T (&p[i] == 0); // { dg-warning "-Waddress" }
+}
+
+// Exercise pointer to array.
+void test_pa_plus_eq_0 (int (*p)[], int (*p2)[][2], int i)
+{
+ // The array pointer may be null.
+ T (*p == 0);
+ /* &**P is equivalent to *P and might be the result od macro expansion.
+ Verify it doesn't cause a warning. */
+ T (&**p == 0);
+
+ /* *P + 0 is invalid but folded to *P before the warning has a chance
+ to trigger. */
+ T (*p + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+
+ T (&(*p)[0] == 0); // { dg-warning "-Waddress" }
+ T (*p - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p)[-1]); // { dg-warning "-Waddress" }
+ T (*p + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p)[-2]); // { dg-warning "-Waddress" }
+ T (*p - i == 0); // { dg-warning "-Waddress" }
+ T (&(*p)[i] == 0); // { dg-warning "-Waddress" }
+
+
+ /* Similar to the above but for a pointer to a two-dimensional array,
+ referring to the higher-level element (i.e., an array itself). */
+ T (*p2 == 0);
+ T (**p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&**p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&***p2 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&**p2 == 0);
+
+ T (*p2 + 0 == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&(*p2)[0] == 0); // { dg-warning "-Waddress" }
+ T (&(*p2)[0][1] == 0); // { dg-warning "-Waddress" }
+ T (*p2 - 1 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[-1]); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[1][2]); // { dg-warning "-Waddress" }
+ T (*p2 + 2 == 0); // { dg-warning "-Waddress" }
+ T (0 == &(*p2)[-2]); // { dg-warning "-Waddress" }
+ T (*p2 - i == 0); // { dg-warning "-Waddress" }
+ T (&(*p2)[i] == 0); // { dg-warning "-Waddress" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
index 5224bb14234..63295230d51 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
@@ -85,8 +85,11 @@ extern __attribute__ ((weak)) int i;
constexpr int *p1 = &i + 1;
#pragma GCC diagnostic push
+// Suppress warning: ordered comparison of pointer with integer zero.
#pragma GCC diagnostic ignored "-Wextra"
-// Suppress warning: ordered comparison of pointer with integer zero
+// Also suppress -Waddress for comparisons of constant addresses to
+// to null.
+#pragma GCC diagnostic ignored "-Waddress"
constexpr bool b0 = p1; // { dg-error "not a constant expression" }
constexpr bool b1 = p1 == 0; // { dg-error "not a constant expression" }
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-5.C b/gcc/testsuite/g++.dg/warn/Waddress-5.C
new file mode 100644
index 00000000000..a4f4e5eb89f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-5.C
@@ -0,0 +1,115 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+#if __cplusplus < 201103L
+# define nullptr __null
+#endif
+
+struct A
+{
+ void f ();
+ virtual void vf ();
+ virtual void pvf () = 0;
+
+ void sf ();
+
+ int *p;
+ int a[2];
+};
+
+void T (bool);
+
+void warn_memptr_if ()
+{
+ // Exercise warnings for addresses of nonstatic member functions.
+ if (&A::f == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::vf) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::pvf != 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ // Exercise warnings for addresses of static member functions.
+ if (&A::sf == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::sf) // { dg-warning "-Waddress" }
+ T (0);
+
+ // Exercise warnings for addresses of nonstatic data members.
+ if (&A::p == 0) // { dg-warning "-Waddress" }
+ T (0);
+
+ if (&A::a == nullptr) // { dg-warning "-Waddress" }
+ T (0);
+}
+
+void warn_memptr_bool ()
+{
+ // Exercise warnings for addresses of nonstatic member functions.
+ T (&A::f == 0); // { dg-warning "-Waddress" }
+ T (&A::vf); // { dg-warning "-Waddress" }
+ T (&A::pvf != 0); // { dg-warning "-Waddress" }
+
+ // Exercise warnings for addresses of static member functions.
+ T (&A::sf == 0); // { dg-warning "-Waddress" }
+ T (&A::sf); // { dg-warning "-Waddress" }
+
+ // Exercise warnings for addresses of nonstatic data members.
+ T (&A::p == 0); // { dg-warning "-Waddress" }
+ T (&A::a == nullptr); // { dg-warning "-Waddress" }
+}
+
+
+/* Verify that no warnings are issued for a dependent expression in
+ a template. */
+
+template <int>
+struct B
+{
+ // This is why.
+ struct F { void* operator& () const { return 0; } } f;
+};
+
+template <class Type, int N>
+void nowarn_dependent (Type targ)
+{
+ T (&Type::x == 0);
+ T (&targ == 0);
+
+ Type tarr[1];
+ T (&tarr[0] == nullptr);
+
+ T (&B<N>::f == 0);
+
+ /* Like in the case above, the address-of operator could be a member
+ of B<N>::vf that returns zero. */
+ T (&B<N>::vf);
+ T (&B<N>::pvf != 0);
+ T (&B<N>::p == 0);
+ T (&B<N>::a == 0);
+}
+
+
+/* Verify that in an uninstantiated template warnings are not issued
+ for dependent expressions but are issued otherwise. */
+
+template <class Type>
+void warn_non_dependent (Type targ, Type *tptr, int i)
+{
+ /* The address of a pointer to a dependent type cannot be null but
+ the warning doesn't have a chance to see it. */
+ T (&tptr == 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&i == 0); // { dg-warning "-Waddress" }
+
+ int iarr[1];
+ T (&iarr == 0); // { dg-warning "-Waddress" }
+ T (&*iarr != 0); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+ T (&iarr[0] == 0); // { dg-warning "-Waddress" }
+
+ Type tarr[1];
+ T (&tarr == nullptr); // { dg-warning "-Waddress" "pr??????" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-6.C b/gcc/testsuite/g++.dg/warn/Waddress-6.C
new file mode 100644
index 00000000000..c22a83a0dd7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Waddress-6.C
@@ -0,0 +1,79 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ Verify -Waddress for member arrays of structs and notes.
+ { dg-options "-Wall" } */
+
+#if __cplusplus < 201103L
+# define nullptr __null
+#endif
+
+void T (bool);
+
+struct A
+{
+ int n;
+ int ia[]; // { dg-message "'A::ia' declared here" }
+};
+
+struct B
+{
+ A a[3]; // { dg-message "'B::a' declared here" }
+};
+
+struct C
+{
+ B b[3]; // { dg-message "'C::b' declared here" }
+};
+
+struct D
+{
+ C c[3]; // { dg-message "'D::c' declared here" }
+};
+
+
+void test_waddress_1d ()
+{
+ D d[2]; // { dg-message "'d' declared here" }
+
+ T (d); // { dg-warning "address of 'd'" }
+ T (d == nullptr); // { dg-warning "address of 'd'" }
+ T (&d); // { dg-warning "address of 'd'" }
+ T (d->c); // { dg-warning "address of 'D::c'" }
+ T (d->c != nullptr); // { dg-warning "address of 'D::c'" }
+ T (d->c->b); // { dg-warning "address of 'C::b'" }
+ T (d->c[1].b->a); // { dg-warning "address of 'B::a'" }
+ T (d->c->b[2].a->ia); // { dg-warning "address of 'A::ia'" }
+
+ if (d->c->b[2].a[1].ia) // { dg-warning "address of 'A::ia'" }
+ T (0);
+
+ if (bool b = d->c->b[1].a) // { dg-warning "address of 'B::a'" }
+ T (b);
+
+ /* The following is represented as a declaration of P followed
+ by an if statement and so it isn't diagnosed. It's not clear
+ that it should be since the pointer is then used.
+ void *p = d->c->b[2].a;
+ if (p) ...
+ */
+ if (void *p = d->c->b[2].a) // { dg-warning "address of 'A::ia'" "" { xfail *-*-* } }
+ T (p);
+}
+
+
+void test_waddress_2d (int i)
+{
+ D d[2][3]; // { dg-message "'d' declared here" }
+
+ T (d); // { dg-warning "address of 'd'" }
+ T (d == nullptr); // { dg-warning "address of 'd'" }
+ T (&d); // { dg-warning "address of 'd'" }
+ T (*d); // { dg-warning "address of 'd'" }
+ T (d[1] != nullptr); // { dg-warning "address of 'd'" }
+ T (&d[1]->c); // { dg-warning "address of 'D::c'" }
+ T (d[1]->c); // { dg-warning "address of 'D::c'" }
+ T (d[1]->c == nullptr); // { dg-warning "address of 'D::c'" }
+ T (d[i]->c[1].b); // { dg-warning "address of 'C::b'" }
+ T ((*(d + i))->c->b->a); // { dg-warning "address of 'B::a'" }
+ T (d[1][2].c->b->a->ia); // { dg-warning "address of 'A::ia'" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
index bfe14457547..fae8b7e766f 100644
--- a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
+++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
@@ -9,7 +9,7 @@ const intptr_t&
return_addr_label_as_intref (void)
{
label:
- if ((const intptr_t*)&&label == 0)
+ if ((const intptr_t)&&label == 0)
__builtin_exit (1);
return *(const intptr_t*)&&label; // { dg-warning "\\\[-Wreturn-local-addr]" } */
@@ -19,7 +19,7 @@ const intptr_t&
return_addr_local_as_intref (void)
{
int a[1];
- if ((const intptr_t*)a == 0)
+ if ((const intptr_t)a == 0)
__builtin_exit (1);
return (const intptr_t&)a; // { dg-warning "\\\[-Wreturn-local-addr]" } */
diff --git a/gcc/testsuite/g++.dg/warn/pr101219.C b/gcc/testsuite/g++.dg/warn/pr101219.C
index 0d23d73c9ec..d5d44e44852 100644
--- a/gcc/testsuite/g++.dg/warn/pr101219.C
+++ b/gcc/testsuite/g++.dg/warn/pr101219.C
@@ -7,5 +7,7 @@ struct S { void m(); };
template <int> bool f() {
void (S::*mp)();
- return &S::m == mp; // no warning emitted here (no instantiation)
+ /* The expression below isn't type-dependent so also verify
+ it's diagnosed even though the template isn't instantiated. */
+ return &S::m == mp; // { dg-warning "\\\[-Waddress" }
}
diff --git a/gcc/testsuite/gcc.dg/Waddress-3.c b/gcc/testsuite/gcc.dg/Waddress-3.c
new file mode 100644
index 00000000000..1bf0050b000
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Waddress-3.c
@@ -0,0 +1,36 @@
+/* PR c/102103 - missing warning comparing array address to null
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef _Complex float Cflt;
+
+extern Cflt cf, cfa[], cfa2[][2];
+
+Cflt *pcf (void);
+
+void T (int);
+
+void test_complex (Cflt *p, int i)
+{
+ T (&__real__ cf == 0); // { dg-warning "-Waddress" }
+ T (&__imag__ cf == 0); // { dg-warning "-Waddress" }
+
+ T (0 != &__real__ cf); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ cf); // { dg-warning "-Waddress" }
+
+ T (&__real__ cfa[0] == 0); // { dg-warning "-Waddress" }
+ T (&__imag__ cfa[1] == 0); // { dg-warning "-Waddress" }
+
+ T (0 != &__real__ cfa2[i][i]); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ cfa2[i][i]); // { dg-warning "-Waddress" }
+
+ T (0 == &__real__ *p); // { dg-warning "-Waddress" }
+ T (0 == &__imag__ *p); // { dg-warning "-Waddress" }
+
+ T (0 == &__real__ p[i]); // { dg-warning "-Waddress" }
+ T (0 == &__imag__ p[i]); // { dg-warning "-Waddress" }
+
+ T (&__real__ *pcf () == 0); // { dg-warning "-Waddress" }
+ T (0 != &__imag__ *pcf ()); // { dg-warning "-Waddress" }
+}
+
diff --git a/gcc/testsuite/gcc.dg/Waddress.c b/gcc/testsuite/gcc.dg/Waddress.c
index 146b1a932df..b26e7b1f329 100644
--- a/gcc/testsuite/gcc.dg/Waddress.c
+++ b/gcc/testsuite/gcc.dg/Waddress.c
@@ -6,5 +6,5 @@ int
foo(void)
{
char a[1];
- return a == 0;
+ return a == 0; // { dg-warning "-Waddress" }
}
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-17 16:02 ` Martin Sebor
@ 2021-09-21 21:40 ` Jason Merrill
2021-09-22 0:34 ` Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2021-09-21 21:40 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers, Jeff Law
On 9/17/21 12:02, Martin Sebor wrote:
> On 9/8/21 2:06 PM, Jason Merrill wrote:
>> On 9/2/21 7:53 PM, Martin Sebor wrote:
>>> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location,
>>> tree op, tsubst_flags_t complain)
>>> if (!warn_address
>>> || (complain & tf_warning) == 0
>>> || c_inhibit_evaluation_warnings != 0
>>> - || warning_suppressed_p (op, OPT_Waddress))
>>> + || warning_suppressed_p (op, OPT_Waddress)
>>> + || processing_template_decl != 0)
>>
>> Completely suppressing this warning in templates seems like a
>> regression; I'd think we could recognize many relevant cases before
>> instantiation. You just can't assume that ADDR_EXPR has the default
>> meaning if it has unknown type (i.e. because op0 is type-dependent).
>
> I added the suppression to keep g++.dg/warn/pr101219.C from failing
> but in hindsight I should have questioned the reasoning behind
> the "no warning emitted here (no instantiation)" comment in the test.
>
> I agree that it would be helpful to diagnose the type-independent
> subset of the problem even in uninstantiated templates. Current
> trunk doesn't (it never has), but with my patch and the suppression
> above removed it does. I've updated the tests to expect it.
>
> Please see the attached revision.
>
> Martin
>
> PS There are still more opportunities to issue -Waddress in templates
> that this patch doesn't handle, e.g.,:
>
> template <class T> bool f (T *p) { return &p == 0; }
>
> Handling this will take more surgery.
>
> PPS It seems that most other warnings (and even some errors, like
> -Wnarrowing) are suppressed in uninstantiated templates as well,
> even for non-dependent expressions. In the few test cases I looked
> at Clang does better. It sounds like you'd like to see improvements
> in this direction not just for -Waddress but in general. Just for
> the avoidance of doubt, can you confirm that for future reference?
Yes, in general it's better to diagnose sooner.
> + if (TREE_CODE (cop) == NON_LVALUE_EXPR)
> + /* Unwrap the expression for C++ 98. */
> + cop = TREE_OPERAND (cop, 0);
What does this have to do with C++98?
> + if (TREE_CODE (cop) == PTRMEM_CST)
> + {
> + /* The address of a nonstatic data member is never null. */
> + warning_at (location, OPT_Waddress,
> + "the address %qE will never be NULL",
Capitalizing NULL when talking about pointers-to-members seems a bit
odd, but I guess it's fine.
The C++ changes are OK.
Jason
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-21 21:40 ` Jason Merrill
@ 2021-09-22 0:34 ` Martin Sebor
2021-09-22 20:12 ` Jason Merrill
2021-09-24 14:31 ` PING " Martin Sebor
0 siblings, 2 replies; 24+ messages in thread
From: Martin Sebor @ 2021-09-22 0:34 UTC (permalink / raw)
To: Jason Merrill, gcc-patches, Joseph S. Myers, Jeff Law
On 9/21/21 3:40 PM, Jason Merrill wrote:
> On 9/17/21 12:02, Martin Sebor wrote:
>> On 9/8/21 2:06 PM, Jason Merrill wrote:
>>> On 9/2/21 7:53 PM, Martin Sebor wrote:
>>>> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location,
>>>> tree op, tsubst_flags_t complain)
>>>> if (!warn_address
>>>> || (complain & tf_warning) == 0
>>>> || c_inhibit_evaluation_warnings != 0
>>>> - || warning_suppressed_p (op, OPT_Waddress))
>>>> + || warning_suppressed_p (op, OPT_Waddress)
>>>> + || processing_template_decl != 0)
>>>
>>> Completely suppressing this warning in templates seems like a
>>> regression; I'd think we could recognize many relevant cases before
>>> instantiation. You just can't assume that ADDR_EXPR has the default
>>> meaning if it has unknown type (i.e. because op0 is type-dependent).
>>
>> I added the suppression to keep g++.dg/warn/pr101219.C from failing
>> but in hindsight I should have questioned the reasoning behind
>> the "no warning emitted here (no instantiation)" comment in the test.
>>
>> I agree that it would be helpful to diagnose the type-independent
>> subset of the problem even in uninstantiated templates. Current
>> trunk doesn't (it never has), but with my patch and the suppression
>> above removed it does. I've updated the tests to expect it.
>>
>> Please see the attached revision.
>>
>> Martin
>>
>> PS There are still more opportunities to issue -Waddress in templates
>> that this patch doesn't handle, e.g.,:
>>
>> template <class T> bool f (T *p) { return &p == 0; }
>>
>> Handling this will take more surgery.
>>
>> PPS It seems that most other warnings (and even some errors, like
>> -Wnarrowing) are suppressed in uninstantiated templates as well,
>> even for non-dependent expressions. In the few test cases I looked
>> at Clang does better. It sounds like you'd like to see improvements
>> in this direction not just for -Waddress but in general. Just for
>> the avoidance of doubt, can you confirm that for future reference?
>
> Yes, in general it's better to diagnose sooner.
>
>> + if (TREE_CODE (cop) == NON_LVALUE_EXPR)
>> + /* Unwrap the expression for C++ 98. */
>> + cop = TREE_OPERAND (cop, 0);
>
> What does this have to do with C++98?
The code is needed to avoid failures in C++ 98 in the test below
where COP is a NON_LVALUE_EXPR which isn't handled below otherwise.
I didn't investigate why that happens (it works fine if f() is
an ordinary member function).
void f (bool);
void g ()
{
struct A { virtual void vf (); };
f (&A::vf); // missing -Waddress in C++ 98 mode
}
>
>> + if (TREE_CODE (cop) == PTRMEM_CST)
>> + {
>> + /* The address of a nonstatic data member is never null. */
>> + warning_at (location, OPT_Waddress,
>> + "the address %qE will never be NULL",
>
> Capitalizing NULL when talking about pointers-to-members seems a bit
> odd, but I guess it's fine.
I agree. My personal preference is for lowercase null (in all
languages) since that's the technical term for it. I used NULL
here only to conform to the existing style. I'm willing to
change all these warnings to either use null or to some form
that doesn't mention null (there are two in use, although if
I had my druthers I'd choose some other phrasing altogether).
Let me know if you would support such a change.
>
> The C++ changes are OK.
Jeff, should I take your previous "Generally OK" as an approval
for the rest of the patch as well? (It has not changed in v2.)
I have just submitted a Glibc patch to suppress the new instances
there.
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-22 0:34 ` Martin Sebor
@ 2021-09-22 20:12 ` Jason Merrill
2021-09-24 14:31 ` PING " Martin Sebor
1 sibling, 0 replies; 24+ messages in thread
From: Jason Merrill @ 2021-09-22 20:12 UTC (permalink / raw)
To: Martin Sebor, gcc-patches, Joseph S. Myers, Jeff Law
On 9/21/21 20:34, Martin Sebor wrote:
> On 9/21/21 3:40 PM, Jason Merrill wrote:
>> On 9/17/21 12:02, Martin Sebor wrote:
>>> On 9/8/21 2:06 PM, Jason Merrill wrote:
>>>> On 9/2/21 7:53 PM, Martin Sebor wrote:
>>>>> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location,
>>>>> tree op, tsubst_flags_t complain)
>>>>> if (!warn_address
>>>>> || (complain & tf_warning) == 0
>>>>> || c_inhibit_evaluation_warnings != 0
>>>>> - || warning_suppressed_p (op, OPT_Waddress))
>>>>> + || warning_suppressed_p (op, OPT_Waddress)
>>>>> + || processing_template_decl != 0)
>>>>
>>>> Completely suppressing this warning in templates seems like a
>>>> regression; I'd think we could recognize many relevant cases before
>>>> instantiation. You just can't assume that ADDR_EXPR has the default
>>>> meaning if it has unknown type (i.e. because op0 is type-dependent).
>>>
>>> I added the suppression to keep g++.dg/warn/pr101219.C from failing
>>> but in hindsight I should have questioned the reasoning behind
>>> the "no warning emitted here (no instantiation)" comment in the test.
>>>
>>> I agree that it would be helpful to diagnose the type-independent
>>> subset of the problem even in uninstantiated templates. Current
>>> trunk doesn't (it never has), but with my patch and the suppression
>>> above removed it does. I've updated the tests to expect it.
>>>
>>> Please see the attached revision.
>>>
>>> Martin
>>>
>>> PS There are still more opportunities to issue -Waddress in templates
>>> that this patch doesn't handle, e.g.,:
>>>
>>> template <class T> bool f (T *p) { return &p == 0; }
>>>
>>> Handling this will take more surgery.
>>>
>>> PPS It seems that most other warnings (and even some errors, like
>>> -Wnarrowing) are suppressed in uninstantiated templates as well,
>>> even for non-dependent expressions. In the few test cases I looked
>>> at Clang does better. It sounds like you'd like to see improvements
>>> in this direction not just for -Waddress but in general. Just for
>>> the avoidance of doubt, can you confirm that for future reference?
>>
>> Yes, in general it's better to diagnose sooner.
>>
>>> + if (TREE_CODE (cop) == NON_LVALUE_EXPR)
>>> + /* Unwrap the expression for C++ 98. */
>>> + cop = TREE_OPERAND (cop, 0);
>>
>> What does this have to do with C++98?
>
> The code is needed to avoid failures in C++ 98 in the test below
> where COP is a NON_LVALUE_EXPR which isn't handled below otherwise.
> I didn't investigate why that happens (it works fine if f() is
> an ordinary member function).
>
> void f (bool);
>
> void g ()
> {
> struct A { virtual void vf (); };
>
> f (&A::vf); // missing -Waddress in C++ 98 mode
> }
>
>>
>>> + if (TREE_CODE (cop) == PTRMEM_CST)
>>> + {
>>> + /* The address of a nonstatic data member is never null. */
>>> + warning_at (location, OPT_Waddress,
>>> + "the address %qE will never be NULL",
>>
>> Capitalizing NULL when talking about pointers-to-members seems a bit
>> odd, but I guess it's fine.
>
> I agree. My personal preference is for lowercase null (in all
> languages) since that's the technical term for it. I used NULL
> here only to conform to the existing style. I'm willing to
> change all these warnings to either use null or to some form
> that doesn't mention null (there are two in use, although if
> I had my druthers I'd choose some other phrasing altogether).
> Let me know if you would support such a change.
I don't feel strongly about it. I agree that lowercase or another
phrasing would be better, but probably better to avoid adding work for
the translators with the churn.
>> The C++ changes are OK.
>
> Jeff, should I take your previous "Generally OK" as an approval
> for the rest of the patch as well? (It has not changed in v2.)
> I have just submitted a Glibc patch to suppress the new instances
> there.
>
> Martin
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* PING [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-22 0:34 ` Martin Sebor
2021-09-22 20:12 ` Jason Merrill
@ 2021-09-24 14:31 ` Martin Sebor
2021-09-30 16:14 ` PING #2 " Martin Sebor
1 sibling, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-24 14:31 UTC (permalink / raw)
To: Jeff Law; +Cc: Jason Merrill, gcc-patches, Joseph S. Myers
Ping: Jeff, with the C++ part approved, can you please confirm your
approval with the C parts of the patch?
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579693.html
On 9/21/21 6:34 PM, Martin Sebor wrote:
> On 9/21/21 3:40 PM, Jason Merrill wrote:
....
>> The C++ changes are OK.
>
> Jeff, should I take your previous "Generally OK" as an approval
> for the rest of the patch as well? (It has not changed in v2.)
> I have just submitted a Glibc patch to suppress the new instances
> there.
>
> Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
* PING #2 [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-24 14:31 ` PING " Martin Sebor
@ 2021-09-30 16:14 ` Martin Sebor
2021-09-30 19:35 ` Joseph Myers
0 siblings, 1 reply; 24+ messages in thread
From: Martin Sebor @ 2021-09-30 16:14 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jeff Law, gcc-patches, Joseph S. Myers
Jason, since you approved the C++ changes, would you mind looking
over the C bits and if they look good to you giving me the green
light to commit the patch?
https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579693.html
Thanks in advance for your help!
On 9/24/21 8:31 AM, Martin Sebor wrote:
> Ping: Jeff, with the C++ part approved, can you please confirm your
> approval with the C parts of the patch?
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579693.html
>
> On 9/21/21 6:34 PM, Martin Sebor wrote:
>> On 9/21/21 3:40 PM, Jason Merrill wrote:
> ....
>>> The C++ changes are OK.
>>
>> Jeff, should I take your previous "Generally OK" as an approval
>> for the rest of the patch as well? (It has not changed in v2.)
>> I have just submitted a Glibc patch to suppress the new instances
>> there.
>>
>> Martin
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PING #2 [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-30 16:14 ` PING #2 " Martin Sebor
@ 2021-09-30 19:35 ` Joseph Myers
2021-10-01 17:58 ` Martin Sebor
0 siblings, 1 reply; 24+ messages in thread
From: Joseph Myers @ 2021-09-30 19:35 UTC (permalink / raw)
To: Martin Sebor; +Cc: Jason Merrill, gcc-patches
On Thu, 30 Sep 2021, Martin Sebor via Gcc-patches wrote:
> Jason, since you approved the C++ changes, would you mind looking
> over the C bits and if they look good to you giving me the green
> light to commit the patch?
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579693.html
The C changes are OK, with two instances of "for the address %qE will
never be NULL" fixed to refer to the address *of* %qE as elsewhere (those
are for IMAGPART_EXPR and REALPART_EXPR; C++ also has one "the address %qE
will never be NULL"), and the "pr??????" in the tests filled in with an
actual PR number for the XFAILed cases.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PING #2 [PATCH] warn for more impossible null pointer tests [PR102103]
2021-09-30 19:35 ` Joseph Myers
@ 2021-10-01 17:58 ` Martin Sebor
0 siblings, 0 replies; 24+ messages in thread
From: Martin Sebor @ 2021-10-01 17:58 UTC (permalink / raw)
To: Joseph Myers; +Cc: Jason Merrill, gcc-patches
On 9/30/21 1:35 PM, Joseph Myers wrote:
> On Thu, 30 Sep 2021, Martin Sebor via Gcc-patches wrote:
>
>> Jason, since you approved the C++ changes, would you mind looking
>> over the C bits and if they look good to you giving me the green
>> light to commit the patch?
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579693.html
>
> The C changes are OK, with two instances of "for the address %qE will
> never be NULL" fixed to refer to the address *of* %qE as elsewhere (those
> are for IMAGPART_EXPR and REALPART_EXPR; C++ also has one "the address %qE
> will never be NULL"), and the "pr??????" in the tests filled in with an
> actual PR number for the XFAILed cases.
Thanks for the careful review and the approval!
I remember having a reason for dropping the "of" in the two instances
in the C FE but after double-checking the output I see you're right
that it should be there. Good catch!
I believe the C++ instance is correct. It's issued for the address
of a member, as in the former of the two below:
struct S { int i; };
bool f ()
{
return &S::i == 0; // the address ‘&S::i’
}
bool g (S *p)
{
return &p->i == 0; // the address of ‘S::i’
}
z.c: In function ‘bool f()’:
z.c:5:16: warning: the address ‘&S::i’ will never be NULL [-Waddress]
5 | return &S::i == 0;
| ~~~~~~^~~~
z.c: In function ‘bool g(S*)’:
z.c:10:16: warning: the address of ‘S::i’ will never be NULL [-Waddress]
10 | return &p->i == 0;
| ~~~~~~^~~~
z.c:1:16: note: ‘S::i’ declared here
1 | struct S { int i; };
| ^
I've beefed up the tests to verify the expected wording.
Thanks also for prompting me to open bugs for the xfailed tests,
something I tend to forget to do when it depends on the patch I'm
developing. I've raised pr102555 for the C FE folding defeating
the warning. The C++ bug that tracks the xfails in the C++ tests
is pr102378.
I've pushed the updated patch in r12-4059 after retesting the whole
thing on x86_64-linux.
Martin
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-10-01 17:58 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 2:08 [PATCH] warn for more impossible null pointer tests Martin Sebor
2021-09-01 17:35 ` Jeff Law
2021-09-01 18:57 ` Koning, Paul
2021-09-01 19:08 ` Jeff Law
2021-09-01 19:28 ` Koning, Paul
2021-09-01 19:35 ` Iain Sandoe
2021-09-01 19:58 ` Andreas Schwab
2021-09-01 20:36 ` Koning, Paul
2021-09-01 19:21 ` Jason Merrill
2021-09-01 20:33 ` Martin Sebor
2021-09-01 21:39 ` Jason Merrill
2021-09-01 22:27 ` Martin Sebor
2021-09-02 13:43 ` Jason Merrill
2021-09-02 14:39 ` Martin Sebor
2021-09-02 23:53 ` [PATCH] warn for more impossible null pointer tests [PR102103] Martin Sebor
2021-09-08 20:06 ` Jason Merrill
2021-09-17 16:02 ` Martin Sebor
2021-09-21 21:40 ` Jason Merrill
2021-09-22 0:34 ` Martin Sebor
2021-09-22 20:12 ` Jason Merrill
2021-09-24 14:31 ` PING " Martin Sebor
2021-09-30 16:14 ` PING #2 " Martin Sebor
2021-09-30 19:35 ` Joseph Myers
2021-10-01 17:58 ` Martin Sebor
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).