On Mon, Aug 1, 2022 at 12:05 PM H.J. Lu via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Thu, Jul 28, 2022 at 9:31 AM H.J. Lu <hjl.to...@gmail.com> wrote: > > > > On Thu, Jul 28, 2022 at 1:26 AM Alexandre Oliva <ol...@adacore.com> wrote: > > > > > > On Jul 27, 2022, "H.J. Lu" <hjl.to...@gmail.com> wrote: > > > > > > > On Tue, Jul 26, 2022 at 10:14 PM Alexandre Oliva <ol...@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. > > I think this fix should be applied to default_binds_local_p_3: > > if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp)) > || (!targetm.ifunc_ref_local_ok () > && TREE_CODE (exp) == FUNCTION_DECL > && cgraph_node::get (exp) > && cgraph_node::get (exp)->ifunc_resolver)) > return false; > > since the ifunc_resolver check won't work on aliases. > > -- > H.J.
A note if people are going to refactor default_binds_local_p_3 or add a new default_binds_local_p_* variant (unrelated to the ifunc problem): extern_protected_data == true is now legacy. For new variants we want all extern_protected_data == false.
