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]

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


> 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);


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