[LLD] COFF: Define error categories for Driver, Reader and Resolver.

[ruiu]

Summary

Now the errors are categorized so that you can handle them programmatically.
All uses of make_dynamic_error_code are removed.

Because we don't have error reporting mechanism, I just chose to print out
error messages to stdout, and then return an error object. Not sure if
that's the right thing to do, but at least it seems practical.

I'm not very happy about the size of Error.h and its density.
That defines only a few things but it is almost 100 lines long.
Maybe it's unavoidable?

[• rafael]

LGTM, just check if you really need the various ways of saying Success.

COFF/Error.h

21

I don't think you need Success. You can just use std::error_code(), no?

[ruiu]

COFF/Error.h

21

Correct. I'll remove that.

Does it make sense to merge all the error categories into one LLDErrorCategory and let the caller distinguish error type by its value? What do you think?

[Eugene.Zelenko]

Committed in r238714.

COFF/Chunks.cpp

 #include "Chunks.h"
 #include "InputFiles.h"
 #include "Writer.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/COFF.h"

COFF/Driver.cpp

 #include "Memory.h"
 #include "SymbolTable.h"
 #include "Writer.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/Arg.h"

COFF/DriverUtils.cpp

 //===----------------------------------------------------------------------===//
 
 #include "Driver.h"
+#include "Error.h"
 #include "Memory.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"

 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"

                           .Case("x64", IMAGE_FILE_MACHINE_AMD64)
                           .Case("x86", IMAGE_FILE_MACHINE_I386)
                           .Default(IMAGE_FILE_MACHINE_UNKNOWN);
-    if (MT == IMAGE_FILE_MACHINE_UNKNOWN)
-      return make_dynamic_error_code("unknown /machine argument" + S);
+    if (MT == IMAGE_FILE_MACHINE_UNKNOWN) {
+      llvm::errs() << "unknown /machine argument" << S << "\n";
+      return make_error_code(DriverError::InvalidOption);
+    }
     return MT;
   }
   // If /machine option is missing, we need to take a look at

 std::error_code parseNumbers(StringRef Arg, uint64_t *Addr, uint64_t *Size) {
   StringRef S1, S2;
   std::tie(S1, S2) = Arg.split(',');
-  if (S1.getAsInteger(0, *Addr))
-    return make_dynamic_error_code(Twine("invalid number: ") + S1);
-  if (Size && !S2.empty() && S2.getAsInteger(0, *Size))
-    return make_dynamic_error_code(Twine("invalid number: ") + S2);
+  if (S1.getAsInteger(0, *Addr)) {
+    llvm::errs() << "invalid number: " << S1 << "\n";
+    return make_error_code(DriverError::InvalidOption);
+  }
+  if (Size && !S2.empty() && S2.getAsInteger(0, *Size)) {
+    llvm::errs() << "invalid number: " << S2 << "\n";
+    return make_error_code(DriverError::InvalidOption);
+  }
   return std::error_code();
 }
 

 std::error_code parseVersion(StringRef Arg, uint32_t *Major, uint32_t *Minor) {
   StringRef S1, S2;
   std::tie(S1, S2) = Arg.split('.');
-  if (S1.getAsInteger(0, *Major))
-    return make_dynamic_error_code(Twine("invalid number: ") + S1);
+  if (S1.getAsInteger(0, *Major)) {
+    llvm::errs() << "invalid number: " << S1 << "\n";
+    return make_error_code(DriverError::InvalidOption);
+  }
   *Minor = 0;
-  if (!S2.empty() && S2.getAsInteger(0, *Minor))
-    return make_dynamic_error_code(Twine("invalid number: ") + S2);
+  if (!S2.empty() && S2.getAsInteger(0, *Minor)) {
+    llvm::errs() << "invalid number: " << S2 << "\n";
+    return make_error_code(DriverError::InvalidOption);
+  }
   return std::error_code();
 }
 

     .Case("posix", IMAGE_SUBSYSTEM_POSIX_CUI)
     .Case("windows", IMAGE_SUBSYSTEM_WINDOWS_GUI)
     .Default(IMAGE_SUBSYSTEM_UNKNOWN);
