ppc64le: expected localentry:0 `pthread_condattr_destroy'

  On 07/27/2017 11:55 AM, Florian Weimer wrote:
> On 07/27/2017 06:04 AM, Alan Modra wrote:
>>>>> On 07/25/2017 09:11 PM, Josh Stone wrote:
>>>>>> ./bin/rustc: error while loading shared libraries:
>>>>>> ./lib/libstd-c3a1748e15265da7.so: expected localentry:0
>>>>>> `pthread_condattr_destroy'
>>
>> You will get this error if the link-time version of a function symbol
>> is seen as localentry:0 (ie. not needing a global entry point due to
>> not needing a valid r2 toc pointer), but the run-time version does.
>>
>> The most likely thing is that your library was linked against a stub
>> version of pthread_condattr_destroy.  Making the stub weak will
>> disable the generation of the optimized localentry:0 plt call code.
>> So will linking with -Wl,--no-plt-localentry
> 
> The common theme seems to be that this only affects symbols which live
> both in libpthread and libc.  Do you suggest that we have to make the
> implementation in libc a weak symbol?
> 
> Why isn't this a plain regression break ELF symbol interposition?
> 
> I filed a glibc bug for this:
> 
>   https://sourceware.org/bugzilla/show_bug.cgi?id=21847
> 
> I think we need to sort this out before the release.  That does not mean
> we have to fix it, we just need a better understanding what is going on.

I injected -Wl,--no-plt-localentry into the build, using this patch:


But the issue persists afterwards.  I'm afraid we'll need a real fix.

Thanks,
Florian
  

