[ELF] - Treat --just-symbols file as an input.

[grimar]

Summary

This is PR36737.

Patch changes --just-symbols implementation so that we
count such files as inputs now.

Reimplementation also grants additional benefits. Now
--just-symbols file is added to Files and ObjectFiles just as
a regular input file.

With that, it not only allows our code to infer the ELF target automatically
(without additional changes) but also let the rest of LLD code to do its job too.
For example, I added test case showing we are able now to report symbols from
just symbols file with --cref.

[grimar]

• Moved JustSymbols flag to ObjFile.
• Cosmetic changes.

[• espindola]

LGTM

ELF/InputFiles.cpp

293

I think you can pass just "this" to CHECK instead of "toString(this)".

There are a few more cases in this function.

[ruiu]

I think I implemented the exact same thing in my patch already which I haven't submitted, and I think mine is better than this patch. I don't like to mix adding a feature for JustSymbols to ObjFile class. Because JustSymbols is so minor, I don't want to mix code for that feature with the regular code path.

ELF/Driver.cpp

176

You don't need to call this function to add a new file to Files. Let's keep it simple. Just call createObjectFile and directly add its return value to Files. Basically, you should avoid adding this kind of ad-hoc new parameter to an existing function.

[ruiu]

I prefer https://reviews.llvm.org/D42025 over this patch in case we need this feature, though I'm not sure if we need it in the first place. I'm not against adding this, but I'd like to know what's a use case of this feature first.

[• espindola]

In D44516#1039744, @ruiu wrote:

I prefer https://reviews.llvm.org/D42025 over this patch in case we need this feature, though I'm not sure if we need it in the first place. I'm not against adding this, but I'd like to know what's a use case of this feature first.

During the initial implementation Chih-Mao Chen wrote:

                                                                                                                                                                                                                                                                                                 
It can be used to produce an object file containing the offset of symbols in vdso:                                                                                                                                                                                                               
https://github.com/riscv/riscv-linux/blob/ae64f9bd1d3621b5e60d7363bc20afb46aede215/arch/riscv/kernel/vdso/Makefile#L41

Which also points out that we should probably support .o and .so too (as we do now).

[grimar]

In D44516#1039744, @ruiu wrote:

I prefer https://reviews.llvm.org/D42025 over this patch in case we need this feature, though I'm not sure if we need it in the first place. I'm not against adding this, but I'd like to know what's a use case of this feature first.

D42025 looks good for me too. Seems it should work with just-symbols-cref.s from this patch out of the box, I also suggest to include it.

[grimar]

D42025 was landed instead.

