This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix a C++ -Wnonnull-compare regression (PR c++/69850)


Hi!

As -Wnonnull -> -Wnonnull-compare warning has been moved from FE
to after going into SSA form, we have a problem because
delete ptr;
in C++ may introduce a comparison on its own, and on that artificial
comparison can complain about comparison of nonnull argument with NULL.

The following patch fixes this by setting TREE_NO_WARNING on it, making sure
it propagates up to the warn_nonnull_compare pass and not warning if it is
set.

While for "this" (after stripping nops) perhaps already the C++ FE could
optimize and assume this is always non-NULL (i.e. not generate the
comparison at all), in other cases, even if the arg is nonnull_arg_p, it
can't assume that, for the exact same reasons why the warning had to be
moved into the middle-end.  Because we can have:
__attribute__((nonnull)) void
foo (S *p, S *q, int a)
{
  if (a == 1)
    delete p; // here if (p != NULL) could be optimized away, but the
	      // FE doesn't know that
  else if (a == 2)
    {
      bar (&q); // q could have changed here.
      delete q; // so, eventhough nonnull_arg_p (q), we have to if (q != NULL)
    }
  else if (a == 3)
    {
      p = baz (); // p value has changed, thus we need to test
      delete p;   // if (p != NULL) here.
    }
}
In any case, as the source doesn't contain comparison of p != NULL in the a
== 1 case, we don't want to warn.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-02-17  Jakub Jelinek  <jakub@redhat.com>

	PR c++/69850
	* gimplify.c (gimplify_cond_expr): Call gimple_set_no_warning
	on the cond_stmt from TREE_NO_WARNING on COND_EXPR_COND.
	* gimple-ssa-nonnull-compare.c (do_warn_nonnull_compare): Don't
	warn on gimple_no_warning_p statements.

	* init.c (build_delete): Set TREE_NO_WARNING on ifexp.

	* g++.dg/warn/Wnonnull-compare-1.C: New test.

--- gcc/gimplify.c.jj	2016-02-16 21:42:57.000000000 +0100
+++ gcc/gimplify.c	2016-02-17 14:01:45.789728763 +0100
@@ -3219,6 +3219,7 @@ gimplify_cond_expr (tree *expr_p, gimple
 				 &arm2);
   cond_stmt = gimple_build_cond (pred_code, arm1, arm2, label_true,
 				 label_false);
+  gimple_set_no_warning (cond_stmt, TREE_NO_WARNING (COND_EXPR_COND (expr)));
   gimplify_seq_add_stmt (&seq, cond_stmt);
   gimple_stmt_iterator gsi = gsi_last (seq);
   maybe_fold_stmt (&gsi);
--- gcc/gimple-ssa-nonnull-compare.c.jj	2016-02-16 21:46:02.000000000 +0100
+++ gcc/gimple-ssa-nonnull-compare.c	2016-02-17 13:51:52.933996279 +0100
@@ -96,7 +96,8 @@ do_warn_nonnull_compare (function *fun,
 	  }
       if (op
 	  && (POINTER_TYPE_P (TREE_TYPE (arg))
-	      ? integer_zerop (op) : integer_minus_onep (op)))
+	      ? integer_zerop (op) : integer_minus_onep (op))
+	  && !gimple_no_warning_p (stmt))
 	warning_at (loc, OPT_Wnonnull_compare,
 		    "nonnull argument %qD compared to NULL", arg);
     }
--- gcc/cp/init.c.jj	2016-02-15 23:04:42.000000000 +0100
+++ gcc/cp/init.c	2016-02-17 13:54:32.597769729 +0100
@@ -4525,6 +4525,10 @@ build_delete (tree otype, tree addr, spe
 					    complain));
 	  if (ifexp == error_mark_node)
 	    return error_mark_node;
+	  /* This is a compiler generated comparison, don't emit
+	     e.g. -Wnonnull-compare warning for it.  */
+	  else if (TREE_CODE (ifexp) == NE_EXPR)
+	    TREE_NO_WARNING (ifexp) = 1;
 	}
 
       if (ifexp != integer_one_node)
--- gcc/testsuite/g++.dg/warn/Wnonnull-compare-1.C.jj	2016-02-17 14:04:51.644136979 +0100
+++ gcc/testsuite/g++.dg/warn/Wnonnull-compare-1.C	2016-02-17 14:04:24.000000000 +0100
@@ -0,0 +1,9 @@
+// PR c++/69850
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+struct C
+{
+  ~C () { delete this; }	// { dg-bogus "nonnull argument" }
+};
+C c;

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]