[COFF] support /ERRORLIMIT option

[inglorion]

Summary

This adds support for reporting multiple errors in a single invocation of lld-link. The limit defaults to 20 and can be changed with the /ERRORLIMIT command line parameter, or set to unlimited by passing a value of 0.

This is a new attempt after r295507, which was reverted because opening files raced with exiting early, causing the test to be flaky. This version avoids the race by exiting before calling enqueuePath.

[inglorion]

Previous diff: D29691. The early exit in Driver.cpp line 820 is now before any paths are enqueued, avoiding the race that existed in the previous diff.

[ruiu]

COFF/Driver.cpp

124 ↗

(On Diff #94143)

Add a blank line so that this if ~ else is visually separated from the above ifs.

654 ↗

(On Diff #94143)

Flip the condition to remove !.

COFF/Error.cpp

75 ↗

(On Diff #94143)

Remove?

[inglorion]

Fixed issues pointed out by @ruiu (thanks!)

[ruiu]

LGTM

lld/trunk/COFF/Driver.cpp

bool link(ArrayRef<const char *> Args, raw_ostream &Diag) {
   ErrorCount = 0;
   ErrorOS = &Diag;
   Argv0 = Args[0];
   Config = make<Configuration>();
   Config->ColorDiagnostics =
       (ErrorOS == &llvm::errs() && Process::StandardErrHasColors());
   Driver = make<LinkerDriver>();
   Driver->link(Args);
-  return true;
+  return !ErrorCount;
 }
 
 // Drop directory components and replace extension with ".exe" or ".dll".
 static std::string getOutputPath(StringRef Path) {
   auto P = Path.find_last_of("\\/");
   StringRef S = (P == StringRef::npos) ? Path : Path.substr(P + 1);
   const char* E = Config->DLL ? ".dll" : ".exe";
   return (S.substr(0, S.rfind('.')) + E).str();
if (Magic == file_magic::windows_resource) {
     return;
   }
 
   FilePaths.push_back(MBRef.getBufferIdentifier());
   if (Magic == file_magic::archive)
     return Symtab.addFile(make<ArchiveFile>(MBRef));
   if (Magic == file_magic::bitcode)
     return Symtab.addFile(make<BitcodeFile>(MBRef));
+
   if (Magic == file_magic::coff_cl_gl_object)
-    fatal(MBRef.getBufferIdentifier() + ": is not a native COFF file. "
+    error(MBRef.getBufferIdentifier() + ": is not a native COFF file. "
           "Recompile without /GL");
+  else
     Symtab.addFile(make<ObjectFile>(MBRef));
 }
 
 void LinkerDriver::enqueuePath(StringRef Path) {
   auto Future =
       std::make_shared<std::future<MBErrPair>>(createFutureForFile(Path));
   std::string PathStr = Path;
   enqueueTask([=]() {
     auto MBOrErr = Future->get();
     if (MBOrErr.second)
-      fatal(MBOrErr.second, "could not open " + PathStr);
+      error("could not open " + PathStr + ": " + MBOrErr.second.message());
+    else
       Driver->addBuffer(std::move(MBOrErr.first));
   });
 }
 
 void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName,
                                     StringRef ParentName) {
   file_magic Magic = identify_magic(MB.getBuffer());
   if (Magic == file_magic::coff_import_library) {
     Symtab.addFile(make<ImportFile>(MB));
     return;
   }
 
   InputFile *Obj;
-  if (Magic == file_magic::coff_object)
+  if (Magic == file_magic::coff_object) {
     Obj = make<ObjectFile>(MB);
-  else if (Magic == file_magic::bitcode)
+  } else if (Magic == file_magic::bitcode) {
     Obj = make<BitcodeFile>(MB);
-  else
-    fatal("unknown file type: " + MB.getBufferIdentifier());
+  } else {
+    error("unknown file type: " + MB.getBufferIdentifier());
+    return;
+  }
 
   Obj->ParentName = ParentName;
   Symtab.addFile(Obj);
   log("Loaded " + toString(Obj) + " for " + SymName);
 }
 
 void LinkerDriver::enqueueArchiveMember(const Archive::Child &C,
                                         StringRef SymName,
case OPT_section:
       parseSection(Arg->getValue());
       break;
     case OPT_editandcontinue:
     case OPT_fastfail:
     case OPT_guardsym:
     case OPT_throwingnew:
       break;
     default:
-      fatal(Arg->getSpelling() + " is not allowed in .drectve");
+      error(Arg->getSpelling() + " is not allowed in .drectve");
     }
   }
 }
 
 // Find file from search paths. You can omit ".obj", this function takes
 // care of that. Note that the returned path is not guaranteed to exist.
 StringRef LinkerDriver::doFindFile(StringRef Filename) {
   bool HasPathSep = (Filename.find_first_of("/\\") != StringRef::npos);
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
 
   // Parse and evaluate -mllvm options.
   std::vector<const char *> V;
   V.push_back("lld-link (LLVM option parsing)");
   for (auto *Arg : Args.filtered(OPT_mllvm))
     V.push_back(Arg->getValue());
   cl::ParseCommandLineOptions(V.size(), V.data());
 
+  // Handle /errorlimit early, because error() depends on it.
+  if (auto *Arg = Args.getLastArg(OPT_errorlimit)) {
+    int N = 20;
+    StringRef S = Arg->getValue();
+    if (S.getAsInteger(10, N))
+      error(Arg->getSpelling() + " number expected, but got " + S);
+    Config->ErrorLimit = N;
+  }
+
   // Handle /help
   if (Args.hasArg(OPT_help)) {
     printHelp(ArgsArr[0]);
     return;
   }
 
   if (auto *Arg = Args.getLastArg(OPT_linkrepro)) {
     SmallString<64> Path = StringRef(Arg->getValue());
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   }
 
   // Create a dummy PDB file to satisfy build sytem rules.
   if (auto *Arg = Args.getLastArg(OPT_pdb))
     Config->PDBPath = Arg->getValue();
 
   // Handle /noentry
   if (Args.hasArg(OPT_noentry)) {
-    if (!Args.hasArg(OPT_dll))
-      fatal("/noentry must be specified with /dll");
+    if (Args.hasArg(OPT_dll))
       Config->NoEntry = true;
+    else
+      error("/noentry must be specified with /dll");
   }
 
   // Handle /dll
   if (Args.hasArg(OPT_dll)) {
     Config->DLL = true;
     Config->ManifestID = 2;
   }
 
   // Handle /fixed
   if (Args.hasArg(OPT_fixed)) {
-    if (Args.hasArg(OPT_dynamicbase))
-      fatal("/fixed must not be specified with /dynamicbase");
+    if (Args.hasArg(OPT_dynamicbase)) {
+      error("/fixed must not be specified with /dynamicbase");
+    } else {
       Config->Relocatable = false;
       Config->DynamicBase = false;
     }
+  }
 
   // Handle /machine
   if (auto *Arg = Args.getLastArg(OPT_machine))
     Config->Machine = getMachineType(Arg->getValue());
 
   // Handle /nodefaultlib:<filename>
   for (auto *Arg : Args.filtered(OPT_nodefaultlib))
     Config->NoDefaultLibs.insert(doFindLib(Arg->getValue()));
for (StringRef S : Vec) {
       if (S == "noicf") {
         Config->DoICF = false;
         continue;
       }
       if (StringRef(S).startswith("lldlto=")) {
         StringRef OptLevel = StringRef(S).substr(7);
         if (OptLevel.getAsInteger(10, Config->LTOOptLevel) ||
             Config->LTOOptLevel > 3)
-          fatal("/opt:lldlto: invalid optimization level: " + OptLevel);
+          error("/opt:lldlto: invalid optimization level: " + OptLevel);
         continue;
       }
       if (StringRef(S).startswith("lldltojobs=")) {
         StringRef Jobs = StringRef(S).substr(11);
         if (Jobs.getAsInteger(10, Config->LTOJobs) || Config->LTOJobs == 0)
-          fatal("/opt:lldltojobs: invalid job count: " + Jobs);
+          error("/opt:lldltojobs: invalid job count: " + Jobs);
         continue;
       }
       if (StringRef(S).startswith("lldltopartitions=")) {
         StringRef N = StringRef(S).substr(17);
         if (N.getAsInteger(10, Config->LTOPartitions) ||
             Config->LTOPartitions == 0)
-          fatal("/opt:lldltopartitions: invalid partition count: " + N);
+          error("/opt:lldltopartitions: invalid partition count: " + N);
         continue;
       }
       if (S != "ref" && S != "lbr" && S != "nolbr")
-        fatal("/opt: unknown option: " + S);
+        error("/opt: unknown option: " + S);
     }
   }
 
   // Handle /lldsavetemps
   if (Args.hasArg(OPT_lldsavetemps))
     Config->SaveTemps = true;
 
   // Handle /failifmismatch
if (Args.hasArg(OPT_tsaware_no))
     Config->TerminalServerAware = false;
   if (Args.hasArg(OPT_nosymtab))
     Config->WriteSymtab = false;
   Config->DumpPdb = Args.hasArg(OPT_dumppdb);
   Config->DebugPdb = Args.hasArg(OPT_debugpdb);
 
   Config->MapFile = getMapFile(Args);
 
+  if (ErrorCount)
+    return;
+
   // Create a list of input files. Files can be given as arguments
   // for /defaultlib option.
   std::vector<MemoryBufferRef> MBs;
   for (auto *Arg : Args.filtered(OPT_INPUT))
     if (Optional<StringRef> Path = findFile(Arg->getValue()))
       enqueuePath(*Path);
   for (auto *Arg : Args.filtered(OPT_defaultlib))
     if (Optional<StringRef> Path = findLib(Arg->getValue()))
for (auto Pair : Config->AlternateNames) {
           U->WeakAlias = Symtab.addUndefined(To);
     }
 
     // Windows specific -- if __load_config_used can be resolved, resolve it.
     if (Symtab.findUnderscore("_load_config_used"))
       addUndefined(mangle("_load_config_used"));
   } while (run());
 
