[lld] r250311 - Handle dynamic relocs to weak undefined when possible.
Hal Finkel via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 15 12:01:26 PDT 2015
----- Original Message -----
> From: "Hal Finkel via llvm-commits" <llvm-commits at lists.llvm.org>
> To: "Rafael Espindola" <rafael.espindola at gmail.com>
> Cc: llvm-commits at lists.llvm.org
> Sent: Thursday, October 15, 2015 1:31:28 PM
> Subject: Re: [lld] r250311 - Handle dynamic relocs to weak undefined when possible.
>
> ----- Original Message -----
> > From: "Rafael Espindola via llvm-commits"
> > <llvm-commits at lists.llvm.org>
> > To: llvm-commits at lists.llvm.org
> > Sent: Wednesday, October 14, 2015 1:42:17 PM
> > Subject: [lld] r250311 - Handle dynamic relocs to weak undefined
> > when possible.
> >
> > Author: rafael
> > Date: Wed Oct 14 13:42:16 2015
> > New Revision: 250311
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=250311&view=rev
> > Log:
> > Handle dynamic relocs to weak undefined when possible.
> >
> > Added:
> > lld/trunk/test/elf2/dynamic-reloc-weak.s
> > Modified:
> > lld/trunk/ELF/OutputSections.cpp
> > lld/trunk/ELF/OutputSections.h
> > lld/trunk/ELF/Writer.cpp
> >
> > Modified: lld/trunk/ELF/OutputSections.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=250311&r1=250310&r2=250311&view=diff
> > ==============================================================================
> > --- lld/trunk/ELF/OutputSections.cpp (original)
> > +++ lld/trunk/ELF/OutputSections.cpp Wed Oct 14 13:42:16 2015
> > @@ -52,7 +52,7 @@ template <class ELFT> void GotSection<EL
> > for (const SymbolBody *B : Entries) {
> > uint8_t *Entry = Buf;
> > Buf += sizeof(uintX_t);
> > - if (canBePreempted(B))
> > + if (canBePreempted(B, false))
> > continue; // The dynamic linker will take care of it.
> > uintX_t VA = getSymVA<ELFT>(*B);
> > write<uintX_t, ELFT::TargetEndianness, sizeof(uintX_t)>(Entry,
> > VA);
> > @@ -122,7 +122,8 @@ template <class ELFT> void RelocationSec
> >
> > uint32_t Type = RI.getType(IsMips64EL);
> >
> > - bool CanBePreempted = canBePreempted(Body);
> > + bool NeedsGot = Body && Target->relocNeedsGot(Type, *Body);
> > + bool CanBePreempted = canBePreempted(Body, NeedsGot);
> > uintX_t Addend = 0;
> > if (!CanBePreempted) {
> > if (IsRela) {
>
> This does not seem to do the right thing for shared libraries, at
> least not on PPC64. When creating a shared library on PPC64, the
> linker includes crti.o as an input, and that has a R_PPC64_ADDR64
> relocation to __gmon_start__ (used to check the value of
> __gmon_start__ so that it can be only conditionally called). But, in
> the common case, it is weak and undefined (and being a direct
> address, does not require a .got entry), so canBePreempted returns
> false.
>
> But, then when creating a shared library, we add a R_PPC64_RELATIVE
> relocation for this R_PPC64_ADDR64 __gmon_start__ field, using 0 as
> the addend, and so ld.so ends up filling this field with the base
> load address (definitely non-zero), and the resulting library
> understandably segfaults during initialization.
>
> Thanks again,
> Hal
>
> > @@ -134,7 +135,7 @@ template <class ELFT> void RelocationSec
> > P->setSymbolAndType(0, Target->getRelativeReloc(),
> > IsMips64EL);
> > }
> >
> > - if (Body && Target->relocNeedsGot(Type, *Body)) {
> > + if (NeedsGot) {
> > P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
> > if (CanBePreempted)
> > P->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
> > @@ -458,13 +459,34 @@ lld::elf2::getLocalRelTarget(const Objec
> >
> > // Returns true if a symbol can be replaced at load-time by a
> > symbol
> > // with the same name defined in other ELF executable or DSO.
> > -bool lld::elf2::canBePreempted(const SymbolBody *Body) {
> > +bool lld::elf2::canBePreempted(const SymbolBody *Body, bool
> > NeedsGot) {
> > if (!Body)
> > return false; // Body is a local symbol.
> > if (Body->isShared())
> > return true;
> > - if (Body->isUndefined() && !Body->isWeak())
> > - return true;
> > +
> > + if (Body->isUndefined()) {
> > + if (!Body->isWeak())
> > + return true;
> > +
> > + // This is an horrible corner case. Ideally we would like to
> > say
> > that any
> > + // undefined symbol can be preempted so that the dynamic
> > linker
> > has a
> > + // chance of finding it at runtime.
> > + //
> > + // The problem is that the code sequence used to test for weak
> > undef
> > + // functions looks like
> > + // if (func) func()
> > + // If the code is -fPIC the first reference is a load from the
> > got and
> > + // everything works.
> > + // If the code is not -fPIC there is no reasonable way to
> > solve
> > it:
> > + // * A relocation writing to the text segment will fail (it is
> > ro).
> > + // * A copy relocation doesn't work for functions.
> > + // * The trick of using a plt entry as the address would fail
> > here since
> > + // the plt entry would have a non zero address.
> > + // Since we cannot do anything better, we just resolve the
> > symbol to 0 and
> > + // don't produce a dynamic relocation.
> > + return NeedsGot;
> > + }
> > if (!Config->Shared)
> > return false;
> > return Body->getMostConstrainingVisibility() == STV_DEFAULT;
> >
> > Modified: lld/trunk/ELF/OutputSections.h
> > URL:
> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=250311&r1=250310&r2=250311&view=diff
> > ==============================================================================
> > --- lld/trunk/ELF/OutputSections.h (original)
> > +++ lld/trunk/ELF/OutputSections.h Wed Oct 14 13:42:16 2015
> > @@ -39,7 +39,7 @@ template <class ELFT>
> > typename llvm::object::ELFFile<ELFT>::uintX_t
> > getLocalRelTarget(const ObjectFile<ELFT> &File,
> > const typename
> > llvm::object::ELFFile<ELFT>::Elf_Rel &Sym);
> > -bool canBePreempted(const SymbolBody *Body);
> > +bool canBePreempted(const SymbolBody *Body, bool NeedsGot);
> > template <class ELFT> bool includeInSymtab(const SymbolBody &B);
> >
> > bool includeInDynamicSymtab(const SymbolBody &B);
> >
> > Modified: lld/trunk/ELF/Writer.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=250311&r1=250310&r2=250311&view=diff
> > ==============================================================================
> > --- lld/trunk/ELF/Writer.cpp (original)
> > +++ lld/trunk/ELF/Writer.cpp Wed Oct 14 13:42:16 2015
> > @@ -191,20 +191,22 @@ void Writer<ELFT>::scanRelocs(
> >
> > if (Body)
> > Body = Body->repl();
> > + bool NeedsGot = false;
> > if (Body) {
> > if (Target->relocNeedsPlt(Type, *Body)) {
> > if (Body->isInPlt())
> > continue;
> > Out<ELFT>::Plt->addEntry(Body);
> > }
> > - if (Target->relocNeedsGot(Type, *Body)) {
> > + NeedsGot = Target->relocNeedsGot(Type, *Body);
> > + if (NeedsGot) {
> > if (Body->isInGot())
> > continue;
> > Out<ELFT>::Got->addEntry(Body);
> > }
> > }
> >
> > - bool CBP = canBePreempted(Body);
> > + bool CBP = canBePreempted(Body, NeedsGot);
> > if (!CBP && (!Config->Shared || Target->isRelRelative(Type)))
> > continue;
Or, more accurately, maybe the logic here is missing something (once you're writing the relocation section, where I commented above, I imagine it is too late to decide you don't want to have any relocation at all).
Thanks again,
Hal
> > if (CBP)
> >
> > Added: lld/trunk/test/elf2/dynamic-reloc-weak.s
> > URL:
> > http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/dynamic-reloc-weak.s?rev=250311&view=auto
> > ==============================================================================
> > --- lld/trunk/test/elf2/dynamic-reloc-weak.s (added)
> > +++ lld/trunk/test/elf2/dynamic-reloc-weak.s Wed Oct 14 13:42:16
> > 2015
> > @@ -0,0 +1,31 @@
> > +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o
> > %t.o
> > +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux
> > %p/Inputs/shared.s -o %t2.o
> > +// RUN: ld.lld2 -shared %t2.o -o %t2.so
> > +// RUN: ld.lld2 %t.o %t2.so -o %t
> > +// RUN: llvm-readobj -r %t | FileCheck %s
> > +// REQUIRES: x86
> > +
> > + .globl _start
> > +_start:
> > + .type sym1, at function
> > + .weak sym1
> > + .long sym1 at gotpcrel
> > +
> > + .type sym2, at function
> > + .weak sym2
> > + .long sym2 at plt
> > +
> > + .type sym3, at function
> > + .weak sym3
> > + .quad sym3
> > +
> > +// Both gold and bfd ld will produce a relocation for sym1 and
> > sym2
> > only. That
> > +// That seems odd. If the dynamic linker must get a chance to
> > resolve sym1
> > +// and sym2, that should also be the case for sym3.
> > +
> > +// CHECK: Relocations [
> > +// CHECK-NEXT: Section ({{.*}}) .rela.dyn {
> > +// CHECK-NEXT: 0x{{.*}} R_X86_64_GLOB_DAT sym1 0x0
> > +// CHECK-NEXT: 0x{{.*}} R_X86_64_GLOB_DAT sym2 0x0
> > +// CHECK-NEXT: }
> > +// CHECK-NEXT: ]
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list