Re: [PATCH v3] gcc: Introduce -fhardened

[Marek Polacek <polacek@redhat.com>]

On Tue, Oct 24, 2023 at 09:22:25AM +0200, Richard Biener wrote:
> On Mon, Oct 23, 2023 at 9:26 PM Marek Polacek <polacek@redhat.com> wrote:
> >
> > On Thu, Oct 19, 2023 at 02:24:11PM +0200, Richard Biener wrote:
> > > On Wed, Oct 11, 2023 at 10:48 PM Marek Polacek <polacek@redhat.com> wrote:
> > > >
> > > > On Tue, Sep 19, 2023 at 10:58:19AM -0400, Marek Polacek wrote:
> > > > > On Mon, Sep 18, 2023 at 08:57:39AM +0200, Richard Biener wrote:
> > > > > > On Fri, Sep 15, 2023 at 5:09 PM Marek Polacek via Gcc-patches
> > > > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > > > >
> > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, powerpc64le-unknown-linux-gnu,
> > > > > > > and aarch64-unknown-linux-gnu; ok for trunk?
> > > > > > >
> > > > > > > -- >8 --
> > > > > > > In <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628748.html>
> > > > > > > I proposed -fhardened, a new umbrella option that enables a reasonable set
> > > > > > > of hardening flags.  The read of the room seems to be that the option
> > > > > > > would be useful.  So here's a patch implementing that option.
> > > > > > >
> > > > > > > Currently, -fhardened enables:
> > > > > > >
> > > > > > >   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> > > > > > >   -D_GLIBCXX_ASSERTIONS
> > > > > > >   -ftrivial-auto-var-init=pattern
> > >
> > > I think =zero is much better here given the overhead is way
> > > cheaper and pointers get a more reliable behavior.
> >
> > Ok, changed now.
> >
> > > > > > >   -fPIE  -pie  -Wl,-z,relro,-z,now
> > > > > > >   -fstack-protector-strong
> > > > > > >   -fstack-clash-protection
> > > > > > >   -fcf-protection=full (x86 GNU/Linux only)
> > > > > > >
> > > > > > > -fhardened will not override options that were specified on the command line
> > > > > > > (before or after -fhardened).  For example,
> > > > > > >
> > > > > > >      -D_FORTIFY_SOURCE=1 -fhardened
> > > > > > >
> > > > > > > means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> > > > > > >
> > > > > > >       -fhardened -fstack-protector
> > > > > > >
> > > > > > > will not enable -fstack-protector-strong.
> > > > > > >
> > > > > > > In DW_AT_producer it is reflected only as -fhardened; it doesn't expand
> > > > > > > to anything.  I think we need a better way to show what it actually
> > > > > > > enables.
> > > > > >
> > > > > > I do think we need to find a solution here to solve asserting compliance.
> > > > >
> > > > > Fair enough.
> > > > >
> > > > > > Maybe we can have -Whardened that will diagnose any altering of
> > > > > > -fhardened by other options on the command-line or by missed target
> > > > > > implementations?  People might for example use -fstack-protector
> > > > > > but don't really want to make protection lower than requested with -fhardened.
> > > > > >
> > > > > > Any such conflict is much less appearant than when you use the
> > > > > > flags -fhardened composes.
> > > > >
> > > > > How about: --help=hardened says which options -fhardened attempts to
> > > > > enable, and -Whardened warns when it didn't enable an option?  E.g.,
> > > > >
> > > > >   -fstack-protector -fhardened -Whardened
> > > > >
> > > > > would say that it didn't enable -fstack-protector-strong because
> > > > > -fstack-protector was specified on the command line?
> > > > >
> > > > > If !HAVE_LD_NOW_SUPPORT, --help=hardened probably doesn't even have to
> > > > > list -z now, likewise for -z relro.
> > > > >
> > > > > Unclear if -Whardened should be enabled by default, but probably yes?
> > > >
> > > > Here's v2 which adds -Whardened (enabled by default).
> > > >
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > >
> > > I think it's OK but I'd like to see a second ACK here.
> >
> > Thanks!
> >
> > > Can you see how our
> > > primary and secondary targets (+ host OS) behave here?
> >
> > That's very reasonable.  I tried to build gcc on Compile Farm 119 (AIX) but
> > that fails with:
> >
> > ar  -X64 x ../ppc64/libgcc/libgcc_s.a shr.o
> > ar: 0707-100 ../ppc64/libgcc/libgcc_s.a does not exist.
> > make[2]: *** [/home/polacek/gcc/libgcc/config/rs6000/t-slibgcc-aix:98: all] Error 1
> > make[2]: Leaving directory '/home/polacek/x/trunk/powerpc-ibm-aix7.3.1.0/libgcc'
> >
> > and I tried Darwin (104) and that fails with
> >
> > *** Configuration aarch64-apple-darwin21.6.0 not supported
> >
> > Is anyone else able to build gcc on those machines, or test the attached
> > patch?
> >
> > > I think the
> > > documentation should elaborate a bit on expectations for non-Linux/GNU
> > > targets, specifically I think the default configuration for a target should
> > > with -fhardened _not_ have any -Whardened diagnostics.  Maybe we can
> > > have a testcase for this?
> >
> > Sorry, I'm not sure how to test that.  I suppose if -fhardened enables
> > something not supported on those systems, and it's something for which
> > we have a configure test, then we shouldn't warn.  This is already the
> > case for -pie, -z relro, and -z now.
> 
> I was thinking of
> 
> /* { dg-do compile } */
> /* { dg-additional-options "-fhardened -Whardened" } */
> 
> int main () {}
> 
> and excess errors should catch "misconfigurations"?

I see.  fhardened-3.c is basically just like this (-Whardened is on by default).
 
> > Should the docs say something like the following for features without
> > configure checks?
> >
> > @option{-fhardened} can, on certain systems, attempt to enable features
> > not supported on that particular system.  In that case, it's possible to
> > prevent the warning using the @option{-Wno-hardened} option.
> 
> Yeah, but ideally
> 
> @option{-fhardened} can, on certain systems, not enable features not
> available on those systems and @option{-Whardened} will not diagnose
> those as missing.
> 
> But I understand it doesn't work like that?

Right.  It will not diagnose missing features if they have a configure
check, otherwise it will.  And I don't know if we want a configure check
for every feature.  Maybe we can add them in the future if the current
patch turns out to be problematical in practice?

Thanks,

Marek