Verify allowed stmts before labels

Message ID 20171213141249.GH2353@tucnak
State New
Headers show
Series
  • Verify allowed stmts before labels
Related show

Commit Message

Jakub Jelinek Dec. 13, 2017, 2:12 p.m.
Hi!

PR83391/PR83396 failed because debug bind stmts were put before labels.
Alex said that is undesirable, and that right now we want to allow
just debug begin stmt markers before or intermixed with labels.

This patch ensures that through verification, which is what defines
what is and isn't valid GIMPLE.  If we ever reconsider it, either allow
further stmts or disallow even debug begin stmt markers, we can easily also
tweak the verifier.  The patch has been successfully bootstrapped/regtested as
part of the:

https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html                                                                                           
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html                                                                                           
https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861                                                                                               
https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866                                                                                               
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html                                                                                           

patchset, without the msg00811.html patch it of course doesn't survive
bootstrap, as we insert debug bind stmts before labels in those cases.

Ok for trunk once the msg00811.html patch or something similar is committed?

2017-12-13  Jakub Jelinek  <jakub@redhat.com>

	* tree-cfg.c (verify_gimple_in_cfg): Verify no non-label stmts
	with the exception of debug begin stmt markers appear before
	labels.


	Jakub

Comments

Richard Biener Dec. 13, 2017, 2:45 p.m. | #1
On Wed, 13 Dec 2017, Jakub Jelinek wrote:

> Hi!

> 

> PR83391/PR83396 failed because debug bind stmts were put before labels.

> Alex said that is undesirable, and that right now we want to allow

> just debug begin stmt markers before or intermixed with labels.

> 

> This patch ensures that through verification, which is what defines

> what is and isn't valid GIMPLE.  If we ever reconsider it, either allow

> further stmts or disallow even debug begin stmt markers, we can easily also

> tweak the verifier.  The patch has been successfully bootstrapped/regtested as

> part of the:

> 

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html                                                                                           

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html                                                                                           

> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861                                                                                               

> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866                                                                                               

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html                                                                                           

> 

> patchset, without the msg00811.html patch it of course doesn't survive

> bootstrap, as we insert debug bind stmts before labels in those cases.

> 

> Ok for trunk once the msg00811.html patch or something similar is committed?


Ok.

Richard.

> 2017-12-13  Jakub Jelinek  <jakub@redhat.com>

> 

> 	* tree-cfg.c (verify_gimple_in_cfg): Verify no non-label stmts

> 	with the exception of debug begin stmt markers appear before

> 	labels.

> 

> --- gcc/tree-cfg.c.jj	2017-12-12 21:24:23.000000000 +0100

> +++ gcc/tree-cfg.c	2017-12-13 07:44:15.622790922 +0100

> @@ -5380,6 +5380,7 @@ verify_gimple_in_cfg (struct function *f

>  	  err |= err2;

>  	}

>  

> +      bool label_allowed = true;

>        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))

>  	{

>  	  gimple *stmt = gsi_stmt (gsi);

> @@ -5396,6 +5397,19 @@ verify_gimple_in_cfg (struct function *f

>  	      err2 = true;

>  	    }

>  

> +	  /* Labels may be preceded only by debug markers, not debug bind

> +	     or source bind or any other statements.  */

> +	  if (gimple_code (stmt) == GIMPLE_LABEL)

> +	    {

> +	      if (!label_allowed)

> +		{

> +		  error ("gimple label in the middle of a basic block");

> +		  err2 = true;

> +		}

> +	    }

> +	  else if (!gimple_debug_begin_stmt_p (stmt))

> +	    label_allowed = false;

> +

>  	  err2 |= verify_gimple_stmt (stmt);

>  	  err2 |= verify_location (&blocks, gimple_location (stmt));

>  

> 

> 	Jakub

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Jeff Law Dec. 13, 2017, 5:49 p.m. | #2
On 12/13/2017 07:12 AM, Jakub Jelinek wrote:
> Hi!

> 

> PR83391/PR83396 failed because debug bind stmts were put before labels.

> Alex said that is undesirable, and that right now we want to allow

> just debug begin stmt markers before or intermixed with labels.

> 

> This patch ensures that through verification, which is what defines

> what is and isn't valid GIMPLE.  If we ever reconsider it, either allow

> further stmts or disallow even debug begin stmt markers, we can easily also

> tweak the verifier.  The patch has been successfully bootstrapped/regtested as

> part of the:

> 

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00811.html                                                                                           

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00808.html                                                                                           

> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42861                                                                                               

> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42866                                                                                               

> https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00794.html                                                                                           

> 

> patchset, without the msg00811.html patch it of course doesn't survive

> bootstrap, as we insert debug bind stmts before labels in those cases.

> 

> Ok for trunk once the msg00811.html patch or something similar is committed?

> 

> 2017-12-13  Jakub Jelinek  <jakub@redhat.com>

> 

> 	* tree-cfg.c (verify_gimple_in_cfg): Verify no non-label stmts

> 	with the exception of debug begin stmt markers appear before

> 	labels.

OK.
jeff

Patch

--- gcc/tree-cfg.c.jj	2017-12-12 21:24:23.000000000 +0100
+++ gcc/tree-cfg.c	2017-12-13 07:44:15.622790922 +0100
@@ -5380,6 +5380,7 @@  verify_gimple_in_cfg (struct function *f
 	  err |= err2;
 	}
 
+      bool label_allowed = true;
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 	{
 	  gimple *stmt = gsi_stmt (gsi);
@@ -5396,6 +5397,19 @@  verify_gimple_in_cfg (struct function *f
 	      err2 = true;
 	    }
 
+	  /* Labels may be preceded only by debug markers, not debug bind
+	     or source bind or any other statements.  */
+	  if (gimple_code (stmt) == GIMPLE_LABEL)
+	    {
+	      if (!label_allowed)
+		{
+		  error ("gimple label in the middle of a basic block");
+		  err2 = true;
+		}
+	    }
+	  else if (!gimple_debug_begin_stmt_p (stmt))
+	    label_allowed = false;
+
 	  err2 |= verify_gimple_stmt (stmt);
 	  err2 |= verify_location (&blocks, gimple_location (stmt));