On Wed, Aug 30, 2023 at 03:08:46PM +0200, Richard Biener wrote: > On Wed, Aug 30, 2023 at 12:51 PM Jakub Jelinek via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches > > wrote: > > > + if (UNLIKELY (flag_hardened) > > > + && (opt->code == OPT_D || opt->code == OPT_U)) > > > + { > > > + if (!fortify_seen_p) > > > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > > > > Perhaps this should check that the char after it is either '\0' or '=', we > > shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro. > > > > > + if (!cxx_assert_seen_p) > > > + cxx_assert_seen_p = !strcmp (opt->arg, > > > "_GLIBCXX_ASSERTIONS"); > > > > Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42 > > > > > + } > > > + } > > > + > > > + if (flag_hardened) > > > + { > > > + if (!fortify_seen_p && optimize > 0) > > > + { > > > + if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35) > > > + cpp_define (parse_in, "_FORTIFY_SOURCE=3"); > > > + else > > > + cpp_define (parse_in, "_FORTIFY_SOURCE=2"); > > > > I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for > > -fhardened only for targets which actually have such a support in the C > > library. There is some poor man's _FORTIFY_SOURCE support in libssp, > > but e.g. one has to link with -lssp in that case and compile with > > -isystem `gcc -print-include-filename=include`/ssp . > > For glibc that is >= 2.3.4, > > https://maskray.me/blog/2022-11-06-fortify-source > > mentions NetBSD support since 2006, newlib since 2017, some Darwin libc, > > bionic (but seems they have only some clang support and dropped GCC > > support) and some third party reimplementation of libssp. > > Or do we just enable it and hope that either it works well or isn't > > supported at all quietly? E.g. it would certainly break the ssp case > > where -isystem finds ssp headers but -lssp isn't linked in. > > > > > @@ -4976,6 +4993,22 @@ process_command (unsigned int > > > decoded_options_count, > > > #endif > > > } > > > > > > + /* TODO: check if -static -pie works and maybe use it. */ > > > + if (flag_hardened && !any_link_options_p && !static_p) > > > + { > > > + save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false); > > > + /* TODO: check if BIND_NOW/RELRO is supported. */ > > > + if (true) > > > + { > > > + /* These are passed straight down to collect2 so we have to break > > > + it up like this. */ > > > + add_infile ("-z", "*"); > > > + add_infile ("now", "*"); > > > + add_infile ("-z", "*"); > > > + add_infile ("relro", "*"); > > > > As the TODO comment says, to do that we need to check at configure time that > > linker supports -z now and -z relro options. > > > > > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct > > > gcc_options *opts_set, > > > } > > > > > > /* We initialize opts->x_flag_stack_protect to -1 so that targets > > > - can set a default value. */ > > > + can set a default value. With --enable-default-ssp or -fhardened > > > + the default is -fstack-protector-strong. */ > > > if (opts->x_flag_stack_protect == -1) > > > - opts->x_flag_stack_protect = DEFAULT_FLAG_SSP; > > > + opts->x_flag_stack_protect = (opts->x_flag_hardened > > > + ? SPCT_FLAG_STRONG > > > + : DEFAULT_FLAG_SSP); > > > > This needs to be careful, -fstack-protector isn't supported on all targets > > (e.g. ia64) and we don't want toplev.cc warning: > > /* Targets must be able to place spill slots at lower addresses. If the > > target already uses a soft frame pointer, the transition is trivial. > > */ > > if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) > > { > > warning_at (UNKNOWN_LOCATION, 0, > > "%<-fstack-protector%> not supported for this target"); > > flag_stack_protect = 0; > > } > > to be emitted whenever using -fhardened, it should not be enabled there > > silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it > > work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was > > enabled and otherwise keep it !FRAME_GROWS_DOWNWARD). > > I'll note that with selectively enabling parts of -fhardening it can > also give a false > sensation of safety when under the hood we ignore half of the option due to > one or another reason ...
I suppose you're right :/. > How does -fhardening reflect into -[gf]record-gcc-switches? Is it at > least possible > to verify the actually enabled bits? In DW_AT_producer it simply shows "-fhardened". It doesn't expand to what it actually enabled. I imagine gen_producer_string could be tweaked to print the options enabled by -fhardened. But, DW_AT_producer doesn't seem to show -D options, or linked options like -Wl,-z,now. So I think we need something better. Maybe add a line in gcc -v? I would not like to add a new option just for this. (It would have to be careful not to show options that the target doesn't support, like -fstack-protector on BPF.) Marek