[PATCH, PR 49495] Cgraph verifier must look through aliases

[PATCH, PR 49495] Cgraph verifier must look through aliases

[Martin Jambor]

Hi,

PR 49495 is actually a bug in the verifier that does not look through
aliases at one point.  Fixed wit the patch below (created a special
function, otherwise I just wasn't able to fit the 80 column limit).
Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2011-07-02  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/49495
	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
	(verify_cgraph_node): Some functinality moved to
	verify_edge_corresponds_to_fndecl, call it.


Index: src/gcc/cgraphunit.c
===================================================================
--- src.orig/gcc/cgraphunit.c
+++ src/gcc/cgraphunit.c
@@ -450,6 +450,34 @@ cgraph_debug_gimple_stmt (struct functio
   debug_gimple_stmt (stmt);
 }
 
+/* Verify that call graph edge E corresponds to DECL from the associated
+   statement.  Return true if the verification should fail.  */
+
+static bool
+verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
+{
+  if (!e->callee->global.inlined_to
+      && decl
+      && cgraph_get_node (decl)
+      && (e->callee->former_clone_of
+	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
+      /* IPA-CP sometimes redirect edge to clone and then back to the former
+	 function.  This ping-pong has to go, eventaully.  */
+      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
+	  != cgraph_function_or_thunk_node (e->callee, NULL))
+      && !clone_of_p (cgraph_get_node (decl),
+		      e->callee))
+    {
+      error ("edge points to wrong declaration:");
+      debug_tree (e->callee->decl);
+      fprintf (stderr," Instead of:");
+      debug_tree (decl);
+      return true;
+    }
+  else
+    return false;
+}
+
 /* Verify cgraph nodes of given cgraph node.  */
 DEBUG_FUNCTION void
 verify_cgraph_node (struct cgraph_node *node)
@@ -702,24 +730,8 @@ verify_cgraph_node (struct cgraph_node *
 			  }
 			if (!e->indirect_unknown_callee)
 			  {
-			    if (!e->callee->global.inlined_to
-			        && decl
-			        && cgraph_get_node (decl)
-			        && (e->callee->former_clone_of
-				    != cgraph_get_node (decl)->decl)
-				/* IPA-CP sometimes redirect edge to clone and then back to the former
-				   function.  This ping-pong has to go, eventaully.  */
-				&& (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
-				    != cgraph_function_or_thunk_node (e->callee, NULL))
-				&& !clone_of_p (cgraph_get_node (decl),
-					        e->callee))
-			      {
-				error ("edge points to wrong declaration:");
-				debug_tree (e->callee->decl);
-				fprintf (stderr," Instead of:");
-				debug_tree (decl);
-				error_found = true;
-			      }
+			    if (verify_edge_corresponds_to_fndecl (e, decl))
+			      error_found = true;
 			  }
 			else if (decl)
 			  {

Re: [PATCH, PR 49495] Cgraph verifier must look through aliases

[Jan Hubicka]

> Hi,
> 
> PR 49495 is actually a bug in the verifier that does not look through
> aliases at one point.  Fixed wit the patch below (created a special
> function, otherwise I just wasn't able to fit the 80 column limit).
> Bootstrapped and tested on x86_64-linux.  OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2011-07-02  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/49495
> 	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
> 	(verify_cgraph_node): Some functinality moved to
> 	verify_edge_corresponds_to_fndecl, call it.

This is OK.
> 
> 
> Index: src/gcc/cgraphunit.c
> ===================================================================
> --- src.orig/gcc/cgraphunit.c
> +++ src/gcc/cgraphunit.c
> @@ -450,6 +450,34 @@ cgraph_debug_gimple_stmt (struct functio
>    debug_gimple_stmt (stmt);
>  }
>  
> +/* Verify that call graph edge E corresponds to DECL from the associated
> +   statement.  Return true if the verification should fail.  */
> +
> +static bool
> +verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
> +{
> +  if (!e->callee->global.inlined_to
> +      && decl
> +      && cgraph_get_node (decl)
> +      && (e->callee->former_clone_of
> +	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
> +      /* IPA-CP sometimes redirect edge to clone and then back to the former
> +	 function.  This ping-pong has to go, eventaully.  */
> +      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> +	  != cgraph_function_or_thunk_node (e->callee, NULL))
> +      && !clone_of_p (cgraph_get_node (decl),
> +		      e->callee))
> +    {
> +      error ("edge points to wrong declaration:");
> +      debug_tree (e->callee->decl);
> +      fprintf (stderr," Instead of:");
> +      debug_tree (decl);
> +      return true;
> +    }
> +  else
> +    return false;
> +}
> +
>  /* Verify cgraph nodes of given cgraph node.  */
>  DEBUG_FUNCTION void
>  verify_cgraph_node (struct cgraph_node *node)
> @@ -702,24 +730,8 @@ verify_cgraph_node (struct cgraph_node *
>  			  }
>  			if (!e->indirect_unknown_callee)
>  			  {
> -			    if (!e->callee->global.inlined_to
> -			        && decl
> -			        && cgraph_get_node (decl)
> -			        && (e->callee->former_clone_of
> -				    != cgraph_get_node (decl)->decl)
> -				/* IPA-CP sometimes redirect edge to clone and then back to the former
> -				   function.  This ping-pong has to go, eventaully.  */
> -				&& (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> -				    != cgraph_function_or_thunk_node (e->callee, NULL))
> -				&& !clone_of_p (cgraph_get_node (decl),
> -					        e->callee))
> -			      {
> -				error ("edge points to wrong declaration:");
> -				debug_tree (e->callee->decl);
> -				fprintf (stderr," Instead of:");
> -				debug_tree (decl);
> -				error_found = true;
> -			      }
> +			    if (verify_edge_corresponds_to_fndecl (e, decl))
> +			      error_found = true;
Could you please move the error output here, somehow I like it better when all
the diagnostic is output at single place...

Honza
>  			  }
>  			else if (decl)
>  			  {

Re: [PATCH, PR 49495] Cgraph verifier must look through aliases

[Martin Jambor]

Hi,

On Mon, Jul 04, 2011 at 11:13:12AM +0200, Jan Hubicka wrote:
> > Hi,
> > 
> > PR 49495 is actually a bug in the verifier that does not look through
> > aliases at one point.  Fixed wit the patch below (created a special
> > function, otherwise I just wasn't able to fit the 80 column limit).
> > Bootstrapped and tested on x86_64-linux.  OK for trunk?
> > 
> > Thanks,
> > 
> > Martin
> > 
> > 
> > 2011-07-02  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	PR middle-end/49495
> > 	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
> > 	(verify_cgraph_node): Some functinality moved to
> > 	verify_edge_corresponds_to_fndecl, call it.
> 
> This is OK.
> > 
> > 
> > Index: src/gcc/cgraphunit.c
> > ===================================================================
> > --- src.orig/gcc/cgraphunit.c
> > +++ src/gcc/cgraphunit.c
> > @@ -450,6 +450,34 @@ cgraph_debug_gimple_stmt (struct functio
> >    debug_gimple_stmt (stmt);
> >  }
> >  
> > +/* Verify that call graph edge E corresponds to DECL from the associated
> > +   statement.  Return true if the verification should fail.  */
> > +
> > +static bool
> > +verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
> > +{
> > +  if (!e->callee->global.inlined_to
> > +      && decl
> > +      && cgraph_get_node (decl)
> > +      && (e->callee->former_clone_of
> > +	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
> > +      /* IPA-CP sometimes redirect edge to clone and then back to the former
> > +	 function.  This ping-pong has to go, eventaully.  */

BTW, I am having hard time trying to figure out how this comment
relates to any of the tests in the conjunction. I'm obvioulsy asking
because the new IPA-CP does not do this and so we could get rid of it,
whatever the "it" is.

> > +      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> > +	  != cgraph_function_or_thunk_node (e->callee, NULL))
> > +      && !clone_of_p (cgraph_get_node (decl),
> > +		      e->callee))
> > +    {
> > +      error ("edge points to wrong declaration:");

...

> > -				error_found = true;
> > -			      }
> > +			    if (verify_edge_corresponds_to_fndecl (e, decl))
> > +			      error_found = true;
> Could you please move the error output here, somehow I like it better when all
> the diagnostic is output at single place...
> 

Sure, this is what I have just committed as revision 175998 after a
bootstrap and testing on x86_64-linux.

Thanks,

Martin



2011-07-07  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/49495
	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
	(verify_cgraph_node): Some functinality moved to
	verify_edge_corresponds_to_fndecl, call it.


Index: src/gcc/cgraphunit.c
===================================================================
--- src.orig/gcc/cgraphunit.c
+++ src/gcc/cgraphunit.c
@@ -450,6 +450,28 @@ cgraph_debug_gimple_stmt (struct functio
   debug_gimple_stmt (stmt);
 }
 
+/* Verify that call graph edge E corresponds to DECL from the associated
+   statement.  Return true if the verification should fail.  */
+
+static bool
+verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
+{
+  if (!e->callee->global.inlined_to
+      && decl
+      && cgraph_get_node (decl)
+      && (e->callee->former_clone_of
+	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
+      /* IPA-CP sometimes redirect edge to clone and then back to the former
+	 function.  This ping-pong has to go, eventaully.  */
+      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
+	  != cgraph_function_or_thunk_node (e->callee, NULL))
+      && !clone_of_p (cgraph_get_node (decl),
+		      e->callee))
+    return true;
+  else
+    return false;
+}
+
 /* Verify cgraph nodes of given cgraph node.  */
 DEBUG_FUNCTION void
 verify_cgraph_node (struct cgraph_node *node)
@@ -702,17 +724,7 @@ verify_cgraph_node (struct cgraph_node *
 			  }
 			if (!e->indirect_unknown_callee)
 			  {
-			    if (!e->callee->global.inlined_to
-			        && decl
-			        && cgraph_get_node (decl)
-			        && (e->callee->former_clone_of
-				    != cgraph_get_node (decl)->decl)
-				/* IPA-CP sometimes redirect edge to clone and then back to the former
-				   function.  This ping-pong has to go, eventaully.  */
-				&& (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
-				    != cgraph_function_or_thunk_node (e->callee, NULL))
-				&& !clone_of_p (cgraph_get_node (decl),
-					        e->callee))
+			    if (verify_edge_corresponds_to_fndecl (e, decl))
 			      {
 				error ("edge points to wrong declaration:");
 				debug_tree (e->callee->decl);

Re: [PATCH, PR 49495] Cgraph verifier must look through aliases

[Jan Hubicka]

> Hi,
> 
> On Mon, Jul 04, 2011 at 11:13:12AM +0200, Jan Hubicka wrote:
> > > Hi,
> > > 
> > > PR 49495 is actually a bug in the verifier that does not look through
> > > aliases at one point.  Fixed wit the patch below (created a special
> > > function, otherwise I just wasn't able to fit the 80 column limit).
> > > Bootstrapped and tested on x86_64-linux.  OK for trunk?
> > > 
> > > Thanks,
> > > 
> > > Martin
> > > 
> > > 
> > > 2011-07-02  Martin Jambor  <mjambor@suse.cz>
> > > 
> > > 	PR middle-end/49495
> > > 	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
> > > 	(verify_cgraph_node): Some functinality moved to
> > > 	verify_edge_corresponds_to_fndecl, call it.
> > 
> > This is OK.
> > > 
> > > 
> > > Index: src/gcc/cgraphunit.c
> > > ===================================================================
> > > --- src.orig/gcc/cgraphunit.c
> > > +++ src/gcc/cgraphunit.c
> > > @@ -450,6 +450,34 @@ cgraph_debug_gimple_stmt (struct functio
> > >    debug_gimple_stmt (stmt);
> > >  }
> > >  
> > > +/* Verify that call graph edge E corresponds to DECL from the associated
> > > +   statement.  Return true if the verification should fail.  */
> > > +
> > > +static bool
> > > +verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
> > > +{
> > > +  if (!e->callee->global.inlined_to
> > > +      && decl
> > > +      && cgraph_get_node (decl)
> > > +      && (e->callee->former_clone_of
> > > +	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
> > > +      /* IPA-CP sometimes redirect edge to clone and then back to the former
> > > +	 function.  This ping-pong has to go, eventaully.  */
> 
> BTW, I am having hard time trying to figure out how this comment
> relates to any of the tests in the conjunction. I'm obvioulsy asking
> because the new IPA-CP does not do this and so we could get rid of it,
> whatever the "it" is.
      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
	  != cgraph_function_or_thunk_node (e->callee, NULL))
should be possible to change to
      && (cgraph_get_node (decl) != e->callee)
i.e. not try to go through the aliases 
> 
> > > +      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> > > +	  != cgraph_function_or_thunk_node (e->callee, NULL))
> > > +      && !clone_of_p (cgraph_get_node (decl),
> > > +		      e->callee))
> > > +    {
> > > +      error ("edge points to wrong declaration:");
> 
> ...
> 
> > > -				error_found = true;
> > > -			      }
> > > +			    if (verify_edge_corresponds_to_fndecl (e, decl))
> > > +			      error_found = true;
> > Could you please move the error output here, somehow I like it better when all
> > the diagnostic is output at single place...
> > 
> 
> Sure, this is what I have just committed as revision 175998 after a
> bootstrap and testing on x86_64-linux.
> 
> Thanks,
> 
> Martin
> 
> 
> 
> 2011-07-07  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/49495
> 	* cgraphunit.c (verify_edge_corresponds_to_fndecl): New function.
> 	(verify_cgraph_node): Some functinality moved to
> 	verify_edge_corresponds_to_fndecl, call it.
> 
> 
> Index: src/gcc/cgraphunit.c
> ===================================================================
> --- src.orig/gcc/cgraphunit.c
> +++ src/gcc/cgraphunit.c
> @@ -450,6 +450,28 @@ cgraph_debug_gimple_stmt (struct functio
>    debug_gimple_stmt (stmt);
>  }
>  
> +/* Verify that call graph edge E corresponds to DECL from the associated
> +   statement.  Return true if the verification should fail.  */
> +
> +static bool
> +verify_edge_corresponds_to_fndecl (struct cgraph_edge *e, tree decl)
> +{
> +  if (!e->callee->global.inlined_to
> +      && decl
> +      && cgraph_get_node (decl)
> +      && (e->callee->former_clone_of
> +	  != cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)->decl)
> +      /* IPA-CP sometimes redirect edge to clone and then back to the former
> +	 function.  This ping-pong has to go, eventaully.  */
> +      && (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> +	  != cgraph_function_or_thunk_node (e->callee, NULL))
> +      && !clone_of_p (cgraph_get_node (decl),
> +		      e->callee))
> +    return true;
> +  else
> +    return false;
> +}
> +
>  /* Verify cgraph nodes of given cgraph node.  */
>  DEBUG_FUNCTION void
>  verify_cgraph_node (struct cgraph_node *node)
> @@ -702,17 +724,7 @@ verify_cgraph_node (struct cgraph_node *
>  			  }
>  			if (!e->indirect_unknown_callee)
>  			  {
> -			    if (!e->callee->global.inlined_to
> -			        && decl
> -			        && cgraph_get_node (decl)
> -			        && (e->callee->former_clone_of
> -				    != cgraph_get_node (decl)->decl)
> -				/* IPA-CP sometimes redirect edge to clone and then back to the former
> -				   function.  This ping-pong has to go, eventaully.  */
> -				&& (cgraph_function_or_thunk_node (cgraph_get_node (decl), NULL)
> -				    != cgraph_function_or_thunk_node (e->callee, NULL))
> -				&& !clone_of_p (cgraph_get_node (decl),
> -					        e->callee))
> +			    if (verify_edge_corresponds_to_fndecl (e, decl))
>  			      {
>  				error ("edge points to wrong declaration:");
>  				debug_tree (e->callee->decl);