<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 28, 2017 at 2:03 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">First on what to do with -R: I am OK with it being an alias to --rpath<br>
or --just-symbols, but the magic of mapping to one or the other<br>
depending on the file type I think we should avoid.<br>
<br>
Since --just-symbols seems to be the less usual, I vote to keep -R<br>
mapped to --rpath.<br>
<span class=""><br>
<br>
<br>
> +void SymbolTable::<wbr>addAbsoluteOptional(StringRef Name, uint64_t Value) {<br>
> +  outs() << Name << "=" << Value << "\n";<br>
<br>
</span>Debug code.<br>
<span class=""><br>
> +  SymbolBody *B = find(Name);<br>
</span>> +  if (B->isUndefined())<br>
B can be null.<br>
<br>
Why is this optional? We are required to ignore a symbol if it is<br>
provided some other way? We are missing a test for that.<br></blockquote><div><br></div><div>Right, we don't need to make this optional. Chaned the code so that it adds absolute symbols for all defined symbols.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> Index: lld/ELF/InputFiles.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.h<br>
> +++ lld/ELF/InputFiles.h<br>
> @@ -59,6 +59,10 @@<br>
>  // Opens a given file.<br>
>  llvm::Optional<<wbr>MemoryBufferRef> readFile(StringRef Path);<br>
<span class="">><br>
> +// For --just-symbol.<br>
> +template <class ELFT><br>
</span>> +std::vector<std::pair<<wbr>StringRef, uint64_t>> readSymbols(MemoryBufferRef);<br>
<br>
This can be a file static, no?<br></blockquote><div><br></div><div>If we move it to Driver.cpp, yes, but I want to keep this in InputFiles.cpp because it reads an input file.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>  // The root class of input files.<br>
>  class InputFile {<br>
>  public:<br>
> Index: lld/ELF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.cpp<br>
> +++ lld/ELF/InputFiles.cpp<br>
> @@ -1049,6 +1049,38 @@<br>
<span class="">>    }<br>
>  }<br>
><br>
> +// Reads defined symbols from a given ELF file, and returns their<br>
> +// names and values. This is used for -just-symbols.<br>
> +template <class ELFT><br>
</span><span class="">> +std::vector<std::pair<<wbr>StringRef, uint64_t>><br>
> +elf::readSymbols(<wbr>MemoryBufferRef MB) {<br>
</span>> +  typedef typename ELFT::Shdr Elf_Shdr;<br>
> +  typedef typename ELFT::Sym Elf_Sym;<br>
> +  typedef typename ELFT::SymRange Elf_Sym_Range;<br>
> +<br>
> +  StringRef ObjName = MB.getBufferIdentifier();<br>
> +  ELFFile<ELFT> Obj = check(ELFFile<ELFT>::create(<wbr>MB.getBuffer()));<br>
> +  ArrayRef<Elf_Shdr> Sections = check(Obj.sections(), ObjName);<br>
> +<br>
> +  for (const Elf_Shdr &Sec : Sections) {<br>
> +    if (Sec.sh_type != SHT_SYMTAB)<br>
> +      continue;<br>
> +<br>
> +    Elf_Sym_Range Syms = check(Obj.symbols(&Sec), ObjName);<br>
> +    uint32_t FirstNonLocal = Sec.sh_info;<br>
> +    StringRef StringTable =<br>
> +        check(Obj.<wbr>getStringTableForSymtab(Sec, Sections), ObjName);<br>
> +<br>
> +    std::vector<std::pair<<wbr>StringRef, uint64_t>> Ret;<br>
> +    for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))<br>
> +      if (Sym.st_shndx != SHN_UNDEF)<br>
> +        Ret.emplace_back(check(Sym.<wbr>getName(StringTable), ObjName),<br>
> +                         Sym.st_value);<br>
<br>
Why return a vector instead of adding directly to the symbol table?<br>
Adding directly would make it simpler to handle other Elf_Sym fields if<br>
needed.</blockquote><div><br></div><div>Because I wanted to make this function side-effect-free for readability reasons. </div></div><br></div></div>