[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.

[ruiu]

COFF/Driver.cpp

160–164 ↗

(On Diff #87541)

nit: when you add {}, please add {} to other if-elseif-else.

543–545 ↗

(On Diff #87541)

Ditto

616–638 ↗

(On Diff #87541)

I think it is not safe to continue with wrong values. After you parse all command line options, you want to check if ErrorCount == 0, and if it is not, you should exit.

745–746 ↗

(On Diff #87541)

Braces.

[inglorion]

changes requested by @ruiu

[ruiu]

LGTM

[vitalybuka]

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/2851/steps/check-lld%20msan/logs/stdio

Exit Code: 1

Command Output (stderr):
--
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/lld/test/COFF/error-limit.test:30:8: error: expected string not found in input
WRONG: could not open 01
       ^
<stdin>:3:1: note: scanning from here

^

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();
void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> MB) {
   }
 
   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));
   });
 
   if (Config->OutputFile == "")
     Config->OutputFile = getOutputPath(Path);
 }
 
 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);
   if (Config->Verbose)
     outs() << "Loaded " << toString(Obj) << " for " << SymName << "\n";
 }
 
 void LinkerDriver::enqueueArchiveMember(const Archive::Child &C,
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) {
   InitializeAllTargetMCs();
   InitializeAllAsmParsers();
   InitializeAllAsmPrinters();
   InitializeAllDisassemblers();
 
   // Parse command line options.
   opt::InputArgList Args = Parser.parseLINK(ArgsArr.slice(1));
 
+  // 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");
+      error("/noentry must be specified with /dll");
+    else
       Config->NoEntry = true;
   }
 
   // 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
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   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()))
       enqueuePath(*Path);
 
+  if (ErrorCount)
+    return;
+
   // Windows specific -- Create a resource file containing a manifest file.
   if (Config->Manifest == Configuration::Embed)
     addBuffer(createManifestRes());
 
   // Read all input files given via the command line.
   run();
 
   // We should have inferred a machine type by now from the input files, but if
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   } else if (Args.hasArg(OPT_dll) && !Config->NoEntry) {
     StringRef S = (Config->Machine == I386) ? "__DllMainCRTStartup@12"
                                             : "_DllMainCRTStartup";
     Config->Entry = addUndefined(S);
   } else if (!Config->NoEntry) {
     // Windows specific -- If entry point name is not given, we need to
     // infer that from user-defined entry name.
     StringRef S = findDefaultEntry();
-    if (S.empty())
-      fatal("entry point must be defined");
+    if (S.empty()) {
+      error("entry point must be defined");
+    } else {
       Config->Entry = addUndefined(S);
       if (Config->Verbose)
         outs() << "Entry name inferred: " << S << "\n";
     }
+  }
 
   // Handle /export
   for (auto *Arg : Args.filtered(OPT_export)) {
     Export E = parseExport(Arg->getValue());
     if (Config->Machine == I386) {
       if (!isDecorated(E.Name))
         E.Name = Saver.save("_" + E.Name);
       if (!E.ExtName.empty() && !isDecorated(E.ExtName))
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)) {
     std::vector<StringRef> ObjectFiles = Symtab.compileBitcodeFiles();
     runMSVCLinker(Args, ObjectFiles);
     exit(0);
   }
 
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   // 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

if (Config->ColorDiagnostics) {
     *ErrorOS << S;
   }
 }
 
 void error(const Twine &Msg) {
   std::lock_guard<std::mutex> Lock(Mu);
 
   if (Config->ErrorLimit == 0 || ErrorCount < Config->ErrorLimit) {
+    errs() << "error " << ErrorCount << " of " << Config->ErrorLimit << "\n";
     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
+WRONG:      could not open 01
+WRONG:      could not open 19
+WRONG-NEXT: too many errors emitted, stopping now (use /ERRORLIMIT:0 to see all errors)
+WRONG-NOT:  could not open 20