-  if (*Sys == IMAGE_SUBSYSTEM_UNKNOWN)
-    return make_dynamic_error_code(Twine("unknown subsystem: ") + SysStr);
+  if (*Sys == IMAGE_SUBSYSTEM_UNKNOWN) {
+    llvm::errs() << "unknown subsystem: " << SysStr << "\n";
+    return make_error_code(DriverError::InvalidOption);
+  }
   if (!Ver.empty())
     if (auto EC = parseVersion(Ver, Major, Minor))
       return EC;

   std::unique_ptr<llvm::opt::InputArgList> Args(
       Table.ParseArgs(&Argv[1], &Argv[Argc], MissingIndex, MissingCount));
   if (MissingCount) {
-    std::string S;
-    llvm::raw_string_ostream OS(S);
-    OS << llvm::format("missing arg value for \"%s\", expected %d argument%s.",
-                       Args->getArgString(MissingIndex), MissingCount,
-                       (MissingCount == 1 ? "" : "s"));
-    OS.flush();
-    return make_dynamic_error_code(StringRef(S));
+    llvm::errs() << "missing arg value for \""
+                 << Args->getArgString(MissingIndex)
+                 << "\", expected " << MissingCount
+                 << (MissingCount == 1 ? " argument.\n" : " arguments.\n");
+    return make_error_code(DriverError::InvalidOption);
   }
   for (auto *Arg : Args->filtered(OPT_UNKNOWN))
     llvm::errs() << "ignoring unknown argument: " << Arg->getSpelling() << "\n";

COFF/Error.h

+//===- Error.h ------------------------------------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_COFF_ERROR_H
+#define LLD_COFF_ERROR_H
+
+#include <string>
+#include <system_error>
+#include "llvm/Support/ErrorHandling.h"
+
+namespace lld {
+namespace coff {
+
+// Driver errors
+enum class DriverError { Success, InvalidOption };
+
+class DriverErrorCategory : public std::error_category {
+public:
+  const char *name() const override { return "lld.driver"; }
+
+  std::string message(int EV) const override {
+    switch (static_cast<int>(EV)) {
+    case DriverError::Success:
+      return "Success";
+    case DriverError::InvalidOption:
+      return "Invalid option";
+    }
+    llvm_unreachable("unknown error");
+  }
+};
+
+inline std::error_code make_error_code(DriverError Err) {
+  static DriverErrorCategory C;
+  return std::error_code(static_cast<int>(Err), C);
+}
+
+// InputFile errors
+enum class ReaderError { Success, InvalidFile, BrokenFile };
+
+class ReaderErrorCategory : public std::error_category {
+public:
+  const char *name() const override { return "lld.reader"; }
+
+  std::string message(int EV) const override {
+    switch (static_cast<int>(EV)) {
+    case ReaderError::Success:
+      return "Success";
+    case ReaderError::InvalidFile:
+      return "Invalid file";
+    case ReaderError::BrokenFile:
+      return "broken file";
+    }
+    llvm_unreachable("unknown error");
+  }
+};
+
+inline std::error_code make_error_code(ReaderError Err) {
+  static ReaderErrorCategory C;
+  return std::error_code(static_cast<int>(Err), C);
+}
+
+// SymbolTable errors
+enum class SymbolError { Success, Duplicate };
+
+class SymbolErrorCategory : public std::error_category {
+public:
+  const char *name() const override { return "lld.symbol"; }
+
+  std::string message(int EV) const override {
+    switch (static_cast<int>(EV)) {
+    case SymbolError::Success:
+      return "Success";
+    case SymbolError::Duplicate:
+      return "Duplicate symbols";
+    }
+    llvm_unreachable("unknown error");
+  }
+};
+
+inline std::error_code make_error_code(SymbolError Err) {
+  static SymbolErrorCategory C;
+  return std::error_code(static_cast<int>(Err), C);
+}
+
+} // namespace coff
+} // namespace lld
+
+#endif

COFF/InputFiles.cpp

 //===----------------------------------------------------------------------===//
 
 #include "Chunks.h"
