From 4cdfe69982e38455226014621fa81252ab946f66 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sun, 19 Feb 2017 02:25:47 +0000 Subject: [PATCH] Revert "[COFF] support /ERRORLIMIT option" Behavior races on ErrorCount. If the enqueued paths are evaluated eagerly (in enqueuePath) then the behavior is as the test expects. But they may not be evaluated until the future is waited on, in run() - which is after the early return/exit on ErrorCount. (this causes the test to fail (because in the "/ERRORCOUNT:XYZ" test, no other errors are printed), at least for me, on linux) This reverts commit r295507. llvm-svn: 295590 --- lld/COFF/Driver.cpp | 81 ++++++++++++---------------------- lld/COFF/Error.cpp | 3 +- lld/COFF/Options.td | 2 - lld/COFF/SymbolTable.cpp | 2 +- lld/test/COFF/error-limit.test | 33 -------------- 5 files changed, 29 insertions(+), 92 deletions(-) delete mode 100644 lld/test/COFF/error-limit.test diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 4bb46cd23534..c648882aced7 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -63,7 +63,7 @@ bool link(ArrayRef Args, raw_ostream &Diag) { (ErrorOS == &llvm::errs() && Process::StandardErrHasColors()); Driver = make(); Driver->link(Args); - return !ErrorCount; + return true; } // Drop directory components and replace extension with ".exe" or ".dll". @@ -126,10 +126,9 @@ void LinkerDriver::addBuffer(std::unique_ptr MB) { if (Magic == file_magic::bitcode) return Symtab.addFile(make(MBRef)); if (Magic == file_magic::coff_cl_gl_object) - error(MBRef.getBufferIdentifier() + ": is not a native COFF file. " + fatal(MBRef.getBufferIdentifier() + ": is not a native COFF file. " "Recompile without /GL"); - else - Symtab.addFile(make(MBRef)); + Symtab.addFile(make(MBRef)); } void LinkerDriver::enqueuePath(StringRef Path) { @@ -139,9 +138,8 @@ void LinkerDriver::enqueuePath(StringRef Path) { enqueueTask([=]() { auto MBOrErr = Future->get(); if (MBOrErr.second) - error("could not open " + PathStr + ": " + MBOrErr.second.message()); - else - Driver->addBuffer(std::move(MBOrErr.first)); + fatal(MBOrErr.second, "could not open " + PathStr); + Driver->addBuffer(std::move(MBOrErr.first)); }); if (Config->OutputFile == "") @@ -157,14 +155,12 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName, } InputFile *Obj; - if (Magic == file_magic::coff_object) { + if (Magic == file_magic::coff_object) Obj = make(MB); - } else if (Magic == file_magic::bitcode) { + else if (Magic == file_magic::bitcode) Obj = make(MB); - } else { - error("unknown file type: " + MB.getBufferIdentifier()); - return; - } + else + fatal("unknown file type: " + MB.getBufferIdentifier()); Obj->ParentName = ParentName; Symtab.addFile(Obj); @@ -242,7 +238,7 @@ void LinkerDriver::parseDirectives(StringRef S) { case OPT_throwingnew: break; default: - error(Arg->getSpelling() + " is not allowed in .drectve"); + fatal(Arg->getSpelling() + " is not allowed in .drectve"); } } } @@ -460,15 +456,6 @@ void LinkerDriver::link(ArrayRef ArgsArr) { // 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]); @@ -527,9 +514,8 @@ void LinkerDriver::link(ArrayRef ArgsArr) { // Handle /noentry if (Args.hasArg(OPT_noentry)) { if (!Args.hasArg(OPT_dll)) - error("/noentry must be specified with /dll"); - else - Config->NoEntry = true; + fatal("/noentry must be specified with /dll"); + Config->NoEntry = true; } // Handle /dll @@ -540,12 +526,10 @@ void LinkerDriver::link(ArrayRef ArgsArr) { // Handle /fixed if (Args.hasArg(OPT_fixed)) { - if (Args.hasArg(OPT_dynamicbase)) { - error("/fixed must not be specified with /dynamicbase"); - } else { - Config->Relocatable = false; - Config->DynamicBase = false; - } + if (Args.hasArg(OPT_dynamicbase)) + fatal("/fixed must not be specified with /dynamicbase"); + Config->Relocatable = false; + Config->DynamicBase = false; } // Handle /machine @@ -617,24 +601,24 @@ void LinkerDriver::link(ArrayRef ArgsArr) { StringRef OptLevel = StringRef(S).substr(7); if (OptLevel.getAsInteger(10, Config->LTOOptLevel) || Config->LTOOptLevel > 3) - error("/opt:lldlto: invalid optimization level: " + OptLevel); + fatal("/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) - error("/opt:lldltojobs: invalid job count: " + Jobs); + fatal("/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) - error("/opt:lldltopartitions: invalid partition count: " + N); + fatal("/opt:lldltopartitions: invalid partition count: " + N); continue; } if (S != "ref" && S != "lbr" && S != "nolbr") - error("/opt: unknown option: " + S); + fatal("/opt: unknown option: " + S); } } @@ -702,9 +686,6 @@ void LinkerDriver::link(ArrayRef ArgsArr) { if (Optional 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()); @@ -749,13 +730,11 @@ void LinkerDriver::link(ArrayRef ArgsArr) { // 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()) { - error("entry point must be defined"); - } else { - Config->Entry = addUndefined(S); - if (Config->Verbose) - outs() << "Entry name inferred: " << S << "\n"; - } + if (S.empty()) + fatal("entry point must be defined"); + Config->Entry = addUndefined(S); + if (Config->Verbose) + outs() << "Entry name inferred: " << S << "\n"; } // Handle /export @@ -840,9 +819,6 @@ void LinkerDriver::link(ArrayRef ArgsArr) { 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)) { @@ -868,13 +844,10 @@ void LinkerDriver::link(ArrayRef ArgsArr) { } // Handle /safeseh. - if (Args.hasArg(OPT_safeseh)) { + if (Args.hasArg(OPT_safeseh)) for (ObjectFile *File : Symtab.ObjectFiles) if (!File->SEHCompat) - error("/safeseh: " + File->getName() + " is not compatible with SEH"); - if (ErrorCount) - return; - } + fatal("/safeseh: " + File->getName() + " is not compatible with SEH"); // Windows specific -- when we are creating a .dll file, we also // need to create a .lib file. diff --git a/lld/COFF/Error.cpp b/lld/COFF/Error.cpp index fca99560a1f7..680c18784ea8 100644 --- a/lld/COFF/Error.cpp +++ b/lld/COFF/Error.cpp @@ -59,13 +59,12 @@ void error(const Twine &Msg) { std::lock_guard 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 /ERRORLIMIT:0 to see all errors)\n"; + << " (use -error-limit=0 to see all errors)\n"; exitLld(1); } diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td index 56f0e6fe857a..73f43790afa0 100644 --- a/lld/COFF/Options.td +++ b/lld/COFF/Options.td @@ -21,8 +21,6 @@ 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", "">; diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp index 0a536f4d57a3..d16e57cd58a9 100644 --- a/lld/COFF/SymbolTable.cpp +++ b/lld/COFF/SymbolTable.cpp @@ -193,7 +193,7 @@ void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol Sym) { } void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) { - error("duplicate symbol: " + toString(*Existing->body()) + " in " + + fatal("duplicate symbol: " + toString(*Existing->body()) + " in " + toString(Existing->body()->getFile()) + " and in " + (NewFile ? toString(NewFile) : "(internal)")); } diff --git a/lld/test/COFF/error-limit.test b/lld/test/COFF/error-limit.test deleted file mode 100644 index 5792608c7937..000000000000 --- a/lld/test/COFF/error-limit.test +++ /dev/null @@ -1,33 +0,0 @@ -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