[PATCH] D39348: Implement --just-symbols.

[Rui Ueyama via llvm-commits]

On Tue, Nov 28, 2017 at 2:03 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> First on what to do with -R: I am OK with it being an alias to --rpath
> or --just-symbols, but the magic of mapping to one or the other
> depending on the file type I think we should avoid.
>
> Since --just-symbols seems to be the less usual, I vote to keep -R
> mapped to --rpath.
>
>
>
> > +void SymbolTable::addAbsoluteOptional(StringRef Name, uint64_t Value) {
> > +  outs() << Name << "=" << Value << "\n";
>
> Debug code.
>
> > +  SymbolBody *B = find(Name);
> > +  if (B->isUndefined())
> B can be null.
>
> Why is this optional? We are required to ignore a symbol if it is
> provided some other way? We are missing a test for that.
>

Right, we don't need to make this optional. Chaned the code so that it adds
absolute symbols for all defined symbols.


> > Index: lld/ELF/InputFiles.h
> > ===================================================================
> > --- lld/ELF/InputFiles.h
> > +++ lld/ELF/InputFiles.h
> > @@ -59,6 +59,10 @@
> >  // Opens a given file.
> >  llvm::Optional<MemoryBufferRef> readFile(StringRef Path);
> >
> > +// For --just-symbol.
> > +template <class ELFT>
> > +std::vector<std::pair<StringRef, uint64_t>>
> readSymbols(MemoryBufferRef);
>
> This can be a file static, no?
>

If we move it to Driver.cpp, yes, but I want to keep this in InputFiles.cpp
because it reads an input file.


> >  // The root class of input files.
> >  class InputFile {
> >  public:
> > Index: lld/ELF/InputFiles.cpp
> > ===================================================================
> > --- lld/ELF/InputFiles.cpp
> > +++ lld/ELF/InputFiles.cpp
> > @@ -1049,6 +1049,38 @@
> >    }
> >  }
> >
> > +// Reads defined symbols from a given ELF file, and returns their
> > +// names and values. This is used for -just-symbols.
> > +template <class ELFT>
> > +std::vector<std::pair<StringRef, uint64_t>>
> > +elf::readSymbols(MemoryBufferRef MB) {
> > +  typedef typename ELFT::Shdr Elf_Shdr;
> > +  typedef typename ELFT::Sym Elf_Sym;
> > +  typedef typename ELFT::SymRange Elf_Sym_Range;
> > +
> > +  StringRef ObjName = MB.getBufferIdentifier();
> > +  ELFFile<ELFT> Obj = check(ELFFile<ELFT>::create(MB.getBuffer()));
> > +  ArrayRef<Elf_Shdr> Sections = check(Obj.sections(), ObjName);
> > +
> > +  for (const Elf_Shdr &Sec : Sections) {
> > +    if (Sec.sh_type != SHT_SYMTAB)
> > +      continue;
> > +
> > +    Elf_Sym_Range Syms = check(Obj.symbols(&Sec), ObjName);
> > +    uint32_t FirstNonLocal = Sec.sh_info;
> > +    StringRef StringTable =
> > +        check(Obj.getStringTableForSymtab(Sec, Sections), ObjName);
> > +
> > +    std::vector<std::pair<StringRef, uint64_t>> Ret;
> > +    for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))
> > +      if (Sym.st_shndx != SHN_UNDEF)
> > +        Ret.emplace_back(check(Sym.getName(StringTable), ObjName),
> > +                         Sym.st_value);
>
> Why return a vector instead of adding directly to the symbol table?
> Adding directly would make it simpler to handle other Elf_Sym fields if
> needed.


Because I wanted to make this function side-effect-free for readability
reasons.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171130/af59d729/attachment.html>