diff mbox

[C++] Issue hard error even with -fpermissive for certain goto violations (PR c++/67409)

Message ID 20151118225520.GN5675@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 18, 2015, 10:55 p.m. UTC 
Hi!

Before Paolo's changes, e.g. identify_goto always used permerror, and then
depending on the severity of the issue either issued another permerror
describing in detail the issue, or error.
In particular, entering scope of a decl with non-trivial dtor has been
a permerror, pretty much everything else has been a hard error.
The PR Paolo has been fixing was a request to allow with -fpermissive
crossing initialization (i.e. just warn in that case, instead of hard
error), but the patch for consistency turned all the permerror; error
cases to if (permerror ()) inform, which means also other kinds of goto
errors can now be ignored with -fpermissive.  But, as the testcase shows,
in certain cases we ICE on it later on; e.g. the OpenMP SESE violations
definitely should be hard errors, because of the ICEs also the entering of
try or catch blocks through goto, TM violations etc.

The following patch arranges for using error instead of permerror
in the case of the more severe violations; if a single goto violates both
something less severe (first) and then something more severe, we can emit
e.g. the jump to label ... diagnostics twice, for non-permissive once
with [-fpermissive] after it, once without, for -fpermissive once as a
warning, once as an error.

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

2015-11-18  Jakub Jelinek  <jakub@redhat.com>

	PR c++/67409
	* decl.c (identify_goto): Add harderr argument, call error instead of
	permerror if it is true.
	(check_previous_goto_1): Adjust identify_goto callers, treat all cases
	but crossing initialization and entering scope of decl with non-trivial
	dtor as unconditional hard errors.
	(check_goto): Treat all cases but crossing initialization and entering
	scope of decl with non-trivial dtor as unconditional hard errors.

	* g++.dg/eh/goto3.C: New test.


	Jakub

Comments

Manuel López-Ibáñez Nov. 19, 2015, 2:52 a.m. UTC | #1 
On 18/11/15 22:55, Jakub Jelinek wrote:

