Re: [PATCH] Fix unsafe function attributes for special functions (PR 71876)

[Richard Biener <rguenther at suse dot de>]

On Thu, 21 Jul 2016, Bernd Edlinger wrote:

> On 07/21/16 13:35, Richard Biener wrote:
> > On Thu, 21 Jul 2016, Bernd Edlinger wrote:
> >
> >> On 07/21/16 13:25, Bernd Schmidt wrote:
> >>>
> >>>
> >>> On 07/21/2016 01:16 PM, Jakub Jelinek wrote:
> >>>> On Thu, Jul 21, 2016 at 11:04:48AM +0000, Bernd Edlinger wrote:
> >>>>>   bool
> >>>>> +gimple_alloca_call_p (const gimple *stmt)
> >>>>> +{
> >>>>> +  tree fndecl;
> >>>>> +
> >>>>> +  if (!is_gimple_call (stmt))
> >>>>> +    return false;
> >>>>> +
> >>>>> +  fndecl = gimple_call_fndecl (stmt);
> >>>>> +  if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> >>>>> +    switch (DECL_FUNCTION_CODE (fndecl))
> >>>>> +      {
> >>>>> +      case BUILT_IN_ALLOCA:
> >>>>> +      case BUILT_IN_ALLOCA_WITH_ALIGN:
> >>>>> +        return true;
> >>>>> +      }
> >>>>
> >>>> This should have failed bootstrap because of -Wswitch-enum.
> >>>> You need
> >>>>      default:
> >>>>        break;
> >>>> in.
> >>>>
> >>>>> +    switch (DECL_FUNCTION_CODE (fndecl))
> >>>>> +      {
> >>>>> +      case BUILT_IN_ALLOCA:
> >>>>> +      case BUILT_IN_ALLOCA_WITH_ALIGN:
> >>>>> +        return true;
> >>>>
> >>>> Likewise here.
> >>>>
> >>> Or write it in the more natural way as an if.
> >>>
> >>
> >> I'm open for that suggestion.
> >>
> >> Then I should probably also rewrite the switch statement
> >> in special_function_p as an if.
> >
> > I think a switch is a good fit though I don't mind an if as we probably
> > know we'll never get more than two alloca builtins (heh, you never know).
> >
> 
> Thanks, style-nits are always welcome for me.  I also do care about
> that a lot.
> 
> I will keep the switch at the moment, and continue the boot-strap
> with the approved version.
> 
> BTW: in the function below "is_tm_builtin" there is a single switch
> in a block statement, we usually don't do that redundant braces...
> 
> 
> Richard, do you still have objections against the builtin_setjmp patch
> from https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01225.html ?

No, I think it is ok, thus, approved as well.  Good to see that
special_function_p cleaned up ;)

Thanks,
Richard.