Re: [PATCH v3 16/16] Add --enable-fortify-source option

[Frederic Berat <fberat@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]

On Mon, Jul 3, 2023 at 2:51 PM Adhemerval Zanella Netto <
adhemerval.zanella@linaro.org> wrote:

>
>
> On 03/07/23 05:50, Andreas Schwab wrote:
> > On Jun 30 2023, Siddhesh Poyarekar wrote:
> >
> >> On 2023-06-28 04:42, Frédéric Bérat wrote:
> >>> It is now possible to enable fortification through a configure option.
> >>> The level may be given as parameter, if none is provided, the configure
> >>> script will determine what is the highest level possible that can be
> set
> >>> considering GCC built-ins availability and set it.
> >>> If level is explicitly set to 3, configure checks if the compiler
> >>> supports the built-in function necessary for it or raise an error if it
> >>> isn't.
> >>> The result of the configure checks is a new variables,
> ${fortify_source}
> >>> that can be used to appropriately populate CFLAGS.
> >>> Updated NEWS and INSTALL.
> >>> Adding dedicated x86_64 variant that enables the configuration.
> >>
> >> Adhemerval, do you still think we should drop this and only look at
> >> CFLAGS?  I am still not a 100% convinced that we should only look at
> >> CFLAGS (it gives much less control which makes me uneasy) but I see your
> >> point.  We'll be setting CFLAGS in Fedora anyway (which I guess will be
> >> true for Ubuntu, Gentoo, Debian, etc. too) and the pre-commit CI will
> >> likely have _FORTIFY_SOURCE disabled so we may have adequate coverage.
> >
> > I prefer a configure option, mirroring --enable-stack-protector.  Since
> > glibc has very strict requirements wrt compiler flags it needs to handle
> > it specially anyway, and making it explicit is cleaner.
> >
>
> Fair enough, I am aiming to simplify the configure options and thus the
> build permutation that arise for multiple option; but I see that following
> current practice should be ok.
>
>
That would mean for me to do the following on this patch:
- if "--enable-fortify-source" is set, set -D_FORTIFY_SOURCE accordingly
(already done).
- if "--enable-fortify-source" is NOT set (i.e. assume
"--disable-fortify-source"), forcibly undefine _FORTIFY_SOURCE (currently
not done).

Do you all agree with that ?

Fred.