[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