>   static bool
> -identify_goto (tree decl, const location_t *locus)
> +identify_goto (tree decl, const location_t *locus, bool harderr)
>   {
> -  bool complained = (decl
> -		     ? permerror (input_location, "jump to label %qD", decl)
> -		     : permerror (input_location, "jump to case label"));
> +  bool complained;
> +  if (!harderr)
> +    {
> +      if (decl)
> +	complained = permerror (input_location, "jump to label %qD", decl);
> +      else
> +	complained = permerror (input_location, "jump to case label");
> +    }
> +  else
> +    {
> +      if (decl)
> +	error ("jump to label %qD", decl);
> +      else
> +	error ("jump to case label");
> +      complained = true;
> +    }
>     if (complained && locus)
>       inform (*locus, "  from here");
>     return complained;

The above is a bit repetitive. Why not simply?

static bool
error_jumpto (diagnostic_t kind, location_t loc, tree decl)
{
   bool complained = (decl
		     ? emit_diagnostic (kind, input_location, 0,
					"jump to label %qD", decl)
		     : emit_diagnostic (kind, input_location, 0,
					"jump to case label"));
   if (complained && loc)
     inform (loc, " from here");
   return complained;
}

That is, call a function that gives errors about X, error_X; no point in 
passing a pointer to location_t; most diagnostic functions take loc as the 
first argument; no obscure bool parameter. Then call:

> @@ -2991,15 +3004,16 @@ check_previous_goto_1 (tree decl, cp_bin
>   		       bool exited_omp, const location_t *locus)
>   {
>     cp_binding_level *b;
> -  bool identified = false, complained = false;
> +  bool complained = false;
> +  int identified = 0;
>     bool saw_eh = false, saw_omp = false, saw_tm = false;
>
>     if (exited_omp)
>       {
> -      complained = identify_goto (decl, locus);
> +      complained = identify_goto (decl, locus, true);

complained = error_jumpto (DK_ERROR, loc, decl);

> +	      complained = identify_goto (decl, locus, false);

complained = error_jumpto (DK_PERMERROR, loc, decl);


> +      if (ent->in_try_scope || ent->in_catch_scope
> +	  || ent->in_transaction_scope || ent->in_omp_scope)
> +	{
> +	  error_at (DECL_SOURCE_LOCATION (decl), "jump to label %qD", decl);
> +	  complained = true;
> +	  identified = 2;
> +	}
> +      else
> +	{
> +	  complained = permerror (DECL_SOURCE_LOCATION (decl),
> +				  "jump to label %qD", decl);
> +	  identified = 1;
> +	}
>        if (complained)
>  	inform (input_location, "  from here");

Note that if the function above takes another location_t argument, you can also 
simplify this hunk to:

       diagnostic_t kind;
       if (ent->in_try_scope || ent->in_catch_scope
	  || ent->in_transaction_scope || ent->in_omp_scope)
	{
           kind = DK_ERROR;
	  identified = 2;
	}
        else
	{
	  kind = DK_PERMERROR;
	  identified = 1;
	}
	complained = error_jumpto (kind, loc, DECL_SOURCE_LOCATION (decl), decl);

You can even use kind (maybe 'error_kind') directly instead of identified for 
what you are trying to achieve, with error_kind in {DK_UNSPECIFIED, DK_ERROR, 
DK_PERMERROR}.


>
>     FOR_EACH_VEC_SAFE_ELT (ent->bad_decls, ix, bad)
> @@ -3155,6 +3180,14 @@ check_goto (tree decl)
>         if (u > 1 && DECL_ARTIFICIAL (bad))
>   	{
>   	  /* Can't skip init of __exception_info.  */
> +	  if (identified == 1)
> +	    {
> +	      error_at (DECL_SOURCE_LOCATION (decl),
> +			"jump to label %qD", decl);
> +	      inform (input_location, "  from here");
> +	      complained = true;
> +	      identified = 2;
> +	    }

and here:

  kind = DK_ERROR;
  complained = error_jumpto (kind, input_location,
                             DECL_SOURCE_LOCATION (decl), decl);

>   	  if (complained)
>   	    inform (DECL_SOURCE_LOCATION (bad), "  enters catch block");
>   	  saw_catch = true;
> @@ -3195,13 +3228,13 @@ check_goto (tree decl)
>   	    break;
>   	  if (b->kind == sk_omp)
>   	    {
> -	      if (!identified)
> +	      if (identified < 2)
>   		{
> -		  complained = permerror (DECL_SOURCE_LOCATION (decl),
> -					  "jump to label %qD", decl);
> -		  if (complained)
> -		    inform (input_location, "  from here");
> -		  identified = true;
> +		  error_at (DECL_SOURCE_LOCATION (decl),
> +			    "jump to label %qD", decl);
> +		  inform (input_location, "  from here");
> +		  complained = true;
> +		  identified = 2;
>   		}

and the same here.

Cheers,

Manuel.
diff mbox

Patch

--- gcc/cp/decl.c.jj	2015-11-14 19:35:53.000000000 +0100
+++ gcc/cp/decl.c	2015-11-18 12:30:40.085342627 +0100
@@ -2970,11 +2970,24 @@  decl_jump_unsafe (tree decl)
 /* A subroutine of check_previous_goto_1 to identify a branch to the user.  */
 
 static bool
-identify_goto (tree decl, const location_t *locus)
+identify_goto (tree decl, const location_t *locus, bool harderr)
 {
-  bool complained = (decl
-		     ? permerror (input_location, "jump to label %qD", decl)
-		     : permerror (input_location, "jump to case label"));
+  bool complained;
+  if (!harderr)
+    {
+      if (decl)
+	complained = permerror (input_location, "jump to label %qD", decl);
+      else
+	complained = permerror (input_location, "jump to case label");
+    }
+  else
+    {
+      if (decl)
+	error ("jump to label %qD", decl);
+      else
+	error ("jump to case label");
+      complained = true;
+    }
   if (complained && locus)
     inform (*locus, "  from here");
   return complained;
@@ -2991,15 +3004,16 @@  check_previous_goto_1 (tree decl, cp_bin
 		       bool exited_omp, const location_t *locus)
 {
   cp_binding_level *b;
-  bool identified = false, complained = false;
+  bool complained = false;
+  int identified = 0;
   bool saw_eh = false, saw_omp = false, saw_tm = false;
 
   if (exited_omp)
     {
-      complained = identify_goto (decl, locus);
+      complained = identify_goto (decl, locus, true);
       if (complained)
 	inform (input_location, "  exits OpenMP structured block");
-      identified = saw_omp = true;
+      identified = (saw_omp = true) ? 2 : 0;
     }
 
   for (b = current_binding_level; b ; b = b->level_chain)
@@ -3016,8 +3030,8 @@  check_previous_goto_1 (tree decl, cp_bin
 
 	  if (!identified)
 	    {
-	      complained = identify_goto (decl, locus);
-	      identified = true;
+	      complained = identify_goto (decl, locus, false);
+	      identified = 1;
 	    }
 	  if (complained)
 	    {
@@ -3035,10 +3049,10 @@  check_previous_goto_1 (tree decl, cp_bin
 	break;
       if ((b->kind == sk_try || b->kind == sk_catch) && !saw_eh)
 	{
-	  if (!identified)
+	  if (identified < 2)
 	    {
-	      complained = identify_goto (decl, locus);
-	      identified = true;
+	      complained = identify_goto (decl, locus, true);
+	      identified = 2;
 	    }
 	  if (complained)
 	    {
@@ -3051,10 +3065,10 @@  check_previous_goto_1 (tree decl, cp_bin
 	}
       if (b->kind == sk_omp && !saw_omp)
 	{
-	  if (!identified)
+	  if (identified < 2)
 	    {
-	      complained = identify_goto (decl, locus);
-	      identified = true;
+	      complained = identify_goto (decl, locus, true);
+	      identified = 2;
 	    }
 	  if (complained)
 	    inform (input_location, "  enters OpenMP structured block");
@@ -3062,10 +3076,10 @@  check_previous_goto_1 (tree decl, cp_bin
 	}
       if (b->kind == sk_transaction && !saw_tm)
 	{
-	  if (!identified)
+	  if (identified < 2)
 	    {
-	      complained = identify_goto (decl, locus);
-	      identified = true;
+	      complained = identify_goto (decl, locus, true);
+	      identified = 2;
 	    }
 	  if (complained)
 	    inform (input_location,
@@ -3098,7 +3112,8 @@  void
 check_goto (tree decl)
 {
   struct named_label_entry *ent, dummy;
-  bool saw_catch = false, identified = false, complained = false;
+  bool saw_catch = false, complained = false;
+  int identified = 0;
   tree bad;
   unsigned ix;
 
@@ -3141,11 +3156,21 @@  check_goto (tree decl)
   if (ent->in_try_scope || ent->in_catch_scope || ent->in_transaction_scope
       || ent->in_omp_scope || !vec_safe_is_empty (ent->bad_decls))
     {
-      complained = permerror (DECL_SOURCE_LOCATION (decl),
-			      "jump to label %qD", decl);
+      if (ent->in_try_scope || ent->in_catch_scope
+	  || ent->in_transaction_scope || ent->in_omp_scope)
+	{
+	  error_at (DECL_SOURCE_LOCATION (decl), "jump to label %qD", decl);
+	  complained = true;
+	  identified = 2;
+	}
+      else
+	{
+	  complained = permerror (DECL_SOURCE_LOCATION (decl),
+				  "jump to label %qD", decl);
+	  identified = 1;
+	}
       if (complained)
 	inform (input_location, "  from here");
-      identified = true;
     }
 
   FOR_EACH_VEC_SAFE_ELT (ent->bad_decls, ix, bad)
@@ -3155,6 +3180,14 @@  check_goto (tree decl)
       if (u > 1 && DECL_ARTIFICIAL (bad))
 	{
 	  /* Can't skip init of __exception_info.  */
+	  if (identified == 1)
+	    {
+	      error_at (DECL_SOURCE_LOCATION (decl),
+			"jump to label %qD", decl);
+	      inform (input_location, "  from here");
+	      complained = true;
+	      identified = 2;
+	    }
 	  if (complained)
 	    inform (DECL_SOURCE_LOCATION (bad), "  enters catch block");
 	  saw_catch = true;
@@ -3195,13 +3228,13 @@  check_goto (tree decl)
 	    break;
 	  if (b->kind == sk_omp)
 	    {
-	      if (!identified)
+	      if (identified < 2)
 		{
-		  complained = permerror (DECL_SOURCE_LOCATION (decl),
-					  "jump to label %qD", decl);
-		  if (complained)
-		    inform (input_location, "  from here");
-		  identified = true;
+		  error_at (DECL_SOURCE_LOCATION (decl),
+			    "jump to label %qD", decl);
+		  inform (input_location, "  from here");
+		  complained = true;
+		  identified = 2;
 		}
 	      if (complained)
 		inform (input_location, "  exits OpenMP structured block");
--- gcc/testsuite/g++.dg/eh/goto3.C.jj	2015-11-18 12:24:17.313744588 +0100
+++ gcc/testsuite/g++.dg/eh/goto3.C	2015-11-18 12:25:28.940733749 +0100
@@ -0,0 +1,14 @@ 
+// PR c++/67409
+// { dg-options "-fpermissive" }
+
+void f()
+try
+  {
+    goto l2;       // { dg-message "from here" }
+  l1: ;	    // { dg-error "jump to label 'l1'" }
+  } catch (...)
+  {
+  l2: ;	    // { dg-error "jump to label 'l2'" }
+		   // { dg-message "enters catch block" "" { target *-*-*} 11 }
+    goto l1;       // { dg-message "from here|enters try block" }
+  }