[PATCH 3/3] [RFC] Treat a gimplification failure as an internal error

Patrick Palka patrick@parcs.ath.cx
Thu Dec 31 15:40:00 GMT 2015


This patch makes it so that a gimplification failure is considered to be
an internal error under normal circumstances, so that we otherwise avoid
silently generating wrong code if e.g. a buggy frontend fed us a
malformed tree.

The rationale for this change is that it's better to abort compilation
than to silently generate wrong code.  During gimplification we should
only see e.g. an error_mark_node if the frontend has already issued an
error.  Otherwise it is likely a bug in frontend.

This patch, for example, turns the PR c++/68948 wrong-code bug into an
ICE on invalid bug.  During testing it also caught two latent "bugs"
(patches 1 and 2 in this series).

This series was tested on x86_64-pc-linux-gnu, with --enable-languages=all,ada,go,
no new regressions.

Does this seem like a reasonable invariant to add to the gimplifier?

gcc/cp/ChangeLog:

	* cp-gimplify.c (gimplify_expr_stmt): Don't convert an
	error_mark_node to an empty statement.

gcc/ChangeLog:

	* gimplify.c (gimplify_return_expr): Remove a redundant test
	for error_mark_node.
	(gimplify_decl_expr): Return GS_ERROR if an initializer is an
	error_mark_node.
	(gimplify_expr): Treat a gimplification failure as an internal
	error.  Remove now-redundant GIMPLE_CHECKING checking code.
---
 gcc/cp/cp-gimplify.c |  5 +----
 gcc/gimplify.c       | 27 +++++++++++++--------------
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 373c9e1..2b0aaaf 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -424,16 +424,13 @@ gimplify_expr_stmt (tree *stmt_p)
 {
   tree stmt = EXPR_STMT_EXPR (*stmt_p);
 
-  if (stmt == error_mark_node)
-    stmt = NULL;
-
   /* Gimplification of a statement expression will nullify the
      statement if all its side effects are moved to *PRE_P and *POST_P.
 
      In this case we will not want to emit the gimplified statement.
      However, we may still want to emit a warning, so we do that before
      gimplification.  */
-  if (stmt && warn_unused_value)
+  if (stmt && stmt != error_mark_node && warn_unused_value)
     {
       if (!TREE_SIDE_EFFECTS (stmt))
 	{
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index bc90401..9a1d723 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1288,8 +1288,7 @@ gimplify_return_expr (tree stmt, gimple_seq *pre_p)
     }
 
   if (!ret_expr
-      || TREE_CODE (ret_expr) == RESULT_DECL
-      || ret_expr == error_mark_node)
+      || TREE_CODE (ret_expr) == RESULT_DECL)
     {
       greturn *ret = gimple_build_return (ret_expr);
       gimple_set_no_warning (ret, TREE_NO_WARNING (stmt));
@@ -1449,6 +1448,9 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
     {
       tree init = DECL_INITIAL (decl);
 
+      if (init == error_mark_node)
+	return GS_ERROR;
+
       if (TREE_CODE (DECL_SIZE_UNIT (decl)) != INTEGER_CST
 	  || (!TREE_STATIC (decl)
 	      && flag_stack_check == GENERIC_STACK_CHECK
@@ -1464,7 +1466,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
 	  && DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
 	gimple_add_tmp_var (decl);
 
-      if (init && init != error_mark_node)
+      if (init)
 	{
 	  if (!TREE_STATIC (decl))
 	    {
@@ -11036,17 +11038,6 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
     }
   else
     {
-#ifdef ENABLE_GIMPLE_CHECKING
-      if (!(fallback & fb_mayfail))
-	{
-	  fprintf (stderr, "gimplification failed:\n");
-	  print_generic_expr (stderr, *expr_p, 0);
-	  debug_tree (*expr_p);
-	  internal_error ("gimplification failed");
-	}
-#endif
-      gcc_assert (fallback & fb_mayfail);
-
       /* If this is an asm statement, and the user asked for the
 	 impossible, don't die.  Fail and let gimplify_asm_expr
 	 issue an error.  */
@@ -11064,6 +11055,14 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
     }
 
  out:
+  /* If gimplification failed, then either such failure was explicitly permitted
+     (via the FB_MAYFAIL flag) or the frontend fed us a malformed tree, e.g. one
+     containing an ERROR_MARK node -- for which the FE should have already issued an
+     error diagnostic.  Otherwise it is likely that an FE bug was triggered, in
+     which case it is better to abort than to risk silently generating wrong
+     code.  */
+  if (ret == GS_ERROR)
+    gcc_assert ((fallback & fb_mayfail) || seen_error ());
   input_location = saved_location;
   return ret;
 }
-- 
2.7.0.rc3.56.g64157c6.dirty



More information about the Gcc-patches mailing list