ELF: Report multiple errors from the driver.

[ruiu]

Summary

This patch let the driver keep going until it parses all
command line options.

[ruiu]

• Made the code a bit shorter by changing the return type of error.

[• rafael]

LGTM with two nits.

ELF/Driver.cpp

68

This is a broken file case, I would leave it for another patch.

74

Same here.

[ruiu]

ELF/Driver.cpp

68

Fair. Done.

[grimar]

ELF/Driver.cpp

120

This will not work. Problem is that searchLibrary() returns std::string which is destroyed after assigning buffer pointer to StringRef. Buffer is filled with garbage in debug and some tests under windows + msvs2015/debug were failing because of that.
r259186 fixes it:

std::string Path = searchLibrary(Name);

ELF/Driver.h

 
 // Entry point of the ELF linker.
 bool link(ArrayRef<const char *> Args);
 
 class LinkerDriver {
 public:
   void main(ArrayRef<const char *> Args);
   void addFile(StringRef Path);
+  void addLibrary(StringRef Name);
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
   void createFiles(llvm::opt::InputArgList &Args);
   template <class ELFT> void link(llvm::opt::InputArgList &Args);
 
   llvm::BumpPtrAllocator Alloc;
   bool WholeArchive = false;

ELF/Driver.cpp

if (S == "elf64ppc" || S == "elf64ppc_fbsd")
     return {ELF64BEKind, EM_PPC64};
   if (S == "elf_i386")
     return {ELF32LEKind, EM_386};
   if (S == "elf_x86_64")
     return {ELF64LEKind, EM_X86_64};
   if (S == "aarch64linux")
     return {ELF64LEKind, EM_AARCH64};
   if (S == "i386pe" || S == "i386pep" || S == "thumb2pe")
-    fatal("Windows targets are not supported on the ELF frontend: " + S);
-  fatal("Unknown emulation: " + S);
+    error("Windows targets are not supported on the ELF frontend: " + S);
+  else
+    error("Unknown emulation: " + S);
+  return {ELFNoneKind, 0};
 }
 
 // Returns slices of MB by parsing MB as an archive file.
 // Each slice consists of a member file in the archive.
 static std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef MB) {
   ErrorOr<std::unique_ptr<Archive>> FileOrErr = Archive::create(MB);
-  fatal(FileOrErr, "Failed to parse archive");
+  if (error(FileOrErr, "Failed to parse archive"))
+    return {};
   std::unique_ptr<Archive> File = std::move(*FileOrErr);
 
   std::vector<MemoryBufferRef> V;
   for (const ErrorOr<Archive::Child> &C : File->children()) {
-    fatal(C, "Could not get the child of the archive " + File->getFileName());
-    ErrorOr<MemoryBufferRef> MbOrErr = C->getMemoryBufferRef();
-    fatal(MbOrErr, "Could not get the buffer for a child of the archive " +
-                       File->getFileName());
-    V.push_back(*MbOrErr);
+    StringRef S = File->getFileName();
+    if (error(C, "Could not get the child of the archive " + S))
+      return {};
+    ErrorOr<MemoryBufferRef> MB = C->getMemoryBufferRef();
+    if (error(MB, "Could not get the buffer for a child of the archive " + S))
+      return {};
+    V.push_back(*MB);
   }
   return V;
 }
 
 // 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) {
   using namespace llvm::sys::fs;
   if (Config->Verbose)
     llvm::outs() << Path << "\n";
   auto MBOrErr = MemoryBuffer::getFile(Path);
-  fatal(MBOrErr, "cannot open " + Path);
+  if (error(MBOrErr, "cannot open " + Path))
+    return;
   std::unique_ptr<MemoryBuffer> &MB = *MBOrErr;
   MemoryBufferRef MBRef = MB->getMemBufferRef();
   OwningMBs.push_back(std::move(MB)); // take MB ownership
 
   switch (identify_magic(MBRef.getBuffer())) {
   case file_magic::unknown:
     readLinkerScript(&Alloc, MBRef);
     return;
   case file_magic::archive:
     if (WholeArchive) {
       for (MemoryBufferRef MB : getArchiveMembers(MBRef))
         Files.push_back(createObjectFile(MB));
       return;
     }
     Files.push_back(make_unique<ArchiveFile>(MBRef));
     return;
   case file_magic::elf_shared_object:
     Files.push_back(createSharedFile(MBRef));
     return;
   default:
     Files.push_back(createObjectFile(MBRef));
   }
 }
 
