On Tue, 2017-08-22 at 09:35 -0600, Jeff Law wrote:
> On 08/19/2017 12:22 PM, Martin Sebor wrote:
> > On 07/30/2017 11:35 PM, Jeff Law wrote:
> > > This patch introduces the stack clash protection options
> > >
> > > Changes since V2:
> > >
> > > Adds two new params. The first controls the size of the guard
> > > area.
> > > This controls the threshold for when a function prologue requires
> > > probes. The second controls the probing interval -- ie, once
> > > probes are
> > > needed, how often do we emit them. These are really meant more
> > > for
> > > developers to experiment with than users. Regardless I did go
> > > ahead and
> > > document them./PARAM
> > >
> > > It also adds some sanity checking WRT combining stack clash
> > > protection
> > > with -fstack-check.
> >
> > Just a minor nit and suggestion:
> >
> > "supproted" -> "supported"
> >
> > + warning_at (UNKNOWN_LOCATION, 0,
> > + "-fstack-clash_protection is not supproted on targets "
> > + "where the stack grows from lower to higher addresses");
> >
> > and quote the name of the options in diagnostics, i.e., use either
> >
> > "%<"-fstack-clash_protection%> ..."
> >
> > or
> >
> > "%qs is not supported...", "-fstack-clash_protection"
> >
> > as you did in error ("value of parameter %qs must be a power of 2",
> > ompiler_params[i].option);
> >
> > Likewise in
> >
> > + warning_at (UNKNOWN_LOCATION, 0,
> > + "-fstack-check= and -fstack-clash_protection are
> > mutually "
> > + "exclusive. Disabling -fstack-check=");
>
> Thanks. I settled on the %< %> style. None of the other warnings in
> that area use either. Otherwise I would have just selected whatever
> was
> most commonly used in that code.
A nit that don't seem to have been mentioned: the patch refers to
-fstack-clash_protection (erroneous underscore)
in two places, which should be:
-fstack-clash-protection (all dashes)
Dave