+  if (ErrorCount)
+    return;
+
   // If /msvclto is given, we use the MSVC linker to link LTO output files.
   // This is useful because MSVC link.exe can generate complete PDBs.
   if (Args.hasArg(OPT_msvclto)) {
     invokeMSVC(Args);
     exit(0);
   }
 
   // Do LTO by compiling bitcode input files to a set of native COFF files then
   // link those files.
   Symtab.addCombinedLTOObjects();
   run();
 
   // Make sure we have resolved all symbols.
   Symtab.reportRemainingUndefines();
 
   // Windows specific -- if no /subsystem is given, we need to infer
   // that from entry point name.
   if (Config->Subsystem == IMAGE_SUBSYSTEM_UNKNOWN) {
     Config->Subsystem = inferSubsystem();
     if (Config->Subsystem == IMAGE_SUBSYSTEM_UNKNOWN)
       fatal("subsystem must be defined");
   }
 
   // Handle /safeseh.
-  if (Args.hasArg(OPT_safeseh))
+  if (Args.hasArg(OPT_safeseh)) {
     for (ObjectFile *File : Symtab.ObjectFiles)
       if (!File->SEHCompat)
-        fatal("/safeseh: " + File->getName() + " is not compatible with SEH");
+        error("/safeseh: " + File->getName() + " is not compatible with SEH");
+    if (ErrorCount)
+      return;
+  }
 
   // Windows specific -- when we are creating a .dll file, we also
   // need to create a .lib file.
   if (!Config->Exports.empty() || Config->DLL) {
     fixupExports();
     writeImportLibrary();
     assignExportOrdinals();
   }