+#include "Error.h"
 #include "InputFiles.h"
 #include "Writer.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/COFF.h"

     Bin.release();
     COFFObj.reset(Obj);
   } else {
-    return make_dynamic_error_code(Twine(Filename) + " is not a COFF file.");
+    llvm::errs() << Filename << " is not a COFF file.\n";
+    return make_error_code(ReaderError::InvalidFile);
   }
 
   // Read section and symbol tables.

   for (uint32_t I = 1; I < NumSections + 1; ++I) {
     const coff_section *Sec;
     StringRef Name;
-    if (auto EC = COFFObj->getSection(I, Sec))
-      return make_dynamic_error_code(Twine("getSection failed: ") + Name +
-                                     ": " + EC.message());
-    if (auto EC = COFFObj->getSectionName(Sec, Name))
-      return make_dynamic_error_code(Twine("getSectionName failed: ") + Name +
-                                     ": " + EC.message());
+    if (auto EC = COFFObj->getSection(I, Sec)) {
+      llvm::errs() << "getSection failed: " << Name << ": "
+                   << EC.message() << "\n";
+      return make_error_code(ReaderError::BrokenFile);
+    }
+    if (auto EC = COFFObj->getSectionName(Sec, Name)) {
+      llvm::errs() << "getSectionName failed: " << Name << ": "
+                   << EC.message() << "\n";
+      return make_error_code(ReaderError::BrokenFile);
+    }
     if (Name == ".drectve") {
       ArrayRef<uint8_t> Data;
       COFFObj->getSectionContents(Sec, Data);

   for (uint32_t I = 0; I < NumSymbols; ++I) {
     // Get a COFFSymbolRef object.
     auto SymOrErr = COFFObj->getSymbol(I);
-    if (auto EC = SymOrErr.getError())
-      return make_dynamic_error_code(Twine("broken object file: ") + Filename +
-                                     ": " + EC.message());
+    if (auto EC = SymOrErr.getError()) {
+      llvm::dbgs() << "broken object file: " << Filename << ": "
+                   << EC.message() << "\n";
+      return make_error_code(ReaderError::BrokenFile);
+    }
     COFFSymbolRef Sym = SymOrErr.get();
 
     // Get a symbol name.
     StringRef SymbolName;
-    if (auto EC = COFFObj->getSymbolName(Sym, SymbolName))
-      return make_dynamic_error_code(Twine("broken object file: ") + Filename +
-                                     ": " + EC.message());
+    if (auto EC = COFFObj->getSymbolName(Sym, SymbolName)) {
+      llvm::dbgs() << "broken object file: " << Filename << ": "
+                   << EC.message() << "\n";
+      return make_error_code(ReaderError::BrokenFile);
+    }
     // Skip special symbols.
     if (SymbolName == "@comp.id" || SymbolName == "@feat.00")
       continue;

   const auto *Hdr = reinterpret_cast<const coff_import_header *>(Buf);
 
   // Check if the total size is valid.
-  if (End - Buf != sizeof(*Hdr) + Hdr->SizeOfData)
-    return make_dynamic_error_code("broken import library");
+  if (End - Buf != sizeof(*Hdr) + Hdr->SizeOfData) {
+    llvm::errs() << "broken import library\n";
+    return make_error_code(ReaderError::BrokenFile);
+  }
 
   // Read names and create an __imp_ symbol.
   StringRef Name = StringAlloc.save(StringRef(Buf + sizeof(*Hdr)));

COFF/SymbolTable.cpp

 
 #include "Config.h"
 #include "Driver.h"
+#include "Error.h"
 #include "SymbolTable.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"

   int comp = Existing->compare(New);
   if (comp < 0)
     Sym->Body = New;
-  if (comp == 0)
-    return make_dynamic_error_code(Twine("duplicate symbol: ") + Name);
+  if (comp == 0) {
+    llvm::dbgs() << "duplicate symbol: " << Name << "\n";
+    return make_error_code(SymbolError::Duplicate);
+  }
 
   // If we have an Undefined symbol for a Lazy symbol, we need
   // to read an archive member to replace the Lazy symbol with

COFF/Symbols.cpp

 //
 //===----------------------------------------------------------------------===//
 
+#include "Error.h"
 #include "InputFiles.h"
 #include "Symbols.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"

   if (Magic == file_magic::coff_import_library)
     return std::unique_ptr<InputFile>(new ImportFile(MBRef));
 
-  if (Magic != file_magic::coff_object)
-    return make_dynamic_error_code("unknown file type");
+  if (Magic != file_magic::coff_object) {
+    llvm::dbgs() << File->getName() << ": unknown file type\n";
+    return make_error_code(ReaderError::InvalidFile);
+  }
 
   std::unique_ptr<InputFile> Obj(new ObjectFile(MBRef.getBufferIdentifier(), MBRef));
   Obj->setParentName(File->getName());

COFF/Writer.cpp

 
 #include "Config.h"
 #include "Writer.h"
-#include "lld/Core/Error.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/STLExtras.h"

 
 std::error_code Writer::openFile(StringRef Path) {
   if (auto EC = FileOutputBuffer::create(Path, FileSize, Buffer,
-                                         FileOutputBuffer::F_executable))
-    return make_dynamic_error_code(Twine("Failed to open ") + Path + ": " +
-                                   EC.message());
+                                         FileOutputBuffer::F_executable)) {
+    llvm::errs() << "failed to open " << Path << ": " << EC.message() << "\n";
+    return EC;
+  }
   return std::error_code();
 }