[lld] r300007 - [lld] Keep full library path in DT_NEEDED.

Mon Apr 24 13:54:47 PDT 2017

I am giving that a try.

Cheers,
Rafael

On 24 April 2017 at 16:35, Rui Ueyama <ruiu at google.com> wrote:
> On Mon, Apr 24, 2017 at 1:33 PM, Peter Collingbourne <peter at pcc.me.uk>
> wrote:
>>
>>
>>
>> On Mon, Apr 24, 2017 at 1:28 PM, Rui Ueyama via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>>
>>> On Mon, Apr 24, 2017 at 1:22 PM, Rafael Espíndola via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>> On 11 April 2017 at 19:46, Evgeniy Stepanov via llvm-commits
>>>> <llvm-commits at lists.llvm.org> wrote:
>>>> > Author: eugenis
>>>> > Date: Tue Apr 11 18:46:58 2017
>>>> > New Revision: 300007
>>>> >
>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=300007&view=rev
>>>> > Log:
>>>> > [lld] Keep full library path in DT_NEEDED.
>>>> >
>>>> > Fixes PR32572.
>>>> >
>>>> > When
>>>> >     (a) a library has no soname
>>>> > and (b) library is given on the command line with path (and not
>>>> > through -L/-l flags)
>>>> > DT_NEEDED entry for such library keeps the path as given.
>>>> >
>>>> > This behavior is consistent with gold and bfd, and is used in
>>>> > compiler-rt test suite.
>>>> >
>>>> > Added:
>>>> >     lld/trunk/test/ELF/no-soname.s
>>>> > Modified:
>>>> >     lld/trunk/ELF/Driver.cpp
>>>> >     lld/trunk/ELF/Driver.h
>>>> >     lld/trunk/ELF/InputFiles.cpp
>>>> >     lld/trunk/ELF/InputFiles.h
>>>> >     lld/trunk/ELF/ScriptParser.cpp
>>>> >     lld/trunk/test/ELF/as-needed-no-reloc.s
>>>> >     lld/trunk/test/ELF/relro-omagic.s
>>>> >
>>>> > Modified: lld/trunk/ELF/Driver.cpp
>>>> > URL:
>>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=300007&r1=300006&r2=300007&view=diff
>>>> >
>>>> > ==============================================================================
>>>> > --- lld/trunk/ELF/Driver.cpp (original)
>>>> > +++ lld/trunk/ELF/Driver.cpp Tue Apr 11 18:46:58 2017
>>>> > @@ -153,7 +153,7 @@ LinkerDriver::getArchiveMembers(MemoryBu
>>>> >
>>>> >  // Opens and parses a file. Path has to be resolved already.
>>>> >  // Newly created memory buffers are owned by this driver.
>>>> > -void LinkerDriver::addFile(StringRef Path) {
>>>> > +void LinkerDriver::addFile(StringRef Path, bool WithLOption) {
>>>> >    using namespace sys::fs;
>>>> >
>>>> >    Optional<MemoryBufferRef> Buffer = readFile(Path);
>>>> > @@ -184,6 +184,11 @@ void LinkerDriver::addFile(StringRef Pat
>>>> >        return;
>>>> >      }
>>>> >      Files.push_back(createSharedFile(MBRef));
>>>> > +    // If the library is found at an explicitly given path use the
>>>> > entire path
>>>> > +    // as he default soname. Such libraries should not require RPATH
>>>> > or
>>>> > +    // LD_LIBRARY_PATH to run.
>>>> > +    Files.back()->DefaultSoName =
>>>> > +        WithLOption ? sys::path::filename(Path) : Path;
>>>> >      return;
>>>> >    default:
>>>> >      if (InLib)
>>>> > @@ -196,7 +201,7 @@ void LinkerDriver::addFile(StringRef Pat
>>>> >  // Add a given library by searching it from input search paths.
>>>> >  void LinkerDriver::addLibrary(StringRef Name) {
>>>> >    if (Optional<std::string> Path = searchLibrary(Name))
>>>> > -    addFile(*Path);
>>>> > +    addFile(*Path, /*WithLOption=*/true);
>>>> >    else
>>>> >      error("unable to find library -l" + Name);
>>>> >  }
>>>> > @@ -762,7 +767,7 @@ void LinkerDriver::createFiles(opt::Inpu
>>>> >        addLibrary(Arg->getValue());
>>>> >        break;
>>>> >      case OPT_INPUT:
>>>> > -      addFile(Arg->getValue());
>>>> > +      addFile(Arg->getValue(), /*WithLOption=*/false);
>>>> >        break;
>>>> >      case OPT_alias_script_T:
>>>> >      case OPT_script:
>>>> >
>>>> > Modified: lld/trunk/ELF/Driver.h
>>>> > URL:
>>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.h?rev=300007&r1=300006&r2=300007&view=diff
>>>> >
>>>> > ==============================================================================
>>>> > --- lld/trunk/ELF/Driver.h (original)
>>>> > +++ lld/trunk/ELF/Driver.h Tue Apr 11 18:46:58 2017
>>>> > @@ -27,7 +27,7 @@ extern class LinkerDriver *Driver;
>>>> >  class LinkerDriver {
>>>> >  public:
>>>> >    void main(ArrayRef<const char *> Args, bool CanExitEarly);
>>>> > -  void addFile(StringRef Path);
>>>> > +  void addFile(StringRef Path, bool WithLOption);
>>>> >    void addLibrary(StringRef Name);
>>>> >
>>>> >  private:
>>>> >
>>>> > Modified: lld/trunk/ELF/InputFiles.cpp
>>>> > URL:
>>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=300007&r1=300006&r2=300007&view=diff
>>>> >
>>>> > ==============================================================================
>>>> > --- lld/trunk/ELF/InputFiles.cpp (original)
>>>> > +++ lld/trunk/ELF/InputFiles.cpp Tue Apr 11 18:46:58 2017
>>>> > @@ -661,7 +661,7 @@ template <class ELFT> void SharedFile<EL
>>>> >    // DSOs are identified by soname, and they usually contain
>>>> >    // DT_SONAME tag in their header. But if they are missing,
>>>> >    // filenames are used as default sonames.
>>>> > -  SoName = sys::path::filename(this->getName());
>>>> > +  SoName = this->DefaultSoName;
>>>> >
>>>> >    if (!DynamicSec)
>>>> >      return;
>>>> >
>>>> > Modified: lld/trunk/ELF/InputFiles.h
>>>> > URL:
>>>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=300007&r1=300006&r2=300007&view=diff
>>>> >
>>>> > ==============================================================================
>>>> > --- lld/trunk/ELF/InputFiles.h (original)
>>>> > +++ lld/trunk/ELF/InputFiles.h Tue Apr 11 18:46:58 2017
>>>> > @@ -93,6 +93,10 @@ public:
>>>> >    uint16_t EMachine = llvm::ELF::EM_NONE;
>>>> >    uint8_t OSABI = 0;
>>>> >
>>>> > +  // For SharedKind inputs, the string to use in DT_NEEDED when the
>>>> > library
>>>> > +  // has no soname.
>>>> > +  std::string DefaultSoName;
>>>>
>>>> Why do you need this to be in the base class?
>>>
>>>
>>> I found this odd too, but it turned out that SharedFile is a template
>>> class, and in this context we don't know about ELFT, so it's not easy to
>>> cast a file object to SharedFile<ELFT> here.
>>
>>
>> Though couldn't the default soname just be an argument to
>> createSharedFile?
>
>
> I don't know why I didn't notice, but it seems it can.