* [PATCH] ld: harmonize the value of --enable-warn-execstack=no option
@ 2022-05-17 12:51 Clément Chigot
2022-05-20 5:54 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Clément Chigot @ 2022-05-17 12:51 UTC (permalink / raw)
To: binutils
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
This patch sets the configure value of warn-execstack to 2 when
disabled as expected by bfd.
ld/ChangeLog:
* configure.ac: Update ac_default_ld_warn_execstack value when
--enable-warn-execstack=no is given.
* configure: Regenerate
* testsuite/ld-elf/elf.exp: Add --warn-execstack to ensure
the warning is always shown.
[-- Attachment #2: 0001-ld-harmonize-the-value-of-enable-warn-execstack-no-o.patch --]
[-- Type: text/x-patch, Size: 2043 bytes --]
From 00721c0f99049a8e8adebfadd3e1dc60d00c02e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <chigot@adacore.com>
Date: Tue, 17 May 2022 13:53:56 +0200
Subject: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option
This patch sets the configure value of warn-execstack to 2 when
disabled as expected by bfd.
ld/ChangeLog:
* configure.ac: Update ac_default_ld_warn_execstack value when
--enable-warn-execstack=no is given.
* configure: Regenerate
* testsuite/ld-elf/elf.exp: Add --warn-execstack to ensure
the warning is always shown.
---
ld/configure | 2 +-
ld/configure.ac | 2 +-
ld/testsuite/ld-elf/elf.exp | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ld/configure b/ld/configure
index b4b0ce14ed9..b957227c6c1 100755
--- a/ld/configure
+++ b/ld/configure
@@ -15443,7 +15443,7 @@ ac_default_ld_warn_execstack=unset
if test "${enable_warn_execstack+set}" = set; then :
enableval=$enable_warn_execstack; case "${enableval}" in
yes) ac_default_ld_warn_execstack=1 ;;
- no) ac_default_ld_warn_execstack=-1 ;;
+ no) ac_default_ld_warn_execstack=2 ;;
esac
fi
diff --git a/ld/configure.ac b/ld/configure.ac
index 0b29e810dde..ad807e27465 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -210,7 +210,7 @@ AC_ARG_ENABLE(warn-execstack,
[enable warnings when creating an executable stack]),
[case "${enableval}" in
yes) ac_default_ld_warn_execstack=1 ;;
- no) ac_default_ld_warn_execstack=-1 ;;
+ no) ac_default_ld_warn_execstack=2 ;;
esac])
ac_default_ld_warn_rwx_segments=unset
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index ebb5ba090bf..ec148865e21 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -240,7 +240,7 @@ if { [istarget *-*-*linux*]
run_ld_link_tests [list \
[list "PR ld/29072 (warn about an executable .note.GNU-stack)" \
- "-e 0" \
+ "-e 0 --warn-execstack" \
"" \
"" \
{pr29072-a.s} \
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option 2022-05-17 12:51 [PATCH] ld: harmonize the value of --enable-warn-execstack=no option Clément Chigot @ 2022-05-20 5:54 ` Alan Modra 2022-05-20 6:45 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2022-05-20 5:54 UTC (permalink / raw) To: Clément Chigot; +Cc: binutils On Tue, May 17, 2022 at 02:51:40PM +0200, Clément Chigot via Binutils wrote: > This patch sets the configure value of warn-execstack to 2 when > disabled as expected by bfd. > > ld/ChangeLog: > > * configure.ac: Update ac_default_ld_warn_execstack value when > --enable-warn-execstack=no is given. > * configure: Regenerate > * testsuite/ld-elf/elf.exp: Add --warn-execstack to ensure > the warning is always shown. Thanks for the patch, but I'm suspicious of the testsuite change and I think we need a little more here. I agree a change is needed to make ld/bfd values consistent but I'm going to jump the other way, changing link_info.warn_execstack. Making it consistent that way is nicer in that the flag becomes an "extended" boolean, ie. 0 => don't warn, 1 => always warn, 2 or 3 => conditional warning depending on link_info.execstack. Testing still in progress, I'll commit this shortly if I don't discover the need for a testsuite change. bfd/ * elflink.c (bfd_elf_size_dynamic_sections): Adjust warn_execstack test. include/ * bfdlink.h (warn_execstack): Swap 0 and 2 meaning. ld/ * configure.ac (DEFAULT_LD_WARN_EXECSTACK): Use values of 0, 1, 2 consistent with link_info.warn_execstack. * ld.texi: Typo fixes. * lexsup.c (parse_args): Adjust setting of link_info.warn_execstack. (elf_static_list_options): Adjust help message conditions. * configure: Regenerate. diff --git a/bfd/elflink.c b/bfd/elflink.c index 4d6fe663f68..96eb36aa5bf 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -7179,7 +7179,7 @@ warning: enabling an executable stack because of -z execstack command line optio { if (exec) { - if (info->warn_execstack != 2) + if (info->warn_execstack != 0) { /* PR 29072: Because an executable stack is a serious security risk, make sure that the user knows that it is diff --git a/include/bfdlink.h b/include/bfdlink.h index 27a8e11cc22..09a3ec01685 100644 --- a/include/bfdlink.h +++ b/include/bfdlink.h @@ -493,12 +493,9 @@ struct bfd_link_info unsigned int noexecstack: 1; /* Tri-state variable: - 0 => warn if the linker is creating an executable stack, but - execstack (above) is 0. - 1 => warn if the linker is creating an executable stack; ignores - the value of execstack. - 2 => do not warn. - 3 => not used. */ + 0 => do not warn when creating an executable stack. + 1 => always warn when creating an executable stack. + >1 => warn when creating an executable stack if execstack is 0. */ unsigned int warn_execstack: 2; /* TRUE if warnings should not be generated for TLS segments with eXecute diff --git a/ld/configure b/ld/configure index b4b0ce14ed9..16db825d5ab 100755 --- a/ld/configure +++ b/ld/configure @@ -15438,12 +15438,14 @@ fi -ac_default_ld_warn_execstack=unset +# By default warn when an executable stack is created due to object files +# requesting such, not when the user specifies -z execstack. +ac_default_ld_warn_execstack=2 # Check whether --enable-warn-execstack was given. if test "${enable_warn_execstack+set}" = set; then : enableval=$enable_warn_execstack; case "${enableval}" in yes) ac_default_ld_warn_execstack=1 ;; - no) ac_default_ld_warn_execstack=-1 ;; + no) ac_default_ld_warn_execstack=0 ;; esac fi @@ -16997,9 +16999,6 @@ _ACEOF -if test "${ac_default_ld_warn_execstack}" = unset; then - ac_default_ld_warn_execstack=0 -fi cat >>confdefs.h <<_ACEOF #define DEFAULT_LD_WARN_EXECSTACK $ac_default_ld_warn_execstack diff --git a/ld/configure.ac b/ld/configure.ac index 0b29e810dde..01121480c6d 100644 --- a/ld/configure.ac +++ b/ld/configure.ac @@ -204,13 +204,15 @@ AC_ARG_ENABLE(separate-code, esac]) -ac_default_ld_warn_execstack=unset +# By default warn when an executable stack is created due to object files +# requesting such, not when the user specifies -z execstack. +ac_default_ld_warn_execstack=2 AC_ARG_ENABLE(warn-execstack, AS_HELP_STRING([--enable-warn-execstack], [enable warnings when creating an executable stack]), [case "${enableval}" in yes) ac_default_ld_warn_execstack=1 ;; - no) ac_default_ld_warn_execstack=-1 ;; + no) ac_default_ld_warn_execstack=0 ;; esac]) ac_default_ld_warn_rwx_segments=unset @@ -531,9 +533,6 @@ AC_DEFINE_UNQUOTED(DEFAULT_LD_Z_SEPARATE_CODE, [Define to 1 if you want to enable -z separate-code in ELF linker by default.]) -if test "${ac_default_ld_warn_execstack}" = unset; then - ac_default_ld_warn_execstack=0 -fi AC_DEFINE_UNQUOTED(DEFAULT_LD_WARN_EXECSTACK, $ac_default_ld_warn_execstack, [Define to 1 if you want to enable --warn-execstack in ELF linker by default.]) diff --git a/ld/ld.texi b/ld/ld.texi index 8cad8478140..a2b162ce5b5 100644 --- a/ld/ld.texi +++ b/ld/ld.texi @@ -2654,7 +2654,7 @@ object file formats. For formats like COFF or ELF, the linker can not detect the use of global constructors. @kindex --warn-execstack -@cindex warnings, on exectuable stack +@cindex warnings, on executable stack @cindex executable stack, warnings on @item --warn-execstack @itemx --no-warn-execstack @@ -2667,7 +2667,7 @@ line option has been used, but this behaviour can be overridden by the On the other hand the linker will normally warn if the stack is made executable because one or more of the input files need an execuable stack and neither of the @command{-z execstack} or @command{-z -noexecstack} comman line options have been specified. This warning +noexecstack} command line options have been specified. This warning can be disabled via the @command{--no-warn-execstack} option. Note: ELF format input files specify that they need an executable diff --git a/ld/lexsup.c b/ld/lexsup.c index 78190472907..82c459adb51 100644 --- a/ld/lexsup.c +++ b/ld/lexsup.c @@ -927,7 +927,7 @@ parse_args (unsigned argc, char **argv) link_info.warn_execstack = 1; break; case OPTION_NO_WARN_EXECSTACK: - link_info.warn_execstack = 2; + link_info.warn_execstack = 0; break; case OPTION_WARN_RWX_SEGMENTS: link_info.no_warn_rwx_segments = 0; @@ -2169,14 +2169,14 @@ elf_static_list_options (FILE *file) -z execstack Mark executable as requiring executable stack\n")); fprintf (file, _("\ -z noexecstack Mark executable as not requiring executable stack\n")); -#if DEFAULT_LD_WARN_EXECSTACK > 0 +#if DEFAULT_LD_WARN_EXECSTACK == 1 fprintf (file, _("\ --warn-execstack Generate a warning if the stack is executable (default)\n")); #else fprintf (file, _("\ --warn-execstack Generate a warning if the stack is executable\n")); #endif -#if DEFAULT_LD_WARN_EXECSTACK < 0 +#if DEFAULT_LD_WARN_EXECSTACK == 0 fprintf (file, _("\ --no-warn-execstack Do not generate a warning if the stack is executable (default)\n")); #else -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option 2022-05-20 5:54 ` Alan Modra @ 2022-05-20 6:45 ` Alan Modra 2022-05-20 7:07 ` Clément Chigot 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2022-05-20 6:45 UTC (permalink / raw) To: Clément Chigot, Nick Clifton; +Cc: binutils On Fri, May 20, 2022 at 03:24:23PM +0930, Alan Modra wrote: > On Tue, May 17, 2022 at 02:51:40PM +0200, Clément Chigot via Binutils wrote: > > This patch sets the configure value of warn-execstack to 2 when > > disabled as expected by bfd. > > > > ld/ChangeLog: > > > > * configure.ac: Update ac_default_ld_warn_execstack value when > > --enable-warn-execstack=no is given. > > * configure: Regenerate > > * testsuite/ld-elf/elf.exp: Add --warn-execstack to ensure > > the warning is always shown. > > Thanks for the patch, but I'm suspicious of the testsuite change and I > think we need a little more here. I agree a change is needed to make > ld/bfd values consistent but I'm going to jump the other way, changing > link_info.warn_execstack. Making it consistent that way is nicer in > that the flag becomes an "extended" boolean, ie. 0 => don't warn, > 1 => always warn, 2 or 3 => conditional warning depending on > link_info.execstack. > > Testing still in progress, I'll commit this shortly if I don't > discover the need for a testsuite change. So my patch results in aarch64_be-linux-gnu_ilp32 +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) aarch64-linux +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) armeb-linuxeabi +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) arm-linuxeabi +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) arm-nacl +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) That's really an arm/aarch64 backend problem in that those targets define a gld${EMULATION_NAME}_before_parse function that overrides the one in elf.em. So at the moment it looks like DEFAULT_LD_Z_SEPARATE_CODE, DEFAULT_LD_WARN_EXECSTACK, DEFAULT_LD_WARN_RWX_SEGMENTS and DEFAULT_LD_EXECSTACK all do nothing on arm/aarch64. The obvious fix would be to add a few lines to the arm/aarch64 before_parse functions but I'm going to leave that to an ARM maintainer. A more elegant solution might be possible. I'm going to ignore buildbots or user complaints that this change of mine introduced regressions as the truth is the new failure exposes a real bug in arm/aarch64. Committed. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option 2022-05-20 6:45 ` Alan Modra @ 2022-05-20 7:07 ` Clément Chigot 2022-05-20 10:16 ` Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Clément Chigot @ 2022-05-20 7:07 UTC (permalink / raw) To: Alan Modra; +Cc: Nick Clifton, binutils Hi Alan On Fri, May 20, 2022 at 8:45 AM Alan Modra <amodra@gmail.com> wrote: > > On Fri, May 20, 2022 at 03:24:23PM +0930, Alan Modra wrote: > > On Tue, May 17, 2022 at 02:51:40PM +0200, Clément Chigot via Binutils wrote: > > > This patch sets the configure value of warn-execstack to 2 when > > > disabled as expected by bfd. > > > > > > ld/ChangeLog: > > > > > > * configure.ac: Update ac_default_ld_warn_execstack value when > > > --enable-warn-execstack=no is given. > > > * configure: Regenerate > > > * testsuite/ld-elf/elf.exp: Add --warn-execstack to ensure > > > the warning is always shown. > > > > Thanks for the patch, but I'm suspicious of the testsuite change and I > > think we need a little more here. I agree a change is needed to make > > ld/bfd values consistent but I'm going to jump the other way, changing > > link_info.warn_execstack. Making it consistent that way is nicer in > > that the flag becomes an "extended" boolean, ie. 0 => don't warn, > > 1 => always warn, 2 or 3 => conditional warning depending on > > link_info.execstack. > > > > Testing still in progress, I'll commit this shortly if I don't > > discover the need for a testsuite change. > > So my patch results in > aarch64_be-linux-gnu_ilp32 +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) > aarch64-linux +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) > armeb-linuxeabi +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) > arm-linuxeabi +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) > arm-nacl +FAIL: PR ld/29072 (warn about an executable .note.GNU-stack) Thanks for handling this patch. However, the testsuite changes look mandatory to me. As of now, the results are different dependending on the configure options being passed. If the warnings are disabled, ld/29072 will always fail. If the warnings are forced, stack exec will fail too. I agree that forcing --warn-execstack might not be the right approach. Maybe disabling the problematic tests and creating new ones: tests that are run only when a given configure option is set, would be better. That way we can also check that the behavior of each configure choice is preserved. Clément ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option 2022-05-20 7:07 ` Clément Chigot @ 2022-05-20 10:16 ` Alan Modra 2022-05-26 11:06 ` bit-rot in target before_parse function option Alan Modra 0 siblings, 1 reply; 6+ messages in thread From: Alan Modra @ 2022-05-20 10:16 UTC (permalink / raw) To: Clément Chigot; +Cc: Nick Clifton, binutils On Fri, May 20, 2022 at 09:07:06AM +0200, Clément Chigot wrote: > However, the testsuite changes look mandatory to me. > As of now, the results are different dependending on the > configure options being passed. Yes, you may well be correct. I'm going to leave this to Nick. Incidentally, the reason why I didn't fix the arm/aarch64 regressions is that if link_info.separate_code = DEFAULT_LD_Z_SEPARATE_CODE; link_info.warn_execstack = DEFAULT_LD_WARN_EXECSTACK; link_info.no_warn_rwx_segments = ! DEFAULT_LD_WARN_RWX_SEGMENTS; link_info.default_execstack = DEFAULT_LD_EXECSTACK; is added to armelf.em:before_parse then we get FAIL: PR ld/29072 (ignore absent .note.GNU-stack) The reason being that arm is listed in ld/testsuite/ld-elf/elf.exp target_defaults_to_execstack but doesn't have elf_backend_default_execstack set. > If the warnings are disabled, ld/29072 will always fail. > If the warnings are forced, stack exec will fail too. > > I agree that forcing --warn-execstack might not be the > right approach. > Maybe disabling the problematic tests and creating > new ones: tests that are run only when a given > configure option is set, would be better. That way > we can also check that the behavior of each configure > choice is preserved. > > Clément -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
* bit-rot in target before_parse function option 2022-05-20 10:16 ` Alan Modra @ 2022-05-26 11:06 ` Alan Modra 0 siblings, 0 replies; 6+ messages in thread From: Alan Modra @ 2022-05-26 11:06 UTC (permalink / raw) To: binutils On Fri, May 20, 2022 at 07:46:10PM +0930, Alan Modra wrote: > On Fri, May 20, 2022 at 09:07:06AM +0200, Clément Chigot wrote: > > However, the testsuite changes look mandatory to me. > > As of now, the results are different dependending on the > > configure options being passed. > > Yes, you may well be correct. I'm going to leave this to Nick. > Incidentally, the reason why I didn't fix the arm/aarch64 regressions > is that if > > link_info.separate_code = DEFAULT_LD_Z_SEPARATE_CODE; > link_info.warn_execstack = DEFAULT_LD_WARN_EXECSTACK; > link_info.no_warn_rwx_segments = ! DEFAULT_LD_WARN_RWX_SEGMENTS; > link_info.default_execstack = DEFAULT_LD_EXECSTACK; > > is added to armelf.em:before_parse then we get > > FAIL: PR ld/29072 (ignore absent .note.GNU-stack) > > The reason being that arm is listed in ld/testsuite/ld-elf/elf.exp > target_defaults_to_execstack but doesn't have > elf_backend_default_execstack set. I guess I really shouldn't just palm this off to Nick, leaving testsuite regressions. Copy initialisation over from the elf.em before_parse. Commit ba951afb999 2022-05-03 changed behaviour on arm and score regarding exec stack. This patch restores the previous behaviour. * emultempl/aarch64elf.em (before_parse): Init separate_code, warn_execstack, no_warn_rwx_segments and default_execstack. * emultempl/armelf.em (before_parse): Likewise. * emultempl/scoreelf.em (before_parse): Likewise. * testsuite/ld-elf/elf.exp (target_defaults_to_execstack): Return true for arm and nacl. diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em index 26e3dd243fc..1968f12cf29 100644 --- a/ld/emultempl/aarch64elf.em +++ b/ld/emultempl/aarch64elf.em @@ -54,6 +54,10 @@ fragment <<EOF EOF fi fragment <<EOF + link_info.separate_code = DEFAULT_LD_Z_SEPARATE_CODE; + link_info.warn_execstack = DEFAULT_LD_WARN_EXECSTACK; + link_info.no_warn_rwx_segments = ! DEFAULT_LD_WARN_RWX_SEGMENTS; + link_info.default_execstack = DEFAULT_LD_EXECSTACK; } static void diff --git a/ld/emultempl/armelf.em b/ld/emultempl/armelf.em index 885067fa256..8f5334df198 100644 --- a/ld/emultempl/armelf.em +++ b/ld/emultempl/armelf.em @@ -67,6 +67,10 @@ fragment <<EOF EOF fi fragment <<EOF + link_info.separate_code = DEFAULT_LD_Z_SEPARATE_CODE; + link_info.warn_execstack = DEFAULT_LD_WARN_EXECSTACK; + link_info.no_warn_rwx_segments = ! DEFAULT_LD_WARN_RWX_SEGMENTS; + link_info.default_execstack = DEFAULT_LD_EXECSTACK; } static void diff --git a/ld/emultempl/scoreelf.em b/ld/emultempl/scoreelf.em index f5dd06246de..d781dd96057 100644 --- a/ld/emultempl/scoreelf.em +++ b/ld/emultempl/scoreelf.em @@ -47,6 +47,10 @@ fragment <<EOF EOF fi fragment <<EOF + link_info.separate_code = DEFAULT_LD_Z_SEPARATE_CODE; + link_info.warn_execstack = DEFAULT_LD_WARN_EXECSTACK; + link_info.no_warn_rwx_segments = ! DEFAULT_LD_WARN_RWX_SEGMENTS; + link_info.default_execstack = DEFAULT_LD_EXECSTACK; } static void diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp index ebb5ba090bf..621d4e121d7 100644 --- a/ld/testsuite/ld-elf/elf.exp +++ b/ld/testsuite/ld-elf/elf.exp @@ -182,8 +182,6 @@ if { [check_gc_sections_available] && ![istarget "v850-*-*"] } { proc target_defaults_to_execstack {} { if { [istarget "aarch64*-*-*"] - || [istarget "arm*-*-*"] - || [istarget "*-*-nacl"] || [istarget "arc*-*-*"] || [istarget "ia64*-*-*"] || [istarget "nios2*-*-*"] -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-26 11:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-17 12:51 [PATCH] ld: harmonize the value of --enable-warn-execstack=no option Clément Chigot 2022-05-20 5:54 ` Alan Modra 2022-05-20 6:45 ` Alan Modra 2022-05-20 7:07 ` Clément Chigot 2022-05-20 10:16 ` Alan Modra 2022-05-26 11:06 ` bit-rot in target before_parse function option Alan Modra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).