This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH RFA: C++ frontend warning: comparing function pointer to NULL
Mark Mitchell <mark@codesourcery.com> writes:
> I hate to be niggly, but it looks like you've introduced two copies of
> the same test here (modulo op0 vs. op1), and it sounds like the C front
> end has at least one more copy. Would you mind creating a function,
> shared as widely as possible?
How about this version? Since I changed the C frontend, I added C
test cases too (they are actually exactly the same as the C++ test
cases, except for comment syntax).
Bootstrapped on i686-pc-linux-gnu, testsuite run in progress.
Ian
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c (revision 120376)
+++ gcc/c-common.c (working copy)
@@ -2591,6 +2591,18 @@ pointer_int_sum (enum tree_code resultco
return fold_build2 (resultcode, result_type, ptrop, intop);
}
+/* Return whether EXPR is a declaration whose address can never be
+ NULL. */
+
+bool
+decl_with_nonnull_addr_p (tree expr)
+{
+ return (DECL_P (expr)
+ && (TREE_CODE (expr) == PARM_DECL
+ || TREE_CODE (expr) == LABEL_DECL
+ || !DECL_WEAK (expr)));
+}
+
/* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
or for an `if' or `while' statement or ?..: exp. It should already
have been validated to be of suitable type; otherwise, a bad
@@ -2656,23 +2668,22 @@ c_common_truthvalue_conversion (tree exp
case ADDR_EXPR:
{
tree inner = TREE_OPERAND (expr, 0);
- if (DECL_P (inner)
- && (TREE_CODE (inner) == PARM_DECL
- || TREE_CODE (inner) == LABEL_DECL
- || !DECL_WEAK (inner)))
+ if (decl_with_nonnull_addr_p (inner))
{
- /* Common Ada/Pascal programmer's mistake. We always warn
- about this since it is so bad. */
- warning (OPT_Walways_true, "the address of %qD will always evaluate as %<true%>",
+ /* Common Ada/Pascal programmer's mistake. */
+ warning (OPT_Walways_true,
+ "the address of %qD will always evaluate as %<true%>",
inner);
return truthvalue_true_node;
}
- /* If we are taking the address of an external decl, it might be
- zero if it is weak, so we cannot optimize. */
- if (DECL_P (inner)
- && DECL_EXTERNAL (inner))
- break;
+ /* If we still have a decl, it is possible for its address to
+ be NULL, so we cannot optimize. */
+ if (DECL_P (inner))
+ {
+ gcc_assert (DECL_WEAK (inner));
+ break;
+ }
if (TREE_SIDE_EFFECTS (inner))
return build2 (COMPOUND_EXPR, truthvalue_type_node,
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c (revision 120376)
+++ gcc/c-typeck.c (working copy)
@@ -8013,10 +8013,7 @@ build_binary_op (enum tree_code code, tr
else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
{
if (TREE_CODE (op0) == ADDR_EXPR
- && DECL_P (TREE_OPERAND (op0, 0))
- && (TREE_CODE (TREE_OPERAND (op0, 0)) == PARM_DECL
- || TREE_CODE (TREE_OPERAND (op0, 0)) == LABEL_DECL
- || !DECL_WEAK (TREE_OPERAND (op0, 0))))
+ && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
warning (OPT_Walways_true, "the address of %qD will never be NULL",
TREE_OPERAND (op0, 0));
result_type = type0;
@@ -8024,10 +8021,7 @@ build_binary_op (enum tree_code code, tr
else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
{
if (TREE_CODE (op1) == ADDR_EXPR
- && DECL_P (TREE_OPERAND (op1, 0))
- && (TREE_CODE (TREE_OPERAND (op1, 0)) == PARM_DECL
- || TREE_CODE (TREE_OPERAND (op1, 0)) == LABEL_DECL
- || !DECL_WEAK (TREE_OPERAND (op1, 0))))
+ && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
warning (OPT_Walways_true, "the address of %qD will never be NULL",
TREE_OPERAND (op1, 0));
result_type = type1;
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h (revision 120376)
+++ gcc/c-common.h (working copy)
@@ -652,6 +652,7 @@ extern tree c_common_unsigned_type (tree
extern tree c_common_signed_type (tree);
extern tree c_common_signed_or_unsigned_type (int, tree);
extern tree c_build_bitfield_integer_type (unsigned HOST_WIDE_INT, int);
+extern bool decl_with_nonnull_addr_p (tree);
extern tree c_common_truthvalue_conversion (tree);
extern void c_apply_type_quals_to_decl (int, tree);
extern tree c_sizeof_or_alignof_type (tree, bool, int);
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c (revision 120376)
+++ gcc/cp/typeck.c (working copy)
@@ -3334,10 +3334,22 @@ build_binary_op (enum tree_code code, tr
"comparison");
else if ((code0 == POINTER_TYPE || TYPE_PTRMEM_P (type0))
&& null_ptr_cst_p (op1))
- result_type = type0;
+ {
+ if (TREE_CODE (op0) == ADDR_EXPR
+ && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
+ warning (OPT_Walways_true, "the address of %qD will never be NULL",
+ TREE_OPERAND (op0, 0));
+ result_type = type0;
+ }
else if ((code1 == POINTER_TYPE || TYPE_PTRMEM_P (type1))
&& null_ptr_cst_p (op0))
- result_type = type1;
+ {
+ if (TREE_CODE (op1) == ADDR_EXPR
+ && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
+ warning (OPT_Walways_true, "the address of %qD will never be NULL",
+ TREE_OPERAND (op1, 0));
+ result_type = type1;
+ }
else if (code0 == POINTER_TYPE && code1 == INTEGER_TYPE)
{
result_type = type0;
Index: gcc/testsuite/gcc.dg/Walways-true-1.c
===================================================================
--- gcc/testsuite/gcc.dg/Walways-true-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/Walways-true-1.c (revision 0)
@@ -0,0 +1,57 @@
+/* Test -Walways-true for testing an address against NULL.
+ Origin: Ian Lance Taylor <iant@google.com>. */
+
+/* { dg-do compile} */
+/* { dg-options "-Walways-true" } */
+
+extern int foo (int);
+
+int i;
+
+void
+bar (int a)
+{
+ lab:
+ if (foo) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (0);
+ if (foo (1))
+ ;
+ if (&i) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (2);
+ if (i)
+ foo (3);
+ if (&a) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (4);
+ if (a)
+ foo (5);
+ if (&&lab) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (6);
+ if (foo == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (7);
+ if (foo (1) == 0)
+ foo (8);
+ if (&i == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (9);
+ if (i == 0)
+ foo (10);
+ if (&a == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (11);
+ if (a == 0)
+ foo (12);
+ if (&&lab == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (13);
+ if (0 == foo) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (14);
+ if (0 == foo (1))
+ foo (15);
+ if (0 == &i) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (16);
+ if (0 == i)
+ foo (17);
+ if (0 == &a) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (18);
+ if (0 == a)
+ foo (19);
+ if (0 == &&lab) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (20);
+}
Index: gcc/testsuite/gcc.dg/Walways-true-2.c
===================================================================
--- gcc/testsuite/gcc.dg/Walways-true-2.c (revision 0)
+++ gcc/testsuite/gcc.dg/Walways-true-2.c (revision 0)
@@ -0,0 +1,60 @@
+/* Make sure we don't assume that a weak symbol is always non-NULL.
+ This is just like Walways-true-1.C, except that it uses a weak
+ symbol.
+ Origin: Ian Lance Taylor <iant@google.com>. */
+
+/* { dg-do compile} */
+/* { dg-options "-Walways-true" } */
+/* { dg-require-weak "" } */
+
+extern int foo (int) __attribute__ ((weak));
+
+int i __attribute__ ((weak));
+
+void
+bar (int a)
+{
+ lab:
+ if (foo)
+ foo (0);
+ if (foo (1))
+ ;
+ if (&i)
+ foo (2);
+ if (i)
+ foo (3);
+ if (&a) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (4);
+ if (a)
+ foo (5);
+ if (&&lab) /* { dg-warning "always evaluate as" "correct warning" } */
+ foo (6);
+ if (foo == 0)
+ foo (7);
+ if (foo (1) == 0)
+ foo (8);
+ if (&i == 0)
+ foo (9);
+ if (i == 0)
+ foo (10);
+ if (&a == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (11);
+ if (a == 0)
+ foo (12);
+ if (&&lab == 0) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (13);
+ if (0 == foo)
+ foo (14);
+ if (0 == foo (1))
+ foo (15);
+ if (0 == &i)
+ foo (16);
+ if (0 == i)
+ foo (17);
+ if (0 == &a) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (18);
+ if (0 == a)
+ foo (19);
+ if (0 == &&lab) /* { dg-warning "never be NULL" "correct warning" } */
+ foo (20);
+}
Index: gcc/testsuite/g++.dg/warn/Walways-true-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Walways-true-1.C (revision 0)
+++ gcc/testsuite/g++.dg/warn/Walways-true-1.C (revision 0)
@@ -0,0 +1,57 @@
+// Test -Walways-true for testing an address against NULL.
+// Origin: Ian Lance Taylor <iant@google.com>
+
+// { dg-do compile}
+// { dg-options "-Walways-true" }
+
+extern int foo (int);
+
+int i;
+
+void
+bar (int a)
+{
+ lab:
+ if (foo) // { dg-warning "always evaluate as" "correct warning" }
+ foo (0);
+ if (foo (1))
+ ;
+ if (&i) // { dg-warning "always evaluate as" "correct warning" }
+ foo (2);
+ if (i)
+ foo (3);
+ if (&a) // { dg-warning "always evaluate as" "correct warning" }
+ foo (4);
+ if (a)
+ foo (5);
+ if (&&lab) // { dg-warning "always evaluate as" "correct warning" }
+ foo (6);
+ if (foo == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (7);
+ if (foo (1) == 0)
+ foo (8);
+ if (&i == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (9);
+ if (i == 0)
+ foo (10);
+ if (&a == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (11);
+ if (a == 0)
+ foo (12);
+ if (&&lab == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (13);
+ if (0 == foo) // { dg-warning "never be NULL" "correct warning" }
+ foo (14);
+ if (0 == foo (1))
+ foo (15);
+ if (0 == &i) // { dg-warning "never be NULL" "correct warning" }
+ foo (16);
+ if (0 == i)
+ foo (17);
+ if (0 == &a) // { dg-warning "never be NULL" "correct warning" }
+ foo (18);
+ if (0 == a)
+ foo (19);
+ if (0 == &&lab) // { dg-warning "never be NULL" "correct warning" }
+ foo (20);
+}
Index: gcc/testsuite/g++.dg/warn/Walways-true-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Walways-true-2.C (revision 0)
+++ gcc/testsuite/g++.dg/warn/Walways-true-2.C (revision 0)
@@ -0,0 +1,60 @@
+// Make sure we don't assume that a weak symbol is always non-NULL.
+// This is just like Walways-true-1.C, except that it uses a weak
+// symbol.
+// Origin: Ian Lance Taylor <iant@google.com>
+
+// { dg-do compile}
+// { dg-options "-Walways-true" }
+// { dg-require-weak "" }
+
+extern int foo (int) __attribute__ ((weak));
+
+int i __attribute__ ((weak));
+
+void
+bar (int a)
+{
+ lab:
+ if (foo)
+ foo (0);
+ if (foo (1))
+ ;
+ if (&i)
+ foo (2);
+ if (i)
+ foo (3);
+ if (&a) // { dg-warning "always evaluate as" "correct warning" }
+ foo (4);
+ if (a)
+ foo (5);
+ if (&&lab) // { dg-warning "always evaluate as" "correct warning" }
+ foo (6);
+ if (foo == 0)
+ foo (7);
+ if (foo (1) == 0)
+ foo (8);
+ if (&i == 0)
+ foo (9);
+ if (i == 0)
+ foo (10);
+ if (&a == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (11);
+ if (a == 0)
+ foo (12);
+ if (&&lab == 0) // { dg-warning "never be NULL" "correct warning" }
+ foo (13);
+ if (0 == foo)
+ foo (14);
+ if (0 == foo (1))
+ foo (15);
+ if (0 == &i)
+ foo (16);
+ if (0 == i)
+ foo (17);
+ if (0 == &a) // { dg-warning "never be NULL" "correct warning" }
+ foo (18);
+ if (0 == a)
+ foo (19);
+ if (0 == &&lab) // { dg-warning "never be NULL" "correct warning" }
+ foo (20);
+}