public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Alexandre Oliva <oliva@adacore.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Jan Hubicka <hubicka@ucw.cz>,  Uros Bizjak <ubizjak@gmail.com>
Subject: Re: [PATCH] [PR83782] i386 PIE: avoid @GOTOFF for ifuncs and their aliases
Date: Thu, 28 Jul 2022 09:31:27 -0700	[thread overview]
Message-ID: <CAMe9rOpG17d173YKpKb88tCBe5HvQPUo5ZL_No=YyqpjcUvpLA@mail.gmail.com> (raw)
In-Reply-To: <or4jz1prrd.fsf@lxoliva.fsfla.org>

On Thu, Jul 28, 2022 at 1:26 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jul 27, 2022, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
> > On Tue, Jul 26, 2022 at 10:14 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> >> The use of @GOTOFF for locally-bound but externally-visible symbols
> >> (e.g. protected visibility) also breaks pointer identity if the
> >> canonical address ends up preempted by a PLT entry.
>
> > Here is a different fix:
>
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598667.html
>
> Oh, thanks, I'd missed that.
>
> It doesn't seem to fix the part of the problem I quoted above, though.
> I think fixing that requires testing the visibility, to make sure the
> symbol's canonical address cannot be preempted, which may occur with
> local binding, if the symbol is protected and referenced in the main
> program, otherwise pointer identity is broken again, admittedly for a
> more obscure case, but pointer identity was the point of the PR.

The protected symbol issue isn't IFUNC specific.   The protected
symbol handling is changed in glibc 2.36 and binutils 2.39.   Only
the address of the protected symbol definition should be used as
its address.

> >> * config/i386/i386.cc (ix86_call_use_plt_p): Follow the alias
> >> chain looking for an ifunc, as in gcc.target/i386/mvc10.c.
>
> You may also need to do something like this bit for mvc10.c on ia32 PIE.
> Because the ifunc is called through an alias, AFAICT we don't even
> notice that the call target is (an alias to) an ifunc.  GCC's
> gotoff_operand predicate accepts it, but binutils (the linker, IIRC)
> then rejects that reference, because the named symbol is an alias to an
> ifunc.

Yes, this change is needed.

>
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>



-- 
H.J.

  reply	other threads:[~2022-07-28 16:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27  5:13 Alexandre Oliva
2022-07-27 14:31 ` H.J. Lu
2022-07-28  8:26   ` Alexandre Oliva
2022-07-28 16:31     ` H.J. Lu [this message]
2022-08-01 19:03       ` H.J. Lu
2022-08-01 19:15         ` Fangrui Song
2022-08-08 19:29         ` Alexandre Oliva

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='CAMe9rOpG17d173YKpKb88tCBe5HvQPUo5ZL_No=YyqpjcUvpLA@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=oliva@adacore.com \
    --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).