From: "Fāng-ruì Sòng" <maskray@google.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: Uros Bizjak <ubizjak@gmail.com>,
GCC Patches <gcc-patches@gcc.gnu.org>, Jan Hubicka <jh@suse.cz>,
Florian Weimer <fweimer@redhat.com>,
Richard Biener <rguenther@suse.de>,
Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
Date: Fri, 24 Sep 2021 11:14:00 -0700 [thread overview]
Message-ID: <CAFP8O3Lrmx+Z22rOM4=Oy_pobU9=CkLH0Dh+S7tX33LznJVBEw@mail.gmail.com> (raw)
In-Reply-To: <CAMe9rOriDjsCxyRxg_VtFN2qbyHVtGM2yh=ZeEXe9EYHqKrVrw@mail.gmail.com>
On Fri, Sep 24, 2021 at 10:41 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Sep 24, 2021 at 10:29 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > > > >
> > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches
> > > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > > >
> > > > > > PING^5 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > > > > >
> > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > > >
> > > > > > > PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > > > > > >
> > > > > > > One major design goal of PIE was to avoid copy relocations.
> > > > > > > The original patch for GCC 5 caused problems for many years.
> > > > > > >
> > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > > >>
> > > > > > >> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > > > > > >>
> > > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > > >> >
> > > > > > >> > PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > > > > > >> >
> > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> > > > > > >> > >
> > > > > > >> > > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > > > > > >> > >
> > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song <maskray@google.com> wrote:
> > > > > > >> > > >
> > > > > > >> > > > This was introduced in 2014-12 to use local binding for external symbols
> > > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
> > > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
> > > > > > >> > > > should retire now.
> > > > > > >> > > >
> > > > > > >> > > > One design goal of -fPIE was to avoid copy relocations.
> > > > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change, the
> > > > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
> > > > > > >> > > >
> > > > > > >> > > > ---
> > > > > > >> > > >
> > > > > > >> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
> > > > > > >> > > > of fixed and unfixed (e.g. gold incompatibility with protected
> > > > > > >> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
> > > > > > >> > > >
> > > > > > >> > > > If you prefer a longer write-up, see
> > > > > > >> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
> > > > > > >> > > > ---
> > > > > > >> > > > gcc/config.in | 6 ---
> > > > > > >> > > > gcc/config/i386/i386.c | 11 +---
> > > > > > >> > > > gcc/configure | 52 -------------------
> > > > > > >> > > > gcc/configure.ac | 48 -----------------
> > > > > > >> > > > gcc/doc/sourcebuild.texi | 3 --
> > > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-1.c | 14 -----
> > > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-2.c | 14 -----
> > > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-3.c | 14 -----
> > > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-4.c | 17 ------
> > > > > > >> > > > gcc/testsuite/lib/target-supports.exp | 47 -----------------
> > > > > > >> > > > 10 files changed, 2 insertions(+), 224 deletions(-)
> > > > > > >> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
> > > > > > >> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
> > > > > > >> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
> > > > > > >> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
> > > > >
> > > > > From x86 maintainer's PoV, the implementation is trivially correct,
> > > > > but I have no idea about functionality. HJ, can you please review the
> > > > > functionality and post your opinion on the patch to move it forward?
> > > > >
> > > > > Thanks,
> > > > > Uros.
> > > >
> > > > I prefer to leave it alone and apply this:
> > > >
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-August/576736.html
> > > >
> > > > instead. I am working to add a nodirect_extern_access attribute based
> > > > on feedback at LPC 2021.
> > >
> > > I think -fpie should be fixed as soon as possible.
> > >
> > > "Add -f[no-]direct-extern-access" says "-fdirect-extern-access is the default."
> > > IMHO this is not a good choice for -fpie.
> > > As the description of this patch says, one of the design goals of
> > > -fpie is to avoid copy relocations.
> > >
> > > > In executable and shared library, bind symbols with the STV_PROTECTED visibility locally
> > >
> > > As I have repeated many times (also Clang's behavior), STV_PROTECTED
> > > visibility symbol should be bound locally regardless of
> > > -fno-direct-extern-access.
> > >
> > > I think it is fair to say all of Michael Matz, Alan Modra, and I think
> > > adding so many behaviors under -fno-direct-extern-access is
> > > over-engineering (well, because I don't think
> > > -fno-direct-extern-access can be selected as the default behavior any
> > > time soon).
> > >
> > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected#summary
> >
> > -fno-direct-extern-access should focus on the semantics of non-pic
> > code, which is the traditional configuration which may introduce copy
> > relocations.
> > An important design goal of -fpie/-fPIE was to avoid copy relocations.
> > The 2014 x86-64 patch deviated the direction (I am sorry that some
> > Google folks originated it) and the revert is long due. I am glad that
> > all other architectures still keep the nice property that -fpie/-fPIE
> > never introduces copy relocations.
> >
> > -fdirect-extern-access can do something with -fpie/-fPIE but I doubt
> > anyone may enable it to get the small size benefit.
> > The -f(no-)?direct-extern-access patch doesn't need to check
> > HAVE_LD_PIE_COPYRELOC.
> > The few users (if ever exist) who use -fpie -fdirect-extern-access
> > should ensure their GNU ld supports copy relocations by themselves.
> > GCC configure doesn't need to pay the availability check cost. The
> > support has been available for many years (~2014/2015).
>
> I'd like to get rid of copy relocation for both PIE and non-PIE. But
> we should keep copy relocation as an option for both PIE and non-PIE
> if people ask for it.
They still have a copy relocation option for PIE with your
-fdirect-extern-access :)
People's complaints are about the default behavior for PIE: on all
other architectures, the default code generation should avoid copy
relocations.
For example the Qt issue (https://bugreports.qt.io/browse/QTBUG-45755)
was related to GCC 5 x86-64 switching to copy relocations behavior for
PIE.
Qt folks forced PIC to avoid copy relocations.
-f(no-)?direct-extern-access is a great addition. (As I previously
mentioned I have some small reservation whether a GNU property should
be used, but that is minor preference.)
I don't mind -fno-pic defaulting to -fno-direct-extern-access like
mips in the future.
But I do mind whether PIE users need to explicitly specify
-fno-direct-extern-access to avoid copy relocations.
Like most other architecture and Clang x86-64, they should not need to
specify anything.
They shouldn't need a GNU property to prevent that.
I hope I've given sufficient justification why I think this revert is
still useful with your -f(no-)direct-extern-access.
next prev parent reply other threads:[~2021-09-24 18:14 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 3:29 Fangrui Song
2021-05-12 5:27 ` Rainer Orth
2021-05-12 6:33 ` Fangrui Song
2021-05-24 16:43 ` Fāng-ruì Sòng
2021-06-04 22:04 ` Fāng-ruì Sòng
2021-08-19 6:54 ` Fāng-ruì Sòng
2021-09-04 19:11 ` Fāng-ruì Sòng
2021-09-20 18:19 ` Fāng-ruì Sòng
2021-09-21 16:16 ` Uros Bizjak
2021-09-22 1:56 ` H.J. Lu
2021-09-22 2:08 ` Fāng-ruì Sòng
2021-09-24 17:29 ` Fāng-ruì Sòng
2021-09-24 17:41 ` H.J. Lu
2021-09-24 18:14 ` Fāng-ruì Sòng [this message]
2021-09-24 18:29 ` H.J. Lu
2021-10-08 17:57 ` Fāng-ruì Sòng
2021-11-01 2:36 ` Fāng-ruì Sòng
2022-06-02 7:48 ` Fāng-ruì Sòng
2022-06-15 8:34 Fangrui Song
2022-06-21 3:51 ` Fangrui Song
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='CAFP8O3Lrmx+Z22rOM4=Oy_pobU9=CkLH0Dh+S7tX33LznJVBEw@mail.gmail.com' \
--to=maskray@google.com \
--cc=fweimer@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=jakub@redhat.com \
--cc=jh@suse.cz \
--cc=rguenther@suse.de \
--cc=ubizjak@gmail.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).