From: "Clément Chigot" <chigot@adacore.com>
To: Alan Modra <amodra@gmail.com>
Cc: Nick Clifton <nickc@redhat.com>, binutils@sourceware.org
Subject: Re: [PATCH] ld: harmonize the value of --enable-warn-execstack=no option
Date: Fri, 20 May 2022 09:07:06 +0200 [thread overview]
Message-ID: <CAJ307EiODo2YZSFTgkOs5igW581S4wbmQ-qGGuC4E1w7QMi8fQ@mail.gmail.com> (raw)
In-Reply-To: <Yoc5BuOKOPlNJeKt@squeak.grove.modra.org>
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
next prev parent reply other threads:[~2022-05-20 7:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-17 12:51 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 [this message]
2022-05-20 10:16 ` Alan Modra
2022-05-26 11:06 ` bit-rot in target before_parse function option Alan Modra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ307EiODo2YZSFTgkOs5igW581S4wbmQ-qGGuC4E1w7QMi8fQ@mail.gmail.com \
--to=chigot@adacore.com \
--cc=amodra@gmail.com \
--cc=binutils@sourceware.org \
--cc=nickc@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).