lld/trunk/COFF/Error.cpp

void error(const Twine &Msg) {
   std::lock_guard<std::mutex> Lock(Mu);
 
   if (Config->ErrorLimit == 0 || ErrorCount < Config->ErrorLimit) {
     print("error: ", raw_ostream::RED);
     *ErrorOS << Msg << "\n";
   } else if (ErrorCount == Config->ErrorLimit) {
     print("error: ", raw_ostream::RED);
     *ErrorOS << "too many errors emitted, stopping now"
-             << " (use -error-limit=0 to see all errors)\n";
+             << " (use /ERRORLIMIT:0 to see all errors)\n";
     exitLld(1);
   }
 
   ++ErrorCount;
 }
 
 void fatal(const Twine &Msg) {
   if (Config->ColorDiagnostics) {

lld/trunk/COFF/Options.td

 }
 
 def align   : P<"align", "Section alignment">;
 def alternatename : P<"alternatename", "Define weak alias">;
 def base    : P<"base", "Base address of the program">;
 def defaultlib : P<"defaultlib", "Add the library to the list of input files">;
 def delayload : P<"delayload", "Delay loaded DLL name">;
 def entry   : P<"entry", "Name of entry point symbol">;
+def errorlimit : P<"errorlimit",
+    "Maximum number of errors to emit before stopping (0 = no limit)">;
 def export  : P<"export", "Export a function">;
 // No help text because /failifmismatch is not intended to be used by the user.
 def failifmismatch : P<"failifmismatch", "">;
 def heap    : P<"heap", "Size of the heap">;
 def implib  : P<"implib", "Import library name">;
 def libpath : P<"libpath", "Additional library search path">;
 def linkrepro : P<"linkrepro", "Dump linker invocation and input files for debugging">;
 def lldsavetemps : F<"lldsavetemps">,

lld/trunk/COFF/SymbolTable.cpp

void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol Sym) {
   auto *U = dyn_cast<Undefined>(S->body());
   if (!U || U->WeakAlias || S->PendingArchiveLoad)
     return;
   S->PendingArchiveLoad = true;
   F->addMember(&Sym);
 }
 
 void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) {
-  fatal("duplicate symbol: " + toString(*Existing->body()) + " in " +
+  error("duplicate symbol: " + toString(*Existing->body()) + " in " +
         toString(Existing->body()->getFile()) + " and in " +
         (NewFile ? toString(NewFile) : "(internal)"));
 }
 
 Symbol *SymbolTable::addAbsolute(StringRef N, COFFSymbolRef Sym) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N);

