[lld] r295507 - [COFF] support /ERRORLIMIT option

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 18 18:37:58 PST 2017


Reverted in r295507:

    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)

On Fri, Feb 17, 2017 at 2:57 PM Bob Haarman via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: inglorion
> Date: Fri Feb 17 16:46:06 2017
> New Revision: 295507
>
> URL: http://llvm.org/viewvc/llvm-project?rev=295507&view=rev
> Log:
> [COFF] support /ERRORLIMIT option
>
> 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.
>
> Reviewers: pcc, ruiu
>
> Reviewed By: ruiu
>
> Differential Revision: https://reviews.llvm.org/D29691
>
> Added:
>     lld/trunk/test/COFF/error-limit.test
> Modified:
>     lld/trunk/COFF/Driver.cpp
>     lld/trunk/COFF/Error.cpp
>     lld/trunk/COFF/Options.td
>     lld/trunk/COFF/SymbolTable.cpp
>
> Modified: lld/trunk/COFF/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.cpp?rev=295507&r1=295506&r2=295507&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/Driver.cpp (original)
> +++ lld/trunk/COFF/Driver.cpp Fri Feb 17 16:46:06 2017
> @@ -63,7 +63,7 @@ bool link(ArrayRef<const char *> Args, r
>        (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".
> @@ -126,9 +126,10 @@ void LinkerDriver::addBuffer(std::unique
>    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");
> -  Symtab.addFile(make<ObjectFile>(MBRef));
> +  else
> +    Symtab.addFile(make<ObjectFile>(MBRef));
>  }
>
>  void LinkerDriver::enqueuePath(StringRef Path) {
> @@ -138,8 +139,9 @@ void LinkerDriver::enqueuePath(StringRef
>    enqueueTask([=]() {
>      auto MBOrErr = Future->get();
>      if (MBOrErr.second)
> -      fatal(MBOrErr.second, "could not open " + PathStr);
> -    Driver->addBuffer(std::move(MBOrErr.first));
> +      error("could not open " + PathStr + ": " +
> MBOrErr.second.message());
> +    else
> +      Driver->addBuffer(std::move(MBOrErr.first));
>    });
>
>    if (Config->OutputFile == "")
> @@ -155,12 +157,14 @@ void LinkerDriver::addArchiveBuffer(Memo
>    }
>
>    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);
> @@ -238,7 +242,7 @@ void LinkerDriver::parseDirectives(Strin
>      case OPT_throwingnew:
>        break;
>      default:
> -      fatal(Arg->getSpelling() + " is not allowed in .drectve");
> +      error(Arg->getSpelling() + " is not allowed in .drectve");
>      }
>    }
>  }
> @@ -456,6 +460,15 @@ void LinkerDriver::link(ArrayRef<const c
>    // 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]);
> @@ -514,8 +527,9 @@ void LinkerDriver::link(ArrayRef<const c
>    // Handle /noentry
>    if (Args.hasArg(OPT_noentry)) {
>      if (!Args.hasArg(OPT_dll))
> -      fatal("/noentry must be specified with /dll");
> -    Config->NoEntry = true;
> +      error("/noentry must be specified with /dll");
> +    else
> +      Config->NoEntry = true;
>    }
>
>    // Handle /dll
> @@ -526,10 +540,12 @@ void LinkerDriver::link(ArrayRef<const c
>
>    // Handle /fixed
>    if (Args.hasArg(OPT_fixed)) {
> -    if (Args.hasArg(OPT_dynamicbase))
> -      fatal("/fixed must not be specified with /dynamicbase");
> -    Config->Relocatable = false;
> -    Config->DynamicBase = false;
> +    if (Args.hasArg(OPT_dynamicbase)) {
> +      error("/fixed must not be specified with /dynamicbase");
> +    } else {
> +      Config->Relocatable = false;
> +      Config->DynamicBase = false;
> +    }
>    }
>
>    // Handle /machine
> @@ -601,24 +617,24 @@ void LinkerDriver::link(ArrayRef<const c
>          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);
>      }
>    }
>
> @@ -686,6 +702,9 @@ void LinkerDriver::link(ArrayRef<const c
>      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());
> @@ -730,11 +749,13 @@ void LinkerDriver::link(ArrayRef<const c
>      // 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");
> -    Config->Entry = addUndefined(S);
> -    if (Config->Verbose)
> -      outs() << "Entry name inferred: " << S << "\n";
> +    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
> @@ -819,6 +840,9 @@ void LinkerDriver::link(ArrayRef<const c
>        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)) {
> @@ -844,10 +868,13 @@ void LinkerDriver::link(ArrayRef<const c
>    }
>
>    // 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.
>
> Modified: lld/trunk/COFF/Error.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Error.cpp?rev=295507&r1=295506&r2=295507&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/Error.cpp (original)
> +++ lld/trunk/COFF/Error.cpp Fri Feb 17 16:46:06 2017
> @@ -59,12 +59,13 @@ 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);
>    }
>
>
> Modified: lld/trunk/COFF/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Options.td?rev=295507&r1=295506&r2=295507&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/Options.td (original)
> +++ lld/trunk/COFF/Options.td Fri Feb 17 16:46:06 2017
> @@ -21,6 +21,8 @@ def base    : P<"base", "Base address of
>  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", "">;
>
> Modified: lld/trunk/COFF/SymbolTable.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=295507&r1=295506&r2=295507&view=diff
>
> ==============================================================================
> --- lld/trunk/COFF/SymbolTable.cpp (original)
> +++ lld/trunk/COFF/SymbolTable.cpp Fri Feb 17 16:46:06 2017
> @@ -193,7 +193,7 @@ void SymbolTable::addLazy(ArchiveFile *F
>  }
>
>  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)"));
>  }
>
> Added: lld/trunk/test/COFF/error-limit.test
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/error-limit.test?rev=295507&view=auto
>
> ==============================================================================
> --- lld/trunk/test/COFF/error-limit.test (added)
> +++ lld/trunk/test/COFF/error-limit.test Fri Feb 17 16:46:06 2017
> @@ -0,0 +1,33 @@
> +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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170219/3f91207f/attachment-0001.html>


More information about the llvm-commits mailing list