This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Jeff Law <law at redhat dot com>, Martin Sebor <msebor at gmail dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 22 Aug 2017 12:26:55 -0400
- Subject: Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=dmalcolm at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 801541F57C
- References: <4194c6f8-4efb-3eaa-f165-367b252605c0@redhat.com> <87da148c-77f9-84dd-983f-749d7421b575@gmail.com> <bf8a7b04-43ad-db53-2465-21ff7fc779f8@redhat.com>
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