lld/trunk/test/COFF/error-limit.test

+RUN: not lld-link 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 \
+RUN:   21 22 2>&1 | FileCheck -check-prefix=DEFAULT %s
+
+DEFAULT:      could not open 01
+DEFAULT:      could not open 20
+DEFAULT-NEXT: too many errors emitted, stopping now (use /ERRORLIMIT:0 to see all errors)
+DEFAULT-NOT:  could not open 21
+
+RUN: not lld-link /ERRORLIMIT:5 01 02 03 04 05 06 07 08 09 10 2>&1 \
+RUN:   | FileCheck -check-prefix=LIMIT5 %s
+
+LIMIT5:      could not open 01
+LIMIT5:      could not open 05
+LIMIT5-NEXT: too many errors emitted, stopping now (use /ERRORLIMIT:0 to see all errors)
+LIMIT5-NOT:  could not open 06
+
+RUN: not lld-link /ERRORLIMIT:0 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 \
+RUN:   16 17 18 19 20 21 22 2>&1 | FileCheck -check-prefix=UNLIMITED %s
+
+UNLIMITED:     could not open 01
+UNLIMITED:     could not open 20
+UNLIMITED:     could not open 21
+UNLIMITED:     could not open 22
+UNLIMITED-NOT: too many errors emitted, stopping now (use /ERRORLIMIT:0 to see all errors)
+
+RUN: not lld-link /ERRORLIMIT:XYZ 01 02 03 04 05 06 07 08 09 10 11 12 13 14 \
+RUN:   15 16 17 18 19 20 21 22 2>&1 | FileCheck -check-prefix=WRONG %s
+
+WRONG:      /ERRORLIMIT: number expected, but got XYZ