On Thu, Jul 27, 2017 at 06:34:33PM +0200, Florian Weimer wrote:
> On 07/27/2017 11:55 AM, Florian Weimer wrote:
> > On 07/27/2017 06:04 AM, Alan Modra wrote:
> >>>>> On 07/25/2017 09:11 PM, Josh Stone wrote:
> >>>>>> ./bin/rustc: error while loading shared libraries:
> >>>>>> ./lib/libstd-c3a1748e15265da7.so: expected localentry:0
> >>>>>> `pthread_condattr_destroy'
> >>
> >> You will get this error if the link-time version of a function symbol
> >> is seen as localentry:0 (ie. not needing a global entry point due to
> >> not needing a valid r2 toc pointer), but the run-time version does.
> >>
> >> The most likely thing is that your library was linked against a stub
> >> version of pthread_condattr_destroy.  Making the stub weak will
> >> disable the generation of the optimized localentry:0 plt call code.
> >> So will linking with -Wl,--no-plt-localentry
> > 
> > The common theme seems to be that this only affects symbols which live
> > both in libpthread and libc.  Do you suggest that we have to make the
> > implementation in libc a weak symbol?

Right, the stub symbols in libpthread.so should be make weak.
See my comment in pr21847.  We're getting a mismatch between ld symbol
resolution and ld.so resolution.

> > Why isn't this a plain regression break ELF symbol interposition?
> > 
> > I filed a glibc bug for this:
> > 
> >   https://sourceware.org/bugzilla/show_bug.cgi?id=21847
> > 
> > I think we need to sort this out before the release.  That does not mean
> > we have to fix it, we just need a better understanding what is going on.
> 
> I injected -Wl,--no-plt-localentry into the build, using this patch:
> 
> --- a/sysdeps/powerpc/powerpc64le/Makefile
> +++ b/sysdeps/powerpc/powerpc64le/Makefile
> @@ -6,6 +6,10 @@
>  # linked executables, forcing to link the loader after libgcc link.
>  f128-loader-link = $(as-needed) $(elf-objpfx)ld.so $(no-as-needed)
> 
> +sysdep-LDFLAGS += -Wl,--no-plt-localentry
> +# Linking libc_pic.os ignores sysdep-LDFLAGS.
> +LDFLAGS-c_pic.os += -Wl,--no-plt-localentry
> +
>  ifeq ($(subdir),math)
>  # sqrtf128 requires emulation before POWER9.
>  CPPFLAGS += -I../soft-fp
> 
> But the issue persists afterwards.  I'm afraid we'll need a real fix.

No, you misunderstood my comment about --no-plt-localentry.  The
*user* library would need to be built with that option, which isn't a
good solution.
  

On 07/28/2017 12:07 PM, Alan Modra wrote:
> No, you misunderstood my comment about --no-plt-localentry.  The
> *user* library would need to be built with that option, which isn't a
> good solution.

Could you clarify how this optimization works?  Is the following
description accurate?

At static link time, when processing a relocation to a function, the
static link editor looks at the implementation of the called function
(which can live in a different DSO).  If the current implementation does
not use the TOC register, the caller is adjusted to call an optimized
PLT stub which does not set the TOC.  Otherwise, a full PLT stub which
sets the TOC is used.  If the link edit chose to apply the optimization,
there is no way to go back, and future implementations of the called
function must never require a TOC pointer.  The dynamic linker cannot
recover and thus bails out with the localentry error message.

Thanks,
Florian
  

On Fri, Jul 28, 2017 at 02:39:13PM +0200, Florian Weimer wrote:
> On 07/28/2017 12:07 PM, Alan Modra wrote:
> > No, you misunderstood my comment about --no-plt-localentry.  The
> > *user* library would need to be built with that option, which isn't a
> > good solution.
> 
> Could you clarify how this optimization works?  Is the following
> description accurate?
> 
> At static link time, when processing a relocation to a function, the
> static link editor looks at the implementation of the called function
> (which can live in a different DSO).  If the current implementation does
> not use the TOC register, the caller is adjusted to call an optimized
> PLT stub which does not set the TOC.  Otherwise, a full PLT stub which
> sets the TOC is used.  If the link edit chose to apply the optimization,
> there is no way to go back, and future implementations of the called
> function must never require a TOC pointer.  The dynamic linker cannot
> recover and thus bails out with the localentry error message.

Yes, that's correct.  If the linker doesn't see a definition, or if
the definition is weak, then the optimization does not apply.
  

On Jul 28 2017, Alan Modra <amodra@gmail.com> wrote:

> On Fri, Jul 28, 2017 at 02:39:13PM +0200, Florian Weimer wrote:
>> At static link time, when processing a relocation to a function, the
>> static link editor looks at the implementation of the called function
>> (which can live in a different DSO).  If the current implementation does
>> not use the TOC register, the caller is adjusted to call an optimized
>> PLT stub which does not set the TOC.  Otherwise, a full PLT stub which
>> sets the TOC is used.  If the link edit chose to apply the optimization,
>> there is no way to go back, and future implementations of the called
>> function must never require a TOC pointer.  The dynamic linker cannot
>> recover and thus bails out with the localentry error message.
>
> Yes, that's correct.  If the linker doesn't see a definition, or if
> the definition is weak, then the optimization does not apply.

That looks like a design bug.  The link-time object will almost never
exactly match the run-time object.

Andreas.
  

On 07/28/2017 02:52 PM, Alan Modra wrote:
> On Fri, Jul 28, 2017 at 02:39:13PM +0200, Florian Weimer wrote:
>> On 07/28/2017 12:07 PM, Alan Modra wrote:
>>> No, you misunderstood my comment about --no-plt-localentry.  The
>>> *user* library would need to be built with that option, which isn't a
>>> good solution.
>>
>> Could you clarify how this optimization works?  Is the following
>> description accurate?
>>
>> At static link time, when processing a relocation to a function, the
>> static link editor looks at the implementation of the called function
>> (which can live in a different DSO).  If the current implementation does
>> not use the TOC register, the caller is adjusted to call an optimized
>> PLT stub which does not set the TOC.  Otherwise, a full PLT stub which
>> sets the TOC is used.  If the link edit chose to apply the optimization,
>> there is no way to go back, and future implementations of the called
>> function must never require a TOC pointer.  The dynamic linker cannot
>> recover and thus bails out with the localentry error message.
> 
> Yes, that's correct.  If the linker doesn't see a definition, or if
> the definition is weak, then the optimization does not apply.

Ugh.

This absolutely has to be disabled by default.  No one is prepared for
an ABI change because a global variable access has been added to a
function (or the compiler has decided to inline a function with a global
variable access).

In fact, I'm *extremely* reluctant to ship a distribution which as
support for this in the link editor and dynamic linker, even as an
optional feature, because developers will see this option and use it,
without realizing the full consequences (that they lose ABI forward
compatibility).

The optimization is only possible if it is source-directed, that is, the
compiler can take measures to preserve ABI in the presence of
implementation changes.  This would have to be encoded in the DSO (as a
promise “this function will never use the TOC pointer”), and static link
editor could pick up that flag and skip setting that TOC.

But we can't do that automatically just because the current
implementation has some convenient property.  Nobody knows if future
library upgrades will preserve this property.

Thanks,
Florian