ELF/Driver.h

 namespace lld {
 namespace elf {
 
 extern class LinkerDriver *Driver;
 
 class LinkerDriver {
 public:
   void main(ArrayRef<const char *> Args);
-  void addFile(StringRef Path, bool WithLOption);
+  void addFile(StringRef Path, bool WithLOption, bool JustSymbols);
   void addLibrary(StringRef Name);
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template <class ELFT> void link(llvm::opt::InputArgList &Args);
 

ELF/Driver.cpp

std::vector<std::pair<MemoryBufferRef, uint64_t>> static getArchiveMembers(
   // Take ownership of memory buffers created for members of thin archives.
   for (std::unique_ptr<MemoryBuffer> &MB : File->takeThinBuffers())
     make<std::unique_ptr<MemoryBuffer>>(std::move(MB));
 
   return V;
 }
 
 // Opens a file and create a file object. Path has to be resolved already.
-void LinkerDriver::addFile(StringRef Path, bool WithLOption) {
+void LinkerDriver::addFile(StringRef Path, bool WithLOption, bool JustSymbols) {
   using namespace sys::fs;
 
   Optional<MemoryBufferRef> Buffer = readFile(Path);
   if (!Buffer.hasValue())
     return;
   MemoryBufferRef MBRef = *Buffer;
 
   if (InBinary) {
     Files.push_back(make<BinaryFile>(MBRef));
     return;
   }
 
   switch (identify_magic(MBRef.getBuffer())) {
+  case file_magic::elf_executable: {
+    // We allow executable files as --just-symbols inputs.
+    Files.push_back(createObjectFile(MBRef, /*ArchiveName=*/"",
+                                     /*OffsetInArchive=*/0,
+                                     /*JustSymbols*/ true));
+    if (!JustSymbols)
+      error(Path + ": is an executable ELF file");
+    return;
+  }
   case file_magic::unknown:
     readLinkerScript(MBRef);
     return;
   case file_magic::archive: {
     // Handle -whole-archive.
     if (InWholeArchive) {
       for (const auto &P : getArchiveMembers(MBRef))
         Files.push_back(createObjectFile(P.first, Path, P.second));
void LinkerDriver::addFile(StringRef Path, bool WithLOption, bool JustSymbols) {
   default:
     error(Path + ": unknown file type");
   }
 }
 
 // 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, /*WithLOption=*/true);
+    addFile(*Path, /*WithLOption=*/true, /*JustSymbols=*/false);
   else
     error("unable to find library -l" + Name);
 }
 
 // This function is called on startup. We need this for LTO since
 // LTO calls LLVM functions to compile bitcode files to native code.
 // Technically this can be delayed until we read bitcode files, but
 // we don't bother to do lazily because the initialization is fast.
static bool getBinaryOption(StringRef S) {
   error("unknown -format value: " + S +
         " (supported formats: elf, default, binary)");
   return false;
 }
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
     switch (Arg->getOption().getUnaliasedOption().getID()) {
+    case OPT_just_symbols:
+      addFile(Arg->getValue(), /*WithLOption=*/false, /*JustSymbols=*/true);
+      break;
     case OPT_library:
       addLibrary(Arg->getValue());
       break;
     case OPT_INPUT:
-      addFile(Arg->getValue(), /*WithLOption=*/false);
+      addFile(Arg->getValue(), /*WithLOption=*/false, /*JustSymbols=*/false);
       break;
     case OPT_defsym: {
       StringRef From;
       StringRef To;
       std::tie(From, To) = StringRef(Arg->getValue()).split('=');
       readDefsym(From, MemoryBufferRef(To, "-defsym"));
       break;
     }
template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   // are undefined symbols for them, so we add these to trigger that logic.
   for (StringRef Sym : Script->ReferencedSymbols)
     Symtab->addUndefined<ELFT>(Sym);
 
   // Handle the `--undefined <sym>` options.
   for (StringRef S : Config->Undefined)
     Symtab->fetchIfLazy<ELFT>(S);
 
-  // Handle the --just-symbols option. This may add absolute symbols
-  // to the symbol table.
-  for (auto *Arg : Args.filtered(OPT_just_symbols))
-    if (Optional<MemoryBufferRef> MB = readFile(Arg->getValue()))
-      readJustSymbolsFile<ELFT>(*MB);
 
   // If an entry symbol is in a static archive, pull out that file now
   // to complete the symbol table. After this, no new names except a
   // few linker-synthesized ones will be added to the symbol table.
   Symtab->fetchIfLazy<ELFT>(Config->Entry);
 
   // Return if there were name resolution errors.
   if (errorCount())

ELF/InputFiles.h

template <class ELFT> class ObjFile : public ELFFileBase<ELFT> {
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Word Elf_Word;
 
   StringRef getShtGroupSignature(ArrayRef<Elf_Shdr> Sections,
                                  const Elf_Shdr &Sec);
   ArrayRef<Elf_Word> getShtGroupEntries(const Elf_Shdr &Sec);
 
+  void readJustSymbols();
+
 public:
   static bool classof(const InputFile *F) { return F->kind() == Base::ObjKind; }
 
   ArrayRef<Symbol *> getLocalSymbols();
 
-  ObjFile(MemoryBufferRef M, StringRef ArchiveName);
+  ObjFile(MemoryBufferRef M, StringRef ArchiveName, bool JustSymbols);
   void parse(llvm::DenseSet<llvm::CachedHashStringRef> &ComdatGroups);
 
   Symbol &getSymbol(uint32_t SymbolIndex) const {
     if (SymbolIndex >= this->Symbols.size())
       fatal(toString(this) + ": invalid symbol index");
     return *this->Symbols[SymbolIndex];
   }
 
private:
 
   // Debugging information to retrieve source file and line for error
   // reporting. Linker may find reasonable number of errors in a
   // single object file, so we cache debugging information in order to
   // parse it only once for each object file we link.
   std::unique_ptr<llvm::DWARFDebugLine> DwarfLine;
   llvm::DenseMap<StringRef, std::pair<unsigned, unsigned>> VariableLoc;
   llvm::once_flag InitDwarfLine;
+
+  // True if it is a -just-symbols file.
+  bool JustSymbols;
 };
 
 // LazyObjFile is analogous to ArchiveFile in the sense that
 // the file contains lazy symbols. The difference is that
 // LazyObjFile wraps a single file instead of multiple files.
 //
 // This class is used for --start-lib and --end-lib options which
 // instruct the linker to link object files between them with the

 class BinaryFile : public InputFile {
 public:
   explicit BinaryFile(MemoryBufferRef M) : InputFile(BinaryKind, M) {}
   static bool classof(const InputFile *F) { return F->kind() == BinaryKind; }
   void parse();
 };
 
 InputFile *createObjectFile(MemoryBufferRef MB, StringRef ArchiveName = "",
-                            uint64_t OffsetInArchive = 0);
+                            uint64_t OffsetInArchive = 0,
+                            bool JustSymbols = false);
 InputFile *createSharedFile(MemoryBufferRef MB, StringRef DefaultSoName);
 
-// For --just-symbols
-template <class ELFT> void readJustSymbolsFile(MemoryBufferRef MB);
-
 extern std::vector<BinaryFile *> BinaryFiles;
 extern std::vector<BitcodeFile *> BitcodeFiles;
 extern std::vector<InputFile *> ObjectFiles;
 extern std::vector<InputFile *> SharedFiles;
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/InputFiles.cpp

void ELFFileBase<ELFT>::initSymtab(ArrayRef<Elf_Shdr> Sections,
   if (FirstNonLocal == 0 || FirstNonLocal > ELFSyms.size())
     fatal(toString(this) + ": invalid sh_info in symbol table");
 
   StringTable =
       CHECK(getObj().getStringTableForSymtab(*Symtab, Sections), this);
 }
 
 template <class ELFT>
-ObjFile<ELFT>::ObjFile(MemoryBufferRef M, StringRef ArchiveName)
-    : ELFFileBase<ELFT>(Base::ObjKind, M) {
+ObjFile<ELFT>::ObjFile(MemoryBufferRef M, StringRef ArchiveName,
+                       bool JustSymbols)
+    : ELFFileBase<ELFT>(Base::ObjKind, M), JustSymbols(JustSymbols) {
   this->ArchiveName = ArchiveName;
 }
 
 template <class ELFT> ArrayRef<Symbol *> ObjFile<ELFT>::getLocalSymbols() {
   if (this->Symbols.empty())
     return {};
   return makeArrayRef(this->Symbols).slice(1, this->FirstNonLocal - 1);
 }
 
+// This is for --just-symbols.
+//
+// This option allows you to link your output against other existing
+// program, so that if you load both your program and the other program
+// into memory, your output can use program's symbols.
+//
+// What we are doing here is reading defined symbols from the object.
+template <class ELFT> void ObjFile<ELFT>::readJustSymbols() {
+  ArrayRef<Elf_Shdr> Sections =
+      CHECK(this->getObj().sections(), toString(this));
+
+  for (const Elf_Shdr &Sec : Sections) {
+    if (Sec.sh_type != SHT_SYMTAB)
+      continue;
+
+    Elf_Sym_Range Syms = CHECK(this->getObj().symbols(&Sec), toString(this));
+    this->FirstNonLocal = Sec.sh_info;
+    StringRef StringTable = CHECK(
+        this->getObj().getStringTableForSymtab(Sec, Sections), toString(this));
+
+    for (const Elf_Sym &Sym : Syms.slice(this->FirstNonLocal))
+      if (Sym.st_shndx != SHN_UNDEF)
+        this->Symbols.push_back(
+            Symtab->addRegular(CHECK(Sym.getName(StringTable), toString(this)),
+                               Sym.st_other, Sym.getType(), Sym.st_value,
+                               Sym.st_size, Sym.getBinding(), nullptr, this));
+    return;
+  }
+}
+
 template <class ELFT>
 void ObjFile<ELFT>::parse(DenseSet<CachedHashStringRef> &ComdatGroups) {
+  if (JustSymbols) {
+    readJustSymbols();
+  } else {
     // Read section and symbol tables.
     initializeSections(ComdatGroups);
     initializeSymbols();
   }
+}
 
 // Sections with SHT_GROUP and comdat bits define comdat section groups.
 // They are identified and deduplicated by group name. This function
 // returns a group name.
 template <class ELFT>
 StringRef ObjFile<ELFT>::getShtGroupSignature(ArrayRef<Elf_Shdr> Sections,
                                               const Elf_Shdr &Sec) {
   // Group signatures are stored as symbol names in object files.

 }
 
 static bool isBitcode(MemoryBufferRef MB) {
   using namespace sys::fs;
   return identify_magic(MB.getBuffer()) == file_magic::bitcode;
 }
 
 InputFile *elf::createObjectFile(MemoryBufferRef MB, StringRef ArchiveName,
-                                 uint64_t OffsetInArchive) {
+                                 uint64_t OffsetInArchive, bool JustSymbols) {
   if (isBitcode(MB))
     return make<BitcodeFile>(MB, ArchiveName, OffsetInArchive);
 
   switch (getELFKind(MB)) {
   case ELF32LEKind:
-    return make<ObjFile<ELF32LE>>(MB, ArchiveName);
+    return make<ObjFile<ELF32LE>>(MB, ArchiveName, JustSymbols);
   case ELF32BEKind:
-    return make<ObjFile<ELF32BE>>(MB, ArchiveName);
+    return make<ObjFile<ELF32BE>>(MB, ArchiveName, JustSymbols);
   case ELF64LEKind:
-    return make<ObjFile<ELF64LE>>(MB, ArchiveName);
+    return make<ObjFile<ELF64LE>>(MB, ArchiveName, JustSymbols);
   case ELF64BEKind:
-    return make<ObjFile<ELF64BE>>(MB, ArchiveName);
+    return make<ObjFile<ELF64BE>>(MB, ArchiveName, JustSymbols);
   default:
     llvm_unreachable("getELFKind");
   }
 }
 
 InputFile *elf::createSharedFile(MemoryBufferRef MB, StringRef DefaultSoName) {
   switch (getELFKind(MB)) {
   case ELF32LEKind:
for (const typename ELFT::Shdr &Sec : Sections) {
     for (const typename ELFT::Sym &Sym : Syms.slice(FirstNonLocal))
       if (Sym.st_shndx != SHN_UNDEF)
         Symtab->addLazyObject<ELFT>(CHECK(Sym.getName(StringTable), this),
                                     *this);
     return;
   }
 }
 
-// This is for --just-symbols.
-//
-// This option allows you to link your output against other existing
-// program, so that if you load both your program and the other program
-// into memory, your output can use program's symbols.
-//
-// What we are doing here is to read defined symbols from a given ELF
-// file and add them as absolute symbols.
-template <class ELFT> void elf::readJustSymbolsFile(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);
-
-    for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))
-      if (Sym.st_shndx != SHN_UNDEF)
-        Symtab->addRegular(CHECK(Sym.getName(StringTable), ObjName),
-                           Sym.st_other, Sym.getType(), Sym.st_value,
-                           Sym.st_size, Sym.getBinding(), nullptr, nullptr);
-    return;
-  }
-}
-
 template void ArchiveFile::parse<ELF32LE>();
 template void ArchiveFile::parse<ELF32BE>();
 template void ArchiveFile::parse<ELF64LE>();
 template void ArchiveFile::parse<ELF64BE>();
 
 template void BitcodeFile::parse<ELF32LE>(DenseSet<CachedHashStringRef> &);
 template void BitcodeFile::parse<ELF32BE>(DenseSet<CachedHashStringRef> &);
 template void BitcodeFile::parse<ELF64LE>(DenseSet<CachedHashStringRef> &);

 template class elf::ObjFile<ELF32BE>;
 template class elf::ObjFile<ELF64LE>;
 template class elf::ObjFile<ELF64BE>;
 
 template class elf::SharedFile<ELF32LE>;
 template class elf::SharedFile<ELF32BE>;
 template class elf::SharedFile<ELF64LE>;
 template class elf::SharedFile<ELF64BE>;
-
-template void elf::readJustSymbolsFile<ELF32LE>(MemoryBufferRef);
-template void elf::readJustSymbolsFile<ELF32BE>(MemoryBufferRef);
-template void elf::readJustSymbolsFile<ELF64LE>(MemoryBufferRef);
-template void elf::readJustSymbolsFile<ELF64BE>(MemoryBufferRef);

ELF/ScriptParser.cpp

void ScriptParser::readDefsym(StringRef Name) {
   Script->SectionCommands.push_back(Cmd);
 }
 
 void ScriptParser::addFile(StringRef S) {
   if (IsUnderSysroot && S.startswith("/")) {
     SmallString<128> PathData;
     StringRef Path = (Config->Sysroot + S).toStringRef(PathData);
     if (sys::fs::exists(Path)) {
-      Driver->addFile(Saver.save(Path), /*WithLOption=*/false);
+      Driver->addFile(Saver.save(Path), /*WithLOption=*/false,
+                      /*JustSymbols=*/false);
       return;
     }
   }
 
   if (S.startswith("/")) {
-    Driver->addFile(S, /*WithLOption=*/false);
+    Driver->addFile(S, /*WithLOption=*/false, /*JustSymbols=*/false);
   } else if (S.startswith("=")) {
     if (Config->Sysroot.empty())
-      Driver->addFile(S.substr(1), /*WithLOption=*/false);
+      Driver->addFile(S.substr(1), /*WithLOption=*/false,
+                      /*JustSymbols=*/false);
     else
       Driver->addFile(Saver.save(Config->Sysroot + "/" + S.substr(1)),
-                      /*WithLOption=*/false);
+                      /*WithLOption=*/false, /*JustSymbols=*/false);
   } else if (S.startswith("-l")) {
     Driver->addLibrary(S.substr(2));
   } else if (sys::fs::exists(S)) {
-    Driver->addFile(S, /*WithLOption=*/false);
+    Driver->addFile(S, /*WithLOption=*/false, /*JustSymbols=*/false);
   } else {
     if (Optional<std::string> Path = findFromSearchPaths(S))
-      Driver->addFile(Saver.save(*Path), /*WithLOption=*/true);
+      Driver->addFile(Saver.save(*Path), /*WithLOption=*/true,
+                      /*JustSymbols=*/false);
     else
       setError("unable to find " + S);
   }
 }
 
 void ScriptParser::readAsNeeded() {
   expect("(");
   bool Orig = Config->AsNeeded;

test/ELF/invalid/executable.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: ld.lld -o %t1.exe %t.o
 # RUN: not ld.lld -o %t2.exe %t1.exe 2>&1 | FileCheck %s
-# CHECK: unknown file type
+# CHECK: {{.*}}1.exe: is an executable ELF file
 
 .global _start
 _start:
   ret

test/ELF/just-symbols-cref.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/just-symbols.s -o %t1
+# RUN: ld.lld %t1 -o %t1.exe
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2
+# RUN: ld.lld %t2 -just-symbols=%t1.exe -o /dev/null -cref | FileCheck -strict-whitespace %s
+
+# CHECK:      Symbol                                            File
+# CHECK-NEXT: _start                                            {{.*}}2
+# CHECK-NEXT: bar                                               {{.*}}1.exe
+# CHECK-NEXT:                                                   {{.*}}2
+# CHECK-NEXT: foo                                               {{.*}}1.exe
+# CHECK-NEXT:                                                   {{.*}}2
+
+.globl _start
+_start:
+  call foo
+  call bar
+  ret

test/ELF/just-symbols-input.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t1.exe
+# RUN: ld.lld -just-symbols=%t1.exe -o %t2.exe
+
+## Check we are able to produce the output when the only input is a -just-symbols file.
+# RUN: llvm-readelf -symbols %t2.exe | FileCheck %s
+# CHECK:      Num:    Value          Size Type    Bind   Vis      Ndx Name
+# CHECK-NEXT:   0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
+# CHECK-NEXT:   1: 0000000000201000     0 NOTYPE  GLOBAL DEFAULT  ABS foo
+
+.globl foo
+foo:
+  ret