| Message ID | 20230720144001.2088610-1-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | scripts: Fix fortify checks if compiler does not support _FORTIFY_SOURCE=3 | expand |
On 2023-07-20 10:40, Adhemerval Zanella via Libc-alpha wrote: > The 30379efad1 added _FORTIFY_SOURCE checks without check if compiler > does support all used fortify levels. This patch fixes it by first > checking at configure time the maximum support fortify level and using > it instead of a pre-defined one. > > Checked on x86_64 with gcc 11, 12, and 13. > --- LGTM, pending approval from RM. At the moment there's no real reason to verify headers for different fortification levels (just 1 or 2 would do) but I reckon it's a future-proof change, in case we ever end up diverging further on those levels. Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Thanks, Sid > Makefile | 4 ++-- > Rules | 4 ++-- > configure | 28 ++++++++++++---------------- > configure.ac | 13 +++++++------ > scripts/check-installed-headers.sh | 11 +++++++---- > 5 files changed, 30 insertions(+), 30 deletions(-) > > diff --git a/Makefile b/Makefile > index f324df7a1f..c6d4817a9e 100644 > --- a/Makefile > +++ b/Makefile > @@ -545,7 +545,7 @@ tests-special += $(objpfx)check-installed-headers-c.out > libof-check-installed-headers-c := testsuite > $(objpfx)check-installed-headers-c.out: \ > scripts/check-installed-headers.sh $(headers) > - $(SHELL) $(..)scripts/check-installed-headers.sh c \ > + $(SHELL) $(..)scripts/check-installed-headers.sh c $(supported-fortify) \ > "$(CC) $(filter-out -std=%,$(CFLAGS)) -D_ISOMAC $(+includes)" \ > $(headers) > $@; \ > $(evaluate-test) > @@ -555,7 +555,7 @@ tests-special += $(objpfx)check-installed-headers-cxx.out > libof-check-installed-headers-cxx := testsuite > $(objpfx)check-installed-headers-cxx.out: \ > scripts/check-installed-headers.sh $(headers) > - $(SHELL) $(..)scripts/check-installed-headers.sh c++ \ > + $(SHELL) $(..)scripts/check-installed-headers.sh c++ $(supported-fortify) \ > "$(CXX) $(filter-out -std=%,$(CXXFLAGS)) -D_ISOMAC $(+includes)" \ > $(headers) > $@; \ > $(evaluate-test) > diff --git a/Rules b/Rules > index 5e945d7347..279ae490ac 100644 > --- a/Rules > +++ b/Rules > @@ -85,7 +85,7 @@ tests-special += $(objpfx)check-installed-headers-c.out > libof-check-installed-headers-c := testsuite > $(objpfx)check-installed-headers-c.out: \ > $(..)scripts/check-installed-headers.sh $(headers) > - $(SHELL) $(..)scripts/check-installed-headers.sh c \ > + $(SHELL) $(..)scripts/check-installed-headers.sh c $(supported-fortify) \ > "$(CC) $(filter-out -std=%,$(CFLAGS)) -D_ISOMAC $(+includes)" \ > $(headers) > $@; \ > $(evaluate-test) > @@ -97,7 +97,7 @@ tests-special += $(objpfx)check-installed-headers-cxx.out > libof-check-installed-headers-cxx := testsuite > $(objpfx)check-installed-headers-cxx.out: \ > $(..)scripts/check-installed-headers.sh $(headers) > - $(SHELL) $(..)scripts/check-installed-headers.sh c++ \ > + $(SHELL) $(..)scripts/check-installed-headers.sh c++ $(supported-fortify) \ > "$(CXX) $(filter-out -std=%,$(CXXFLAGS)) -D_ISOMAC $(+includes)" \ > $(headers) > $@; \ > $(evaluate-test) > diff --git a/configure b/configure > index c02c0b5825..14e2ecce28 100755 > --- a/configure > +++ b/configure > @@ -7610,9 +7610,9 @@ fi > no_fortify_source="-Wp,-U_FORTIFY_SOURCE" > fortify_source="${no_fortify_source}" > > -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for __builtin_dynamic_object_size" >&5 > -printf %s "checking for __builtin_dynamic_object_size... " >&6; } > -if test ${libc_cv___builtin_dynamic_object_size+y} > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for maximum supported _FORTIFY_SOURCE level" >&5 > +printf %s "checking for maximum supported _FORTIFY_SOURCE level... " >&6; } > +if test ${libc_cv_supported_fortify_source+y} > then : > printf %s "(cached) " >&6 > else $as_nop > @@ -7630,30 +7630,24 @@ __builtin_dynamic_object_size("", 0) > _ACEOF > if ac_fn_c_try_link "$LINENO" > then : > - libc_cv___builtin_dynamic_object_size=yes > - if test "$enable_fortify_source" = yes > -then : > - enable_fortify_source=3 > -fi > + libc_cv_supported_fortify_source=3 > else $as_nop > - libc_cv___builtin_dynamic_object_size=no > - if test "$enable_fortify_source" = yes > -then : > - enable_fortify_source=2 > -fi > + libc_cv_supported_fortify_source=2 > fi > rm -f core conftest.err conftest.$ac_objext conftest.beam \ > conftest$ac_exeext conftest.$ac_ext > > fi > -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv___builtin_dynamic_object_size" >&5 > -printf "%s\n" "$libc_cv___builtin_dynamic_object_size" >&6; } > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_supported_fortify_source" >&5 > +printf "%s\n" "$libc_cv_supported_fortify_source" >&6; } > > case $enable_fortify_source in #( > + yes) : > + libc_cv_fortify_source=yes enable_fortify_source=$libc_cv_supported_fortify_source ;; #( > 1|2) : > libc_cv_fortify_source=yes ;; #( > 3) : > - if test "$libc_cv___builtin_dynamic_object_size" = yes > + if test $libc_cv_supported_fortify_source = 3 > then : > libc_cv_fortify_source=yes > else $as_nop > @@ -7673,6 +7667,8 @@ fi > > > > +config_vars="$config_vars > +supported-fortify = $libc_cv_supported_fortify_source" > > { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether the assembler requires one version per symbol" >&5 > printf %s "checking whether the assembler requires one version per symbol... " >&6; } > diff --git a/configure.ac b/configure.ac > index 09553541fb..ff35bf4d6b 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1578,17 +1578,17 @@ dnl support it > no_fortify_source="-Wp,-U_FORTIFY_SOURCE" > fortify_source="${no_fortify_source}" > > -AC_CACHE_CHECK([for __builtin_dynamic_object_size], [libc_cv___builtin_dynamic_object_size], [ > +AC_CACHE_CHECK([for maximum supported _FORTIFY_SOURCE level], > + [libc_cv_supported_fortify_source], [ > AC_LINK_IFELSE([AC_LANG_PROGRAM([], [__builtin_dynamic_object_size("", 0)])], > - [libc_cv___builtin_dynamic_object_size=yes > - AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=3])], > - [libc_cv___builtin_dynamic_object_size=no > - AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=2])]) > + [libc_cv_supported_fortify_source=3], > + [libc_cv_supported_fortify_source=2]) > ]) > > AS_CASE([$enable_fortify_source], > + [yes], [libc_cv_fortify_source=yes enable_fortify_source=$libc_cv_supported_fortify_source], > [1|2], [libc_cv_fortify_source=yes], > - [3], [AS_IF([test "$libc_cv___builtin_dynamic_object_size" = yes], > + [3], [AS_IF([test $libc_cv_supported_fortify_source = 3], > [libc_cv_fortify_source=yes], > [AC_MSG_ERROR([Compiler doesn't provide necessary support for _FORTIFY_SOURCE=3])])], > [libc_cv_fortify_source=no]) > @@ -1601,6 +1601,7 @@ AC_SUBST(enable_fortify_source) > AC_SUBST(libc_cv_fortify_source) > AC_SUBST(no_fortify_source) > AC_SUBST(fortify_source) > +LIBC_CONFIG_VAR([supported-fortify], [$libc_cv_supported_fortify_source]) > > dnl Starting with binutils 2.35, GAS can attach multiple symbol versions > dnl to one symbol (PR 23840). > diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh > index 23506a2514..ee9f534ab0 100644 > --- a/scripts/check-installed-headers.sh > +++ b/scripts/check-installed-headers.sh > @@ -29,11 +29,12 @@ cxx_modes="-std=c++98 -std=gnu++98 -std=c++11 -std=gnu++11" > # These are probably the most commonly used three. > lib_modes="-D_DEFAULT_SOURCE=1 -D_GNU_SOURCE=1 -D_XOPEN_SOURCE=700" > > -# Also check for fortify modes, since it might be enabled as default. > -fortify_modes="1 2 3" > +# Also check for fortify modes, since it might be enabled as default. The > +# maximum value to be checked is define by maximum_fortify argument. > +fortify_modes="" > > if [ $# -lt 3 ]; then > - echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 > + echo "usage: $0 c|c++ maximum_fortify \"compile command\" header header header..." >&2 > exit 2 > fi > case "$1" in > @@ -50,6 +51,8 @@ case "$1" in > exit 2;; > esac > shift > +fortify_modes=$(seq -s' ' 1 $1) > +shift > cc_cmd="$1" > shift > trap "rm -f '$cih_test_c'" 0 > @@ -104,7 +107,6 @@ EOF > for lang_mode in "" $lang_modes; do > for lib_mode in "" $lib_modes; do > for fortify_mode in "" $fortify_modes; do > - echo :::: $lang_mode $lib_mode $fortify_mode > if [ -z "$lib_mode" ]; then > expanded_lib_mode='/* default library mode */' > else > @@ -114,6 +116,7 @@ EOF > if [ ! -z $fortify_mode ]; then > fortify_mode="#define _FORTIFY_SOURCE $fortify_mode" > fi > + echo :::: $lang_mode $lib_mode $fortify_mode > cat >"$cih_test_c" <<EOF > /* These macros may have been defined on the command line. They are > inappropriate for this test. */
* Siddhesh Poyarekar: > On 2023-07-20 10:40, Adhemerval Zanella via Libc-alpha wrote: >> The 30379efad1 added _FORTIFY_SOURCE checks without check if compiler >> does support all used fortify levels. This patch fixes it by first >> checking at configure time the maximum support fortify level and using >> it instead of a pre-defined one. >> Checked on x86_64 with gcc 11, 12, and 13. >> --- > > LGTM, pending approval from RM. At the moment there's no real reason > to verify headers for different fortification levels (just 1 or 2 > would do) but I reckon it's a future-proof change, in case we ever end > up diverging further on those levels. > > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Agreed, and (with GCC 11): Tested-by: Florian Weimer <fweimer@redhat.com> Thanks, Florian
Am Donnerstag, 20. Juli 2023, 18:26:53 CEST schrieb Florian Weimer: > * Siddhesh Poyarekar: > > > On 2023-07-20 10:40, Adhemerval Zanella via Libc-alpha wrote: > >> The 30379efad1 added _FORTIFY_SOURCE checks without check if compiler > >> does support all used fortify levels. This patch fixes it by first > >> checking at configure time the maximum support fortify level and using > >> it instead of a pre-defined one. > >> Checked on x86_64 with gcc 11, 12, and 13. > >> --- > > > > LGTM, pending approval from RM. At the moment there's no real reason > > to verify headers for different fortification levels (just 1 or 2 > > would do) but I reckon it's a future-proof change, in case we ever end > > up diverging further on those levels. > > > > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > > Agreed, and (with GCC 11): > > Tested-by: Florian Weimer <fweimer@redhat.com> > OK let's go with it. Thanks! -a
diff --git a/Makefile b/Makefile index f324df7a1f..c6d4817a9e 100644 --- a/Makefile +++ b/Makefile @@ -545,7 +545,7 @@ tests-special += $(objpfx)check-installed-headers-c.out libof-check-installed-headers-c := testsuite $(objpfx)check-installed-headers-c.out: \ scripts/check-installed-headers.sh $(headers) - $(SHELL) $(..)scripts/check-installed-headers.sh c \ + $(SHELL) $(..)scripts/check-installed-headers.sh c $(supported-fortify) \ "$(CC) $(filter-out -std=%,$(CFLAGS)) -D_ISOMAC $(+includes)" \ $(headers) > $@; \ $(evaluate-test) @@ -555,7 +555,7 @@ tests-special += $(objpfx)check-installed-headers-cxx.out libof-check-installed-headers-cxx := testsuite $(objpfx)check-installed-headers-cxx.out: \ scripts/check-installed-headers.sh $(headers) - $(SHELL) $(..)scripts/check-installed-headers.sh c++ \ + $(SHELL) $(..)scripts/check-installed-headers.sh c++ $(supported-fortify) \ "$(CXX) $(filter-out -std=%,$(CXXFLAGS)) -D_ISOMAC $(+includes)" \ $(headers) > $@; \ $(evaluate-test) diff --git a/Rules b/Rules index 5e945d7347..279ae490ac 100644 --- a/Rules +++ b/Rules @@ -85,7 +85,7 @@ tests-special += $(objpfx)check-installed-headers-c.out libof-check-installed-headers-c := testsuite $(objpfx)check-installed-headers-c.out: \ $(..)scripts/check-installed-headers.sh $(headers) - $(SHELL) $(..)scripts/check-installed-headers.sh c \ + $(SHELL) $(..)scripts/check-installed-headers.sh c $(supported-fortify) \ "$(CC) $(filter-out -std=%,$(CFLAGS)) -D_ISOMAC $(+includes)" \ $(headers) > $@; \ $(evaluate-test) @@ -97,7 +97,7 @@ tests-special += $(objpfx)check-installed-headers-cxx.out libof-check-installed-headers-cxx := testsuite $(objpfx)check-installed-headers-cxx.out: \ $(..)scripts/check-installed-headers.sh $(headers) - $(SHELL) $(..)scripts/check-installed-headers.sh c++ \ + $(SHELL) $(..)scripts/check-installed-headers.sh c++ $(supported-fortify) \ "$(CXX) $(filter-out -std=%,$(CXXFLAGS)) -D_ISOMAC $(+includes)" \ $(headers) > $@; \ $(evaluate-test) diff --git a/configure b/configure index c02c0b5825..14e2ecce28 100755 --- a/configure +++ b/configure @@ -7610,9 +7610,9 @@ fi no_fortify_source="-Wp,-U_FORTIFY_SOURCE" fortify_source="${no_fortify_source}" -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for __builtin_dynamic_object_size" >&5 -printf %s "checking for __builtin_dynamic_object_size... " >&6; } -if test ${libc_cv___builtin_dynamic_object_size+y} +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for maximum supported _FORTIFY_SOURCE level" >&5 +printf %s "checking for maximum supported _FORTIFY_SOURCE level... " >&6; } +if test ${libc_cv_supported_fortify_source+y} then : printf %s "(cached) " >&6 else $as_nop @@ -7630,30 +7630,24 @@ __builtin_dynamic_object_size("", 0) _ACEOF if ac_fn_c_try_link "$LINENO" then : - libc_cv___builtin_dynamic_object_size=yes - if test "$enable_fortify_source" = yes -then : - enable_fortify_source=3 -fi + libc_cv_supported_fortify_source=3 else $as_nop - libc_cv___builtin_dynamic_object_size=no - if test "$enable_fortify_source" = yes -then : - enable_fortify_source=2 -fi + libc_cv_supported_fortify_source=2 fi rm -f core conftest.err conftest.$ac_objext conftest.beam \ conftest$ac_exeext conftest.$ac_ext fi -{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv___builtin_dynamic_object_size" >&5 -printf "%s\n" "$libc_cv___builtin_dynamic_object_size" >&6; } +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_supported_fortify_source" >&5 +printf "%s\n" "$libc_cv_supported_fortify_source" >&6; } case $enable_fortify_source in #( + yes) : + libc_cv_fortify_source=yes enable_fortify_source=$libc_cv_supported_fortify_source ;; #( 1|2) : libc_cv_fortify_source=yes ;; #( 3) : - if test "$libc_cv___builtin_dynamic_object_size" = yes + if test $libc_cv_supported_fortify_source = 3 then : libc_cv_fortify_source=yes else $as_nop @@ -7673,6 +7667,8 @@ fi +config_vars="$config_vars +supported-fortify = $libc_cv_supported_fortify_source" { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether the assembler requires one version per symbol" >&5 printf %s "checking whether the assembler requires one version per symbol... " >&6; } diff --git a/configure.ac b/configure.ac index 09553541fb..ff35bf4d6b 100644 --- a/configure.ac +++ b/configure.ac @@ -1578,17 +1578,17 @@ dnl support it no_fortify_source="-Wp,-U_FORTIFY_SOURCE" fortify_source="${no_fortify_source}" -AC_CACHE_CHECK([for __builtin_dynamic_object_size], [libc_cv___builtin_dynamic_object_size], [ +AC_CACHE_CHECK([for maximum supported _FORTIFY_SOURCE level], + [libc_cv_supported_fortify_source], [ AC_LINK_IFELSE([AC_LANG_PROGRAM([], [__builtin_dynamic_object_size("", 0)])], - [libc_cv___builtin_dynamic_object_size=yes - AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=3])], - [libc_cv___builtin_dynamic_object_size=no - AS_IF([test "$enable_fortify_source" = yes], [enable_fortify_source=2])]) + [libc_cv_supported_fortify_source=3], + [libc_cv_supported_fortify_source=2]) ]) AS_CASE([$enable_fortify_source], + [yes], [libc_cv_fortify_source=yes enable_fortify_source=$libc_cv_supported_fortify_source], [1|2], [libc_cv_fortify_source=yes], - [3], [AS_IF([test "$libc_cv___builtin_dynamic_object_size" = yes], + [3], [AS_IF([test $libc_cv_supported_fortify_source = 3], [libc_cv_fortify_source=yes], [AC_MSG_ERROR([Compiler doesn't provide necessary support for _FORTIFY_SOURCE=3])])], [libc_cv_fortify_source=no]) @@ -1601,6 +1601,7 @@ AC_SUBST(enable_fortify_source) AC_SUBST(libc_cv_fortify_source) AC_SUBST(no_fortify_source) AC_SUBST(fortify_source) +LIBC_CONFIG_VAR([supported-fortify], [$libc_cv_supported_fortify_source]) dnl Starting with binutils 2.35, GAS can attach multiple symbol versions dnl to one symbol (PR 23840). diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index 23506a2514..ee9f534ab0 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -29,11 +29,12 @@ cxx_modes="-std=c++98 -std=gnu++98 -std=c++11 -std=gnu++11" # These are probably the most commonly used three. lib_modes="-D_DEFAULT_SOURCE=1 -D_GNU_SOURCE=1 -D_XOPEN_SOURCE=700" -# Also check for fortify modes, since it might be enabled as default. -fortify_modes="1 2 3" +# Also check for fortify modes, since it might be enabled as default. The +# maximum value to be checked is define by maximum_fortify argument. +fortify_modes="" if [ $# -lt 3 ]; then - echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 + echo "usage: $0 c|c++ maximum_fortify \"compile command\" header header header..." >&2 exit 2 fi case "$1" in @@ -50,6 +51,8 @@ case "$1" in exit 2;; esac shift +fortify_modes=$(seq -s' ' 1 $1) +shift cc_cmd="$1" shift trap "rm -f '$cih_test_c'" 0 @@ -104,7 +107,6 @@ EOF for lang_mode in "" $lang_modes; do for lib_mode in "" $lib_modes; do for fortify_mode in "" $fortify_modes; do - echo :::: $lang_mode $lib_mode $fortify_mode if [ -z "$lib_mode" ]; then expanded_lib_mode='/* default library mode */' else @@ -114,6 +116,7 @@ EOF if [ ! -z $fortify_mode ]; then fortify_mode="#define _FORTIFY_SOURCE $fortify_mode" fi + echo :::: $lang_mode $lib_mode $fortify_mode cat >"$cih_test_c" <<EOF /* These macros may have been defined on the command line. They are inappropriate for this test. */