+// Add a given library by searching it from input search paths.
+void LinkerDriver::addLibrary(StringRef Name) {
+  StringRef Path = searchLibrary(Name);
+  if (Path.empty())
+    error("Unable to find library -l" + Name);
+  else
+    addFile(Path);
+}
+
 // Some command line options or some combinations of them are not allowed.
 // This function checks for such fatals.
 static void checkOptions(opt::InputArgList &Args) {
   // Traditional linkers can generate re-linkable object files instead
   // of executables or DSOs. We don't support that since the feature
   // does not seem to provide more value than the static archiver.
   if (Args.hasArg(OPT_relocatable))
-    fatal("-r option is not supported. Use 'ar' command instead.");
+    error("-r option is not supported. Use 'ar' command instead.");
 
   // The MIPS ABI as of 2016 does not support the GNU-style symbol lookup
   // table which is a relatively new feature.
   if (Config->EMachine == EM_MIPS && Config->GnuHash)
-    fatal("The .gnu.hash section is not compatible with the MIPS target.");
+    error("The .gnu.hash section is not compatible with the MIPS target.");
 
   if (Config->EMachine == EM_AMDGPU && !Config->Entry.empty())
-    fatal("-e option is not valid for AMDGPU.");
+    error("-e option is not valid for AMDGPU.");
 }
 
 static StringRef
 getString(opt::InputArgList &Args, unsigned Key, StringRef Default = "") {
   if (auto *Arg = Args.getLastArg(Key))
     return Arg->getValue();
   return Default;
 }
 
 static bool hasZOption(opt::InputArgList &Args, StringRef Key) {
   for (auto *Arg : Args.filtered(OPT_z))
     if (Key == Arg->getValue())
       return true;
   return false;
 }
 
 void LinkerDriver::main(ArrayRef<const char *> ArgsArr) {
   initSymbols();
 
   opt::InputArgList Args = parseArgs(&Alloc, ArgsArr);
   readConfigs(Args);
   createFiles(Args);
   checkOptions(Args);
+  if (HasError)
+    return;
 
   switch (Config->EKind) {
   case ELF32LEKind:
     link<ELF32LE>(Args);
     return;
   case ELF32BEKind:
     link<ELF32BE>(Args);
     return;
   case ELF64LEKind:
     link<ELF64LE>(Args);
     return;
   case ELF64BEKind:
     link<ELF64BE>(Args);
     return;
   default:
-    fatal("-m or at least a .o file required");
+    error("-m or at least a .o file required");
   }
 }
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   for (auto *Arg : Args.filtered(OPT_L))
     Config->SearchPaths.push_back(Arg->getValue());
 
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZNodelete = hasZOption(Args, "nodelete");
   Config->ZNow = hasZOption(Args, "now");
   Config->ZOrigin = hasZOption(Args, "origin");
   Config->ZRelro = !hasZOption(Args, "norelro");
 
   if (auto *Arg = Args.getLastArg(OPT_O)) {
     StringRef Val = Arg->getValue();
     if (Val.getAsInteger(10, Config->Optimize))
-      fatal("Invalid optimization level");
+      error("Invalid optimization level");
   }
 
   if (auto *Arg = Args.getLastArg(OPT_hash_style)) {
     StringRef S = Arg->getValue();
     if (S == "gnu") {
       Config->GnuHash = true;
       Config->SysvHash = false;
     } else if (S == "both") {
       Config->GnuHash = true;
     } else if (S != "sysv")
-      fatal("Unknown hash style: " + S);
+      error("Unknown hash style: " + S);
   }
 
   for (auto *Arg : Args.filtered(OPT_undefined))
     Config->Undefined.push_back(Arg->getValue());
 }
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
     switch (Arg->getOption().getID()) {
     case OPT_l:
-      addFile(searchLibrary(Arg->getValue()));
+      addLibrary(Arg->getValue());
       break;
     case OPT_INPUT:
     case OPT_script:
       addFile(Arg->getValue());
       break;
     case OPT_as_needed:
       Config->AsNeeded = true;
       break;
case OPT_whole_archive:
       WholeArchive = true;
       break;
     case OPT_no_whole_archive:
       WholeArchive = false;
       break;
     }
   }
 
-  if (Files.empty())
-    fatal("no input files.");
+  if (Files.empty() && !HasError)
+    error("no input files.");
 }
 
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   SymbolTable<ELFT> Symtab;
   Target.reset(createTarget());
 
   if (!Config->Shared) {
     // Add entry symbol.

ELF/DriverUtils.cpp

opt::InputArgList elf2::parseArgs(llvm::BumpPtrAllocator *A,
   // Expand response files. '@<filename>' is replaced by the file's contents.
   SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
   StringSaver Saver(*A);
   llvm::cl::ExpandResponseFiles(Saver, llvm::cl::TokenizeGNUCommandLine, Vec);
 
   // Parse options and then do fatal checking.
   opt::InputArgList Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
   if (MissingCount)
-    fatal(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
+    error(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
           "\", expected " + Twine(MissingCount) +
           (MissingCount == 1 ? " argument.\n" : " arguments"));
 
   iterator_range<opt::arg_iterator> Unknowns = Args.filtered(OPT_UNKNOWN);
   for (auto *Arg : Unknowns)
     warning("warning: unknown argument: " + Arg->getSpelling());
   if (Unknowns.begin() != Unknowns.end())
-    fatal("unknown argument(s) found");
-
+    error("unknown argument(s) found");
   return Args;
 }
 
 std::string elf2::findFromSearchPaths(StringRef Path) {
   for (StringRef Dir : Config->SearchPaths) {
     std::string FullPath = buildSysrootedPath(Dir, Path);
     if (sys::fs::exists(FullPath))
       return FullPath;
if (!Config->Static)
       Names.push_back(("lib" + Path + ".so").str());
     Names.push_back(("lib" + Path + ".a").str());
   }
   for (const std::string &Name : Names) {
     std::string S = findFromSearchPaths(Name);
     if (!S.empty())
       return S;
   }
-  fatal("Unable to find library -l" + Path);
+  return "";
 }
 
 // Makes a path by concatenating Dir and File.
 // If Dir starts with '=' the result will be preceded by Sysroot,
 // which can be set with --sysroot command line switch.
 std::string elf2::buildSysrootedPath(StringRef Dir, StringRef File) {
   SmallString<128> Path;
   if (Dir.startswith("="))
     sys::path::append(Path, Config->Sysroot, Dir.substr(1), File);
   else
     sys::path::append(Path, Dir, File);
   return Path.str();
 }

ELF/Error.h

 namespace lld {
 namespace elf2 {
 
 extern thread_local bool HasError;
 
 void warning(const Twine &Msg);
 
 void error(const Twine &Msg);
-void error(std::error_code EC, const Twine &Prefix);
-void error(std::error_code EC);
+bool error(std::error_code EC, const Twine &Prefix);
+bool error(std::error_code EC);
 
-template <typename T> void error(const ErrorOr<T> &V, const Twine &Prefix) {
-  error(V.getError(), Prefix);
+template <typename T> bool error(const ErrorOr<T> &V, const Twine &Prefix) {
+  return error(V.getError(), Prefix);
+}
+
+template <typename T> bool error(const ErrorOr<T> &V) {
+  return error(V.getError());
 }
-template <typename T> void error(const ErrorOr<T> &V) { error(V.getError()); }
 
 LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg);
 void fatal(std::error_code EC, const Twine &Prefix);
 void fatal(std::error_code EC);
 
 template <typename T> void fatal(const ErrorOr<T> &V, const Twine &Prefix) {
   fatal(V.getError(), Prefix);
 }
+
 template <typename T> void fatal(const ErrorOr<T> &V) { fatal(V.getError()); }
 
 } // namespace elf2
 } // namespace lld
 
 #endif

ELF/Error.cpp

 
 void warning(const Twine &Msg) { llvm::errs() << Msg << "\n"; }
 
 void error(const Twine &Msg) {
   llvm::errs() << Msg << "\n";
   HasError = true;
 }
 
-void error(std::error_code EC, const Twine &Prefix) {
-  if (EC)
-    error(Prefix + ": " + EC.message());
+bool error(std::error_code EC, const Twine &Prefix) {
+  if (!EC)
+    return false;
+  return true;
 }
 
-void error(std::error_code EC) {
-  if (EC)
-    error(EC.message());
+bool error(std::error_code EC) {
+  if (!EC)
+    return false;
+  return true;
 }
 
 void fatal(const Twine &Msg) {
   llvm::errs() << Msg << "\n";
   exit(1);
 }
 
 void fatal(std::error_code EC, const Twine &Prefix) {

ELF/LinkerScript.cpp

void LinkerScript::addFile(StringRef S) {
   if (sys::path::is_absolute(S)) {
     Driver->addFile(S);
   } else if (S.startswith("=")) {
     if (Config->Sysroot.empty())
       Driver->addFile(S.substr(1));
     else
       Driver->addFile(Saver.save(Config->Sysroot + "/" + S.substr(1)));
   } else if (S.startswith("-l")) {
-    Driver->addFile(searchLibrary(S.substr(2)));
+    Driver->addLibrary(S.substr(2));
   } else if (sys::fs::exists(S)) {
     Driver->addFile(S);
   } else {
     std::string Path = findFromSearchPaths(S);
     if (Path.empty())
       fatal("Unable to find " + S);
     Driver->addFile(Saver.save(Path));
   }

test/ELF/driver.test

+# RUN: not ld.lld -unknown1 -unknown2 -m foo /no/such/file -lnosuchlib \
+# RUN:   2>&1 | FileCheck %s
+
+# CHECK: warning: unknown argument: -unknown1
+# CHECK: warning: unknown argument: -unknown2
+# CHECK: unknown argument(s) found
+# CHECK: Unknown emulation: foo
+# CHECK: cannot open /no/such/file
+# CHECK: Unable to find library -lnosuchlib