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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 28 14:03:43 PST 2017


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.

> 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?


>  // 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.

Cheers,
Rafael


More information about the llvm-commits mailing list