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 for c++/87068, missing diagnostic with fallthrough statement


On Thu, Aug 30, 2018 at 05:17:03PM -0400, Marek Polacek wrote:
> > 2018-08-23  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR c++/87068
> > 	* gimplify.c (expand_FALLTHROUGH_r): If IFN_FALLTHROUGH was found
> > 	at the end of a seq, save its location to walk_stmt_info.
> > 	(expand_FALLTHROUGH): Warn if IFN_FALLTHROUGH is at the end of
> > 	a switch.
> > 
> > 	* c-c++-common/Wimplicit-fallthrough-37.c: New test.
> > 
> > diff --git gcc/gimplify.c gcc/gimplify.c
> > index e35137aec2c..04c15016f18 100644
> > --- gcc/gimplify.c
> > +++ gcc/gimplify.c
> > @@ -2231,7 +2231,7 @@ maybe_warn_implicit_fallthrough (gimple_seq seq)
> >  
> >  static tree
> >  expand_FALLTHROUGH_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
> > -		      struct walk_stmt_info *)
> > +		      struct walk_stmt_info *wi)
> >  {
> >    gimple *stmt = gsi_stmt (*gsi_p);
> >  
> > @@ -2250,11 +2250,14 @@ expand_FALLTHROUGH_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
> >        if (gimple_call_internal_p (stmt, IFN_FALLTHROUGH))
> >  	{
> >  	  gsi_remove (gsi_p, true);
> > +	  location_t loc = gimple_location (stmt);
> >  	  if (gsi_end_p (*gsi_p))
> > -	    return integer_zero_node;
> > +	    {
> > +	      wi->info = &loc;
> > +	      return integer_zero_node;

This looks incorrect.  loc is an automatic variable in
expand_FALLTHROUGH_r, you set wi->info to the address of that variable,
then immediately return and then:

> > @@ -2317,6 +2320,14 @@ expand_FALLTHROUGH (gimple_seq *seq_p)
> >    struct walk_stmt_info wi;
> >    memset (&wi, 0, sizeof (wi));
> >    walk_gimple_seq_mod (seq_p, expand_FALLTHROUGH_r, NULL, &wi);
> > +  if (wi.callback_result == integer_zero_node)
> > +    {
> > +      /* We've found [[fallthrough]]; at the end of a switch, which the C++
> > +	 standard says is ill-formed; see [dcl.attr.fallthrough].  */
> > +      location_t *loc = static_cast<location_t *>(wi.info);
> > +      warning_at (*loc, 0, "attribute %<fallthrough%> not preceding "

dereference it here, when the loc variable is no longer in scope.
-fsanitize-use-after-scope should complain here...

> > +		  "a case label or default label");
> > +    }

If the only spot that ises wi->info is in these two new spots, can't you
e.g. set in the expand_FALLTHROUGH function:
  location_t loc;
  wi.info = (void *) &loc;
and then in expand_FALLTHROUGH_r do:
  *static_cast<location_t *>(wi.info) = gimple_location (stmt);
?

	Jakub


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