Index: clang/trunk/include/clang/Driver/CLCompatOptions.td =================================================================== --- clang/trunk/include/clang/Driver/CLCompatOptions.td +++ clang/trunk/include/clang/Driver/CLCompatOptions.td @@ -315,6 +315,8 @@ def _SLASH_Gregcall : CLFlag<"Gregcall">, HelpText<"Set __regcall as a default calling convention">; +def _SLASH_MP : CLCompileJoined<"MP">, HelpText<"Build with multiple processes">; + // Ignored: def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">; @@ -382,7 +384,6 @@ def _SLASH_JMC : CLFlag<"JMC">; def _SLASH_kernel : CLFlag<"kernel">; def _SLASH_LN : CLFlag<"LN">; -def _SLASH_MP : CLJoined<"MP">; def _SLASH_openmp : CLFlag<"openmp">; def _SLASH_Qfast_transcendentals : CLFlag<"Qfast_transcendentals">; def _SLASH_QIfist : CLFlag<"QIfist">; Index: clang/trunk/include/clang/Driver/Compilation.h =================================================================== --- clang/trunk/include/clang/Driver/Compilation.h +++ clang/trunk/include/clang/Driver/Compilation.h @@ -126,6 +126,9 @@ bool ForceKeepTempFiles = false; public: + /// Number of processors/cores to use for compiling the jobs + unsigned CoresToUse = 1; + Compilation(const Driver &D, const ToolChain &DefaultToolChain, llvm::opt::InputArgList *Args, llvm::opt::DerivedArgList *TranslatedArgs, bool ContainsError); @@ -277,15 +280,21 @@ /// \param FailingCommand - For non-zero results, this will be set to the /// Command which failed, if any. /// \return The result code of the subprocess. - int ExecuteCommand(const Command &C, const Command *&FailingCommand) const; + //int ExecuteCommand(const Command &C, const Command *&FailingCommand) const; + + JobExecutionState ExecuteCommandAsync(const Command &C) const; /// ExecuteJob - Execute a single job. /// /// \param FailingCommands - For non-zero results, this will be a vector of /// failing commands and their associated result code. - void ExecuteJobs( + void executeJobs( const JobList &Jobs, - SmallVectorImpl> &FailingCommands) const; + SmallVectorImpl &FailingCommands) const; + + void waitOnJobs(SmallVectorImpl &ActiveJobs) const; + + void terminateAllJobs(SmallVectorImpl &ActiveJobs) const; /// initCompilationForDiagnostics - Remove stale state and suppress output /// so compilation can be reexecuted to generate additional diagnostic Index: clang/trunk/include/clang/Driver/Driver.h =================================================================== --- clang/trunk/include/clang/Driver/Driver.h +++ clang/trunk/include/clang/Driver/Driver.h @@ -13,6 +13,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/LLVM.h" #include "clang/Driver/Action.h" +#include "clang/Driver/Job.h" #include "clang/Driver/Phases.h" #include "clang/Driver/ToolChain.h" #include "clang/Driver/Types.h" @@ -262,7 +263,7 @@ llvm::opt::Arg **FinalPhaseArg = nullptr) const; // Before executing jobs, sets up response files for commands that need them. - void setUpResponseFiles(Compilation &C, Command &Cmd); + void setUpResponseFiles(Compilation &C, Command &Cmd) const; void generatePrefixedToolNames(StringRef Tool, const ToolChain &TC, SmallVectorImpl &Names) const; @@ -405,7 +406,7 @@ /// to just running the subprocesses, for example reporting errors, setting /// up response files, removing temporary files, etc. int ExecuteCompilation(Compilation &C, - SmallVectorImpl< std::pair > &FailingCommands); + SmallVectorImpl &FailingCommands); /// Contains the files in the compilation diagnostic report generated by /// generateCompilationDiagnostics. Index: clang/trunk/include/clang/Driver/Job.h =================================================================== --- clang/trunk/include/clang/Driver/Job.h +++ clang/trunk/include/clang/Driver/Job.h @@ -17,6 +17,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator.h" #include "llvm/Option/Option.h" +#include "llvm/Support/Program.h" #include #include #include @@ -26,6 +27,7 @@ namespace driver { class Action; +class Command; class InputInfo; class Tool; @@ -40,6 +42,14 @@ : Filename(Filename), VFSPath(VFSPath) {} }; +struct JobExecutionState { + bool isFree() const { return !C; } + + JobExecutionState() : C() {} + const Command *C; + llvm::sys::ProcessInfo P; +}; + /// Command - An executable path/name and argument vector to /// execute. class Command { @@ -98,8 +108,11 @@ virtual void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote, CrashReportInfo *CrashInfo = nullptr) const; - virtual int Execute(ArrayRef> Redirects, - std::string *ErrMsg, bool *ExecutionFailed) const; + virtual bool Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const; + + JobExecutionState + ExecuteAsync(ArrayRef> Redirects) const; /// getSource - Return the Action which caused the creation of this job. const Action &getSource() const { return Source; } @@ -142,8 +155,8 @@ void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote, CrashReportInfo *CrashInfo = nullptr) const override; - int Execute(ArrayRef> Redirects, std::string *ErrMsg, - bool *ExecutionFailed) const override; + bool Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const override; private: std::unique_ptr Fallback; @@ -159,8 +172,8 @@ void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote, CrashReportInfo *CrashInfo = nullptr) const override; - int Execute(ArrayRef> Redirects, std::string *ErrMsg, - bool *ExecutionFailed) const override; + bool Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const override; }; /// JobList - A sequence of jobs to perform. Index: clang/trunk/lib/Driver/Compilation.cpp =================================================================== --- clang/trunk/lib/Driver/Compilation.cpp +++ clang/trunk/lib/Driver/Compilation.cpp @@ -149,10 +149,12 @@ return Success; } -int Compilation::ExecuteCommand(const Command &C, - const Command *&FailingCommand) const { - if ((getDriver().CCPrintOptions || - getArgs().hasArg(options::OPT_v)) && !getDriver().CCGenDiagnostics) { +JobExecutionState Compilation::ExecuteCommandAsync(const Command &C) const { + JobExecutionState S; + S.C = &C; + + if ((getDriver().CCPrintOptions || getArgs().hasArg(options::OPT_v)) && + !getDriver().CCGenDiagnostics) { raw_ostream *OS = &llvm::errs(); // Follow gcc implementation of CC_PRINT_OPTIONS; we could also cache the @@ -165,9 +167,10 @@ if (EC) { getDriver().Diag(diag::err_drv_cc_print_options_failure) << EC.message(); - FailingCommand = &C; delete OS; - return 1; + + S.P.ReturnCode = 1; + return S; } } @@ -180,21 +183,38 @@ delete OS; } - std::string Error; - bool ExecutionFailed; - int Res = C.Execute(Redirects, &Error, &ExecutionFailed); - if (!Error.empty()) { - assert(Res && "Error string set with 0 result code!"); - getDriver().Diag(diag::err_drv_command_failure) << Error; + S = C.ExecuteAsync(Redirects); + return S; +} + +void Compilation::waitOnJobs( + SmallVectorImpl &ActiveJobs) const { + SmallVector Waitable; + Waitable.reserve(ActiveJobs.size()); + + for (auto &S : ActiveJobs) { + if (!S.P.running()) + continue; + Waitable.push_back(&S.P); } + llvm::sys::WaitMany(Waitable, /*WaitOnAll*/ false, 0, + /*WaitUntilChildTerminates*/ true); +} - if (Res) - FailingCommand = &C; +void Compilation::terminateAllJobs( + SmallVectorImpl &ActiveJobs) const { + SmallVector Waitable; + Waitable.reserve(ActiveJobs.size()); - return ExecutionFailed ? 1 : Res; + for (auto &S : ActiveJobs) { + if (S.isFree() || S.P.completedSuccess()) + continue; + Waitable.push_back(&S.P); + } + llvm::sys::Terminate(Waitable); } -using FailingCommandList = SmallVectorImpl>; +using FailingCommandList = SmallVectorImpl; static bool ActionFailed(const Action *A, const FailingCommandList &FailingCommands) { @@ -207,8 +227,8 @@ if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP)) return true; - for (const auto &CI : FailingCommands) - if (A == &(CI.second->getSource())) + for (const auto &S : FailingCommands) + if (A == &(S.C->getSource())) return true; for (const auto *AI : A->inputs()) @@ -223,21 +243,88 @@ return !ActionFailed(&C.getSource(), FailingCommands); } -void Compilation::ExecuteJobs(const JobList &Jobs, - FailingCommandList &FailingCommands) const { - // According to UNIX standard, driver need to continue compiling all the - // inputs on the command line even one of them failed. - // In all but CLMode, execute all the jobs unless the necessary inputs for the - // job is missing due to previous failures. - for (const auto &Job : Jobs) { - if (!InputsOk(Job, FailingCommands)) +static bool pruneCompletedJobs(SmallVectorImpl &ActiveJobs, + int &JobsDone, + FailingCommandList &FailingCommands) { + bool HasErrors = false; + for (auto &S : ActiveJobs) { + if (S.isFree() || S.P.running()) continue; - const Command *FailingCommand = nullptr; - if (int Res = ExecuteCommand(Job, FailingCommand)) { - FailingCommands.push_back(std::make_pair(Res, FailingCommand)); - // Bail as soon as one command fails in cl driver mode. - if (TheDriver.IsCLMode()) - return; + + ++JobsDone; + + if (S.P.completedError()) { + FailingCommands.push_back(S); + HasErrors = true; + } + + // Free slot + S = JobExecutionState(); + } + return HasErrors; +} + +void Compilation::executeJobs(const JobList &Jobs, + FailingCommandList &FailingCommands) const { + + // Allocate one slot for each processor/core + SmallVector ActiveJobs( + std::min(CoresToUse, (unsigned)Jobs.size())); + + int JobsDone = 0; + auto NextJob = Jobs.begin(); + + // The job allocation algorithm works as following, on each loop: + // 1. Fill up all processing slots (one per core) by spawning a new + // compilation job on each core + // 2. Wait on all cores, until at least one job finishes (waiting with + // system-level primitives) + // 3. Check all completed jobs for failures + // 4. Loop until the job list is exhausted + while (JobsDone < Jobs.size()) { + + // Spawn new jobs on inactive processor cores + for (auto &S : ActiveJobs) { + if (!S.isFree()) + continue; + + if (NextJob == Jobs.end()) + break; + + Command &C = *NextJob++; + // According to UNIX standard, the driver needs to continue compiling + // all the inputs on the command line even one of them failed. In all + // but CLMode, execute all the jobs unless the necessary inputs for the + // job are missing due to previous failures. + if (!InputsOk(C, FailingCommands)) { + ++JobsDone; + continue; + } + // Start new compilation job + S = ExecuteCommandAsync(C); + } + + // Spin on active jobs with OS primitives, until one of them finishes + waitOnJobs(ActiveJobs); + + // Complete job processing once finished + bool HasErrors = pruneCompletedJobs(ActiveJobs, JobsDone, FailingCommands); + + // Bail as soon as one command fails in cl driver mode. + if (HasErrors && TheDriver.IsCLMode()) { + terminateAllJobs(ActiveJobs); + + // Ensure any killed job is queued as 'FailingCommands', to clean up + // temporary files + for (auto &S : ActiveJobs) { + if (S.isFree()) + continue; + assert(!S.P.running()); + + if (S.P.completedError()) + FailingCommands.push_back(S); + } + return; } } } Index: clang/trunk/lib/Driver/Driver.cpp =================================================================== --- clang/trunk/lib/Driver/Driver.cpp +++ clang/trunk/lib/Driver/Driver.cpp @@ -74,6 +74,7 @@ #include "llvm/Support/Program.h" #include "llvm/Support/StringSaver.h" #include "llvm/Support/TargetRegistry.h" +#include "llvm/Support/Threading.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -1249,8 +1250,8 @@ } // Generate preprocessed output. - SmallVector, 4> FailingCommands; - C.ExecuteJobs(C.getJobs(), FailingCommands); + SmallVector FailingCommands; + C.executeJobs(C.getJobs(), FailingCommands); // If any of the preprocessing commands failed, clean up and exit. if (!FailingCommands.empty()) { @@ -1340,7 +1341,7 @@ << "\n\n********************"; } -void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) { +void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) const { // Since commandLineFitsWithinSystemLimits() may underestimate system's capacity // if the tool does not support response files, there is a chance/ that things // will just work without a response file, so we silently just skip it. @@ -1354,32 +1355,32 @@ int Driver::ExecuteCompilation( Compilation &C, - SmallVectorImpl> &FailingCommands) { + SmallVectorImpl &FailingCommands) { // Just print if -### was present. if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) { C.getJobs().Print(llvm::errs(), "\n", true); - return 0; + return (int)llvm::sys::ProgramReturnCode::Success; } // If there were errors building the compilation, quit now. if (Diags.hasErrorOccurred()) - return 1; + return (int)llvm::sys::ProgramReturnCode::Failure; // Set up response file names for each command, if necessary for (auto &Job : C.getJobs()) setUpResponseFiles(C, Job); - C.ExecuteJobs(C.getJobs(), FailingCommands); + C.executeJobs(C.getJobs(), FailingCommands); // If the command succeeded, we are done. if (FailingCommands.empty()) - return 0; + return (int)llvm::sys::ProgramReturnCode::Success; // Otherwise, remove result files and print extra information about abnormal // failures. - for (const auto &CmdPair : FailingCommands) { - int Res = CmdPair.first; - const Command *FailingCommand = CmdPair.second; + Optional Res; + for (const auto &S : FailingCommands) { + const Command *FailingCommand = S.C; // Remove result files if we're not saving temps. if (!isSaveTempsEnabled()) { @@ -1387,7 +1388,7 @@ C.CleanupFileMap(C.getResultFiles(), JA, true); // Failure result files are valid unless we crashed. - if (Res < 0) + if (S.P.notCompletedOrFailed()) C.CleanupFileMap(C.getFailureResultFiles(), JA, true); } @@ -1400,17 +1401,23 @@ // diagnostics, so always print the diagnostic there. const Tool &FailingTool = FailingCommand->getCreator(); - if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) { - // FIXME: See FIXME above regarding result code interpretation. - if (Res < 0) + if (!FailingCommand->getCreator().hasGoodDiagnostics()) { + if (S.P.notCompletedOrFailed()) Diag(clang::diag::err_drv_command_signalled) << FailingTool.getShortName(); - else - Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName() - << Res; + else if (S.P.completedError() && + S.P.ReturnCode.getValue() != (int)llvm::sys::ProgramReturnCode::Failure) { + if (S.P.ErrMsg.hasValue()) + Diag(diag::err_drv_command_failure) + << FailingTool.getShortName() << S.P.ErrMsg.getValue(); + else + Diag(clang::diag::err_drv_command_failed) + << FailingTool.getShortName() << S.P.ReturnCode.getValue(); + } } + Res = S.P.convertToParentReturnCode(); } - return 0; + return Res.getValueOr((int)llvm::sys::ProgramReturnCode::Success); } void Driver::PrintHelp(bool ShowHidden) const { @@ -3018,6 +3025,12 @@ Args.eraseArg(options::OPT__SLASH_Yu); YcArg = YuArg = nullptr; } + if (FinalPhase != phases::Preprocess) { + if (Arg *A = Args.getLastArg(options::OPT__SLASH_MP)) { + C.CoresToUse = llvm::hardware_concurrency(); + StringRef(A->getValue()).getAsInteger(10, C.CoresToUse); + } + } // Builder to be used to build offloading actions. OffloadingActionBuilder OffloadBuilder(C, Args, Inputs); Index: clang/trunk/lib/Driver/Job.cpp =================================================================== --- clang/trunk/lib/Driver/Job.cpp +++ clang/trunk/lib/Driver/Job.cpp @@ -313,9 +313,9 @@ Environment.push_back(nullptr); } -int Command::Execute(ArrayRef> Redirects, - std::string *ErrMsg, bool *ExecutionFailed) const { - SmallVector Argv; +JobExecutionState +Command::ExecuteAsync(ArrayRef> Redirects) const { + SmallVector Argv; Optional> Env; std::vector ArgvVectorStorage; @@ -326,42 +326,47 @@ Env = makeArrayRef(ArgvVectorStorage); } + JobExecutionState S; + S.C = this; + if (ResponseFile == nullptr) { Argv.push_back(Executable); Argv.append(Arguments.begin(), Arguments.end()); Argv.push_back(nullptr); + } else { + // We need to put arguments in a response file (command is too large) + // Open stream to store the response file contents + std::string RespContents; + llvm::raw_string_ostream SS(RespContents); + + // Write file contents and build the Argv vector + writeResponseFile(SS); + buildArgvForResponseFile(Argv); + Argv.push_back(nullptr); + SS.flush(); - auto Args = llvm::toStringRefArray(Argv.data()); - return llvm::sys::ExecuteAndWait( - Executable, Args, Env, Redirects, /*secondsToWait*/ 0, - /*memoryLimit*/ 0, ErrMsg, ExecutionFailed); - } - - // We need to put arguments in a response file (command is too large) - // Open stream to store the response file contents - std::string RespContents; - llvm::raw_string_ostream SS(RespContents); - - // Write file contents and build the Argv vector - writeResponseFile(SS); - buildArgvForResponseFile(Argv); - Argv.push_back(nullptr); - SS.flush(); - - // Save the response file in the appropriate encoding - if (std::error_code EC = writeFileWithEncoding( - ResponseFile, RespContents, Creator.getResponseFileEncoding())) { - if (ErrMsg) - *ErrMsg = EC.message(); - if (ExecutionFailed) - *ExecutionFailed = true; - return -1; + // Save the response file in the appropriate encoding + if (std::error_code EC = writeFileWithEncoding( + ResponseFile, RespContents, Creator.getResponseFileEncoding())) { + S.P.ReturnCode = -1; + S.P.ErrMsg = toString(createFileError(ResponseFile, llvm::errorCodeToError(EC))); + return S; + } } auto Args = llvm::toStringRefArray(Argv.data()); - return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects, - /*secondsToWait*/ 0, - /*memoryLimit*/ 0, ErrMsg, ExecutionFailed); + + S.P = llvm::sys::ExecuteNoWait(Executable, Args, Env, Redirects, + /*memoryLimit*/ 0); + return S; +} + +bool Command::Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const { + auto S = ExecuteAsync(Redirects); + bool R = llvm::sys::Wait(S.P, 0, /*WaitUntilChildTerminates*/ true); + PI = S.P; + return R; } FallbackCommand::FallbackCommand(const Action &Source_, const Tool &Creator_, @@ -379,30 +384,26 @@ Fallback->Print(OS, Terminator, Quote, CrashInfo); } -static bool ShouldFallback(int ExitCode) { +static bool ShouldFallback(llvm::sys::ProcessInfo &PI) { // FIXME: We really just want to fall back for internal errors, such // as when some symbol cannot be mangled, when we should be able to // parse something but can't, etc. - return ExitCode != 0; + return PI.completedError(); } -int FallbackCommand::Execute(ArrayRef> Redirects, - std::string *ErrMsg, bool *ExecutionFailed) const { - int PrimaryStatus = Command::Execute(Redirects, ErrMsg, ExecutionFailed); - if (!ShouldFallback(PrimaryStatus)) - return PrimaryStatus; - - // Clear ExecutionFailed and ErrMsg before falling back. - if (ErrMsg) - ErrMsg->clear(); - if (ExecutionFailed) - *ExecutionFailed = false; +bool FallbackCommand::Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const { + Command::Execute(Redirects, PI); + if (!ShouldFallback(PI)) + return false; const Driver &D = getCreator().getToolChain().getDriver(); D.Diag(diag::warn_drv_invoking_fallback) << Fallback->getExecutable(); - int SecondaryStatus = Fallback->Execute(Redirects, ErrMsg, ExecutionFailed); - return SecondaryStatus; + PI.clear(); + + Fallback->Execute(Redirects, PI); + return PI.completedSuccess(); } ForceSuccessCommand::ForceSuccessCommand(const Action &Source_, @@ -418,14 +419,10 @@ OS << " || (exit 0)" << Terminator; } -int ForceSuccessCommand::Execute(ArrayRef> Redirects, - std::string *ErrMsg, - bool *ExecutionFailed) const { - int Status = Command::Execute(Redirects, ErrMsg, ExecutionFailed); - (void)Status; - if (ExecutionFailed) - *ExecutionFailed = false; - return 0; +bool ForceSuccessCommand::Execute(ArrayRef> Redirects, + llvm::sys::ProcessInfo &PI) const { + Command::Execute(Redirects, PI); + return true; } void JobList::Print(raw_ostream &OS, const char *Terminator, bool Quote, Index: clang/trunk/tools/driver/cc1_main.cpp =================================================================== --- clang/trunk/tools/driver/cc1_main.cpp +++ clang/trunk/tools/driver/cc1_main.cpp @@ -34,6 +34,7 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/ManagedStatic.h" +#include "llvm/Support/Program.h" #include "llvm/Support/Signals.h" #include "llvm/Support/TargetSelect.h" #include "llvm/Support/Timer.h" @@ -64,7 +65,8 @@ // We cannot recover from llvm errors. When reporting a fatal error, exit // with status 70 to generate crash diagnostics. For BSD systems this is // defined as an internal software error. Otherwise, exit with status 1. - exit(GenCrashDiag ? 70 : 1); + exit(int(GenCrashDiag ? llvm::sys::ProgramReturnCode::GenerateCrashDiagnostic + : llvm::sys::ProgramReturnCode::Failure)); } #ifdef LINK_POLLY_INTO_TOOLS @@ -164,7 +166,7 @@ static void ensureSufficientStack() {} #endif -int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) { +bool cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) { ensureSufficientStack(); std::unique_ptr Clang(new CompilerInstance()); @@ -203,7 +205,7 @@ // Create the actual diagnostics engine. Clang->createDiagnostics(); if (!Clang->hasDiagnostics()) - return 1; + return false; // Set an error handler, so that any LLVM backend diagnostics go through our // error handler. @@ -212,7 +214,7 @@ DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics()); if (!Success) - return 1; + return false; // Execute the frontend actions. Success = ExecuteCompilerInvocation(Clang.get()); @@ -229,8 +231,6 @@ // When running with -disable-free, don't do any destruction or shutdown. if (Clang->getFrontendOpts().DisableFree) { BuryPointer(std::move(Clang)); - return !Success; } - - return !Success; + return Success; } Index: clang/trunk/tools/driver/cc1as_main.cpp =================================================================== --- clang/trunk/tools/driver/cc1as_main.cpp +++ clang/trunk/tools/driver/cc1as_main.cpp @@ -508,7 +508,7 @@ exit(1); } -int cc1as_main(ArrayRef Argv, const char *Argv0, void *MainAddr) { +bool cc1as_main(ArrayRef Argv, const char *Argv0, void *MainAddr) { // Initialize targets and assembly printers/parsers. InitializeAllTargetInfos(); InitializeAllTargetMCs(); @@ -530,14 +530,14 @@ // Parse the arguments. AssemblerInvocation Asm; if (!AssemblerInvocation::CreateFromArgs(Asm, Argv, Diags)) - return 1; + return false; if (Asm.ShowHelp) { std::unique_ptr Opts(driver::createDriverOptTable()); Opts->PrintHelp(llvm::outs(), "clang -cc1as", "Clang Integrated Assembler", /*Include=*/driver::options::CC1AsOption, /*Exclude=*/0, /*ShowAllAliases=*/false); - return 0; + return true; } // Honor -version. @@ -545,7 +545,7 @@ // FIXME: Use a better -version message? if (Asm.ShowVersion) { llvm::cl::PrintVersionMessage(); - return 0; + return true; } // Honor -mllvm. @@ -568,5 +568,5 @@ // results now. TimerGroup::printAll(errs()); - return !!Failed; + return !Failed; } Index: clang/trunk/tools/driver/cc1gen_reproducer_main.cpp =================================================================== --- clang/trunk/tools/driver/cc1gen_reproducer_main.cpp +++ clang/trunk/tools/driver/cc1gen_reproducer_main.cpp @@ -153,11 +153,11 @@ OS << "]\n}\n"; } -int cc1gen_reproducer_main(ArrayRef Argv, const char *Argv0, +bool cc1gen_reproducer_main(ArrayRef Argv, const char *Argv0, void *MainAddr) { if (Argv.size() < 1) { llvm::errs() << "error: missing invocation file\n"; - return 1; + return false; } // Parse the invocation descriptor. StringRef Input = Argv[0]; @@ -166,7 +166,7 @@ if (!Buffer) { llvm::errs() << "error: failed to read " << Input << ": " << Buffer.getError().message() << "\n"; - return 1; + return false; } llvm::yaml::Input YAML(Buffer.get()->getBuffer()); ClangInvocationInfo InvocationInfo; @@ -184,10 +184,10 @@ generateReproducerForInvocationArguments(DriverArgs, InvocationInfo); // Emit the information about the reproduce files to stdout. - int Result = 1; + int Result = false; if (Report) { printReproducerInformation(llvm::outs(), InvocationInfo, *Report); - Result = 0; + Result = true; } // Remove the input file. Index: clang/trunk/tools/driver/driver.cpp =================================================================== --- clang/trunk/tools/driver/driver.cpp +++ clang/trunk/tools/driver/driver.cpp @@ -199,11 +199,11 @@ } } -extern int cc1_main(ArrayRef Argv, const char *Argv0, +extern bool cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr); -extern int cc1as_main(ArrayRef Argv, const char *Argv0, +extern bool cc1as_main(ArrayRef Argv, const char *Argv0, void *MainAddr); -extern int cc1gen_reproducer_main(ArrayRef Argv, +extern bool cc1gen_reproducer_main(ArrayRef Argv, const char *Argv0, void *MainAddr); static void insertTargetAndModeArgs(const ParsedClangName &NameParts, @@ -304,7 +304,7 @@ TheDriver.setInstalledDir(InstalledPathParent); } -static int ExecuteCC1Tool(ArrayRef argv, StringRef Tool) { +static bool ExecuteCC1Tool(ArrayRef argv, StringRef Tool) { void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath; if (Tool == "") return cc1_main(argv.slice(2), argv[0], GetExecutablePathVP); @@ -316,7 +316,7 @@ // Reject unknown tools. llvm::errs() << "error: unknown integrated tool '" << Tool << "'. " << "Valid tools include '-cc1' and '-cc1as'.\n"; - return 1; + return false; } int main(int argc_, const char **argv_) { @@ -324,7 +324,7 @@ SmallVector argv(argv_, argv_ + argc_); if (llvm::sys::Process::FixupStandardFileDescriptors()) - return 1; + return (int)llvm::sys::ProgramReturnCode::Failure; llvm::InitializeAllTargets(); auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]); @@ -379,7 +379,9 @@ auto newEnd = std::remove(argv.begin(), argv.end(), nullptr); argv.resize(newEnd - argv.begin()); } - return ExecuteCC1Tool(argv, argv[1] + 4); + bool R = ExecuteCC1Tool(argv, argv[1] + 4); + return int(R ? llvm::sys::ProgramReturnCode::Success + : llvm::sys::ProgramReturnCode::Failure); } bool CanonicalPrefixes = true; @@ -456,9 +458,9 @@ SetBackdoorDriverOutputsFromEnvVars(TheDriver); std::unique_ptr C(TheDriver.BuildCompilation(argv)); - int Res = 1; + int Res = (int)llvm::sys::ProgramReturnCode::Failure; if (C && !C->containsError()) { - SmallVector, 4> FailingCommands; + SmallVector FailingCommands; Res = TheDriver.ExecuteCompilation(*C, FailingCommands); // Force a crash to test the diagnostics. @@ -466,29 +468,22 @@ Diags.Report(diag::err_drv_force_crash) << !::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"); - // Pretend that every command failed. FailingCommands.clear(); - for (const auto &J : C->getJobs()) - if (const Command *C = dyn_cast(&J)) - FailingCommands.push_back(std::make_pair(-1, C)); + + Command &FirstC = *C->getJobs().begin(); + + JobExecutionState S; + S.C = &FirstC; + S.P.Status = llvm::sys::ExecutionStatus::Crashed; + FailingCommands.push_back(S); } - for (const auto &P : FailingCommands) { - int CommandRes = P.first; - const Command *FailingCommand = P.second; - if (!Res) - Res = CommandRes; - - // If result status is < 0, then the driver command signalled an error. - // If result status is 70, then the driver command reported a fatal error. - // On Windows, abort will return an exit code of 3. In these cases, - // generate additional diagnostic information if possible. - bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70; -#ifdef _WIN32 - DiagnoseCrash |= CommandRes == 3; -#endif - if (DiagnoseCrash) { + for (const auto &S : FailingCommands) { + const Command *FailingCommand = S.C; + + if (S.P.crashed()) { TheDriver.generateCompilationDiagnostics(*C, *FailingCommand); + Res = (int)llvm::sys::ProgramReturnCode::Failure; break; } } @@ -500,14 +495,6 @@ // results now. This happens in -disable-free mode. llvm::TimerGroup::printAll(llvm::errs()); -#ifdef _WIN32 - // Exit status should not be negative on Win32, unless abnormal termination. - // Once abnormal termiation was caught, negative status should not be - // propagated. - if (Res < 0) - Res = 1; -#endif - // If we have multiple failing commands, we return the result of the first // failing command. return Res; Index: llvm/trunk/include/llvm/Support/Program.h =================================================================== --- llvm/trunk/include/llvm/Support/Program.h +++ llvm/trunk/include/llvm/Support/Program.h @@ -24,8 +24,21 @@ namespace llvm { namespace sys { - /// This is the OS-specific separator for PATH like environment variables: - // a colon on Unix or a semicolon on Windows. +/// Some return codes that clang or other LLVM tools can return +enum class ProgramReturnCode { + Success = 0, + Failure = 1, // General program failure. + // Also used by Windows' TaskManager when a program is killed manually by the user. + // Clang also uses this as time-out termination code for its children processes. +#ifdef _WIN32 + Abort = 3, // Returned by abort() on Windows. +#endif + GenerateCrashDiagnostic = 70, // EX_SOFTWARE on BSD, used by Clang to forcibly generate a crash dump + BugpointClientFail = 255 +}; + +/// This is the OS-specific separator for PATH like environment variables: +// a colon on Unix or a semicolon on Windows. #if defined(LLVM_ON_UNIX) const char EnvPathSeparator = ':'; #elif defined (_WIN32) @@ -40,6 +53,24 @@ typedef procid_t process_t; #endif + /// This represents the high-level state of the program, as steps of the + /// following DAG state machine: + /// + /// /--> ExecutionFailure + /// NotStarted ---> StartedAndExecuting ---> CompletedSuccess + /// \--> CompletedWithError + /// \--> Crashed + /// \--> Killed + enum class ExecutionStatus { + NotStarted = 0, + StartedAndExecuting, // process is up and running + CompletedSuccess, // process completed with no error + CompletedWithError, // process completed with error, see ReturnCode + ExecutionFailure, // process can't be started + Crashed, // process crashed + Killed // process was forcibly terminated + }; + /// This struct encapsulates information about a process. struct ProcessInfo { enum : procid_t { InvalidPid = 0 }; @@ -47,10 +78,77 @@ procid_t Pid; /// The process identifier. process_t Process; /// Platform-dependent process object. - /// The return code, set after execution. - int ReturnCode; + /// The program execution status + ExecutionStatus Status; + + /// The return code after execution, only set if the program terminates with + /// an error + Optional ReturnCode; + + /// A possible error message after execution + Optional ErrMsg; ProcessInfo(); + + operator bool() { + return Status == ExecutionStatus::StartedAndExecuting || + Status == ExecutionStatus::CompletedSuccess; + } + bool notStarted() const { + return Status == ExecutionStatus::NotStarted; + } + bool running() const { + return Status == ExecutionStatus::StartedAndExecuting; + } + bool completedError() const { + return Status >= ExecutionStatus::CompletedWithError; + } + bool completedSuccess() const { + return Status == ExecutionStatus::CompletedSuccess; + } + bool notCompletedOrFailed() const { + return Status >= ExecutionStatus::ExecutionFailure; + } + bool crashed(bool ExpectCrash = false) const { + if (Status == ExecutionStatus::Crashed) + return true; + if (!ReturnCode.hasValue()) + return false; + if (ReturnCode.getValue() < 0 || + ReturnCode.getValue() == (int)ProgramReturnCode::GenerateCrashDiagnostic) + return true; +#ifdef _WIN32 + // Handle abort() in msvcrt -- It has exit code as 3. abort(), aka + // unreachable, should be recognized as a crash. However, some binaries use + // exit code 3 on non-crash failure paths, so only do this if we expect a + // crash. + if (ExpectCrash && ReturnCode.getValue() == (int)ProgramReturnCode::Abort) + return true; +#endif + return false; + } + bool killed() const { return Status == ExecutionStatus::Killed; } + int convertToParentReturnCode() const { + assert(Status >= ExecutionStatus::CompletedSuccess && + "Can only be called after program completion."); + if (Status >= ExecutionStatus::ExecutionFailure) + return (int)ProgramReturnCode::Failure; + int R = ReturnCode.getValueOr((int)ProgramReturnCode::Success); +#ifdef _WIN32 + // Exit status should not be negative on Win32, unless abnormal + // termination. Once abnormal termination was caught, negative status + // should not be propagated. + if (R < 0) + return (int)ProgramReturnCode::Failure; +#endif + return R; + } + void clear() { *this = ProcessInfo(); } + bool hasRet(ProgramReturnCode RC) const { + if (!ReturnCode.hasValue()) + return false; + return ReturnCode.getValue() == (int)RC; + } }; /// Find the first executable file \p Name in \p Paths. @@ -82,11 +180,10 @@ /// This function waits for the program to finish, so should be avoided in /// library functions that aren't expected to block. Consider using /// ExecuteNoWait() instead. - /// \returns an integer result code indicating the status of the program. - /// A zero or positive value indicates the result code of the program. - /// -1 indicates failure to execute - /// -2 indicates a crash during execution or timeout - int ExecuteAndWait( + /// \returns true if the program executed with no errors. + /// If false, the program execution failed, check PI->Status, PI->ReturnCode + /// or PI->ErrMsg for further error information. + bool ExecuteAndWait( StringRef Program, ///< Path of the program to be executed. It is ///< presumed this is the result of the findProgramByName method. ArrayRef Args, ///< An array of strings that are passed to the @@ -113,23 +210,17 @@ ///< of memory can be allocated by process. If memory usage will be ///< higher limit, the child is killed and this call returns. If zero ///< - no memory limit. - std::string *ErrMsg = nullptr, ///< If non-zero, provides a pointer to a - ///< string instance in which error messages will be returned. If the - ///< string is non-empty upon return an error occurred while invoking the - ///< program. - bool *ExecutionFailed = nullptr); + ProcessInfo *PI = nullptr); /// Similar to ExecuteAndWait, but returns immediately. - /// @returns The \see ProcessInfo of the newly launced process. + /// @returns The \see ProcessInfo of the newly launched process. /// \note On Microsoft Windows systems, users will need to either call /// \see Wait until the process finished execution or win32 CloseHandle() API /// on ProcessInfo.ProcessHandle to avoid memory leaks. ProcessInfo ExecuteNoWait(StringRef Program, ArrayRef Args, Optional> Env, ArrayRef> Redirects = {}, - unsigned MemoryLimit = 0, - std::string *ErrMsg = nullptr, - bool *ExecutionFailed = nullptr); + unsigned MemoryLimit = 0); /// Return true if the given arguments fit within system-specific /// argument length limits. @@ -176,25 +267,45 @@ WindowsEncodingMethod Encoding = WEM_UTF8); /// This function waits for the process specified by \p PI to finish. - /// \returns A \see ProcessInfo struct with Pid set to: - /// \li The process id of the child process if the child process has changed - /// state. - /// \li 0 if the child process has not changed state. - /// \note Users of this function should always check the ReturnCode member of - /// the \see ProcessInfo returned from this function. - ProcessInfo Wait( - const ProcessInfo &PI, ///< The child process that should be waited on. + /// \returns true if the program executed with no errors. + /// If false, the program execution failed, check PI.Status, PI.ReturnCode + /// or PI.ErrMsg for further error information. + bool Wait( + ProcessInfo &PI, ///< The child process that should be waited on. unsigned SecondsToWait, ///< If non-zero, this specifies the amount of ///< time to wait for the child process to exit. If the time expires, the ///< child is killed and this function returns. If zero, this function ///< will perform a non-blocking wait on the child process. - bool WaitUntilTerminates, ///< If true, ignores \p SecondsToWait and waits + bool WaitUntilTerminates ///< If true, ignores \p SecondsToWait and waits ///< until child has terminated. - std::string *ErrMsg = nullptr ///< If non-zero, provides a pointer to a - ///< string instance in which error messages will be returned. If the - ///< string is non-empty upon return an error occurred while invoking the - ///< program. - ); + ); + + /// This function waits for some/all processes specified by \p PIArray to + /// finish. + /// \returns Whether the wait fulfilled all conditions. + /// \note Users of this function should always check the ReturnCode member of + /// all ProcessInfo(s) passed in PIArray. + bool WaitMany( + MutableArrayRef PIArray, + ///< The children processes that should be waited on. + bool WaitOnAll, ///< If set, succeeds only when all processes complete. + ///< If not set, the function will succeed on the first completed process. + ///< Ignored if \p PIArray contains only one element. + unsigned SecondsToWait, ///< If non-zero, this specifies the amount of + ///< time to wait for the children processes to exit. If the time expires, + ///< all children are killed and this function returns. If zero, this + ///< function will perform a non-blocking wait on all children processes. + bool WaitUntilProcessTerminates ///< If true, ignores \p SecondsToWait and + ///< waits until at least one child has terminated, depending on + ///< \p WaitOnAll. + ); + + /// This function kills the processes specified by \p PIArray, and waits + /// until all resources are closed, including I/O streams. \returns Whether + /// all processes were successfully closed/killed/terminated. \note If the + /// function fails, users must check elements in \p PIArray to find out which + /// process failed. + bool Terminate(MutableArrayRef PIArray); #if defined(_WIN32) /// Given a list of command line arguments, quote and escape them as necessary Index: llvm/trunk/lib/Support/GraphWriter.cpp =================================================================== --- llvm/trunk/lib/Support/GraphWriter.cpp +++ llvm/trunk/lib/Support/GraphWriter.cpp @@ -92,17 +92,17 @@ // Execute the graph viewer. Return true if there were errors. static bool ExecGraphViewer(StringRef ExecPath, std::vector &args, - StringRef Filename, bool wait, - std::string &ErrMsg) { + StringRef Filename, bool wait) { + sys::ProcessInfo PI; if (wait) { - if (sys::ExecuteAndWait(ExecPath, args, None, {}, 0, 0, &ErrMsg)) { - errs() << "Error: " << ErrMsg << "\n"; + if (!sys::ExecuteAndWait(ExecPath, args, None, {}, 0, 0, &PI)) { + errs() << "Error: " << PI.ErrMsg.getValueOr("unknown error") << "\n"; return true; } sys::fs::remove(Filename); errs() << " done. \n"; } else { - sys::ExecuteNoWait(ExecPath, args, None, {}, 0, &ErrMsg); + sys::ExecuteNoWait(ExecPath, args, None); errs() << "Remember to erase graph file: " << Filename << "\n"; } return false; @@ -149,7 +149,6 @@ bool llvm::DisplayGraph(StringRef FilenameRef, bool wait, GraphProgram::Name program) { std::string Filename = FilenameRef; - std::string ErrMsg; std::string ViewerPath; GraphSession S; @@ -162,7 +161,7 @@ args.push_back("-W"); args.push_back(Filename); errs() << "Trying 'open' program... "; - if (!ExecGraphViewer(ViewerPath, args, Filename, wait, ErrMsg)) + if (!ExecGraphViewer(ViewerPath, args, Filename, wait)) return false; } #endif @@ -171,7 +170,7 @@ args.push_back(ViewerPath); args.push_back(Filename); errs() << "Trying 'xdg-open' program... "; - if (!ExecGraphViewer(ViewerPath, args, Filename, wait, ErrMsg)) + if (!ExecGraphViewer(ViewerPath, args, Filename, wait)) return false; } @@ -182,7 +181,7 @@ args.push_back(Filename); errs() << "Running 'Graphviz' program... "; - return ExecGraphViewer(ViewerPath, args, Filename, wait, ErrMsg); + return ExecGraphViewer(ViewerPath, args, Filename, wait); } // xdot @@ -195,7 +194,7 @@ args.push_back(getProgramName(program)); errs() << "Running 'xdot.py' program... "; - return ExecGraphViewer(ViewerPath, args, Filename, wait, ErrMsg); + return ExecGraphViewer(ViewerPath, args, Filename, wait); } enum ViewerKind { @@ -242,7 +241,7 @@ errs() << "Running '" << GeneratorPath << "' program... "; - if (ExecGraphViewer(GeneratorPath, args, Filename, true, ErrMsg)) + if (ExecGraphViewer(GeneratorPath, args, Filename, true)) return true; // The lifetime of StartArg must include the call of ExecGraphViewer @@ -274,9 +273,7 @@ case VK_None: llvm_unreachable("Invalid viewer"); } - - ErrMsg.clear(); - return ExecGraphViewer(ViewerPath, args, OutputFilename, wait, ErrMsg); + return ExecGraphViewer(ViewerPath, args, OutputFilename, wait); } // dotty @@ -290,7 +287,7 @@ wait = false; #endif errs() << "Running 'dotty' program... "; - return ExecGraphViewer(ViewerPath, args, Filename, wait, ErrMsg); + return ExecGraphViewer(ViewerPath, args, Filename, wait); } errs() << "Error: Couldn't find a usable graph viewer program:\n"; Index: llvm/trunk/lib/Support/Program.cpp =================================================================== --- llvm/trunk/lib/Support/Program.cpp +++ llvm/trunk/lib/Support/Program.cpp @@ -26,42 +26,31 @@ static bool Execute(ProcessInfo &PI, StringRef Program, ArrayRef Args, Optional> Env, ArrayRef> Redirects, - unsigned MemoryLimit, std::string *ErrMsg); + unsigned MemoryLimit); -int sys::ExecuteAndWait(StringRef Program, ArrayRef Args, +bool sys::ExecuteAndWait(StringRef Program, ArrayRef Args, Optional> Env, ArrayRef> Redirects, unsigned SecondsToWait, unsigned MemoryLimit, - std::string *ErrMsg, bool *ExecutionFailed) { + ProcessInfo *PI) { assert(Redirects.empty() || Redirects.size() == 3); - ProcessInfo PI; - if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg)) { - if (ExecutionFailed) - *ExecutionFailed = false; - ProcessInfo Result = Wait( - PI, SecondsToWait, /*WaitUntilTerminates=*/SecondsToWait == 0, ErrMsg); - return Result.ReturnCode; - } - - if (ExecutionFailed) - *ExecutionFailed = true; + ProcessInfo LocalPI; + if (!PI) + PI = &LocalPI; + bool Ret = Execute(*PI, Program, Args, Env, Redirects, MemoryLimit); + if (!Ret) + return false; - return -1; + return Wait(*PI, SecondsToWait, /*WaitUntilTerminates=*/SecondsToWait == 0); } ProcessInfo sys::ExecuteNoWait(StringRef Program, ArrayRef Args, Optional> Env, ArrayRef> Redirects, - unsigned MemoryLimit, std::string *ErrMsg, - bool *ExecutionFailed) { + unsigned MemoryLimit) { assert(Redirects.empty() || Redirects.size() == 3); ProcessInfo PI; - if (ExecutionFailed) - *ExecutionFailed = false; - if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg)) - if (ExecutionFailed) - *ExecutionFailed = true; - + Execute(PI, Program, Args, Env, Redirects, MemoryLimit); return PI; } @@ -74,6 +63,13 @@ return commandLineFitsWithinSystemLimits(Program, StringRefArgs); } +bool sys::Wait(ProcessInfo &PI, unsigned SecondsToWait, + bool WaitUntilChildTerminates) { + ProcessInfo *A[] = {&PI}; + return WaitMany(makeMutableArrayRef(A, 1), true, SecondsToWait, + WaitUntilChildTerminates); +} + // Include the platform-specific parts of this class. #ifdef LLVM_ON_UNIX #include "Unix/Program.inc" Index: llvm/trunk/lib/Support/Windows/DynamicLibrary.inc =================================================================== --- llvm/trunk/lib/Support/Windows/DynamicLibrary.inc +++ llvm/trunk/lib/Support/Windows/DynamicLibrary.inc @@ -39,19 +39,27 @@ if (!File) return &(*OpenedHandles); + const char* E = nullptr; + HMODULE Handle = nullptr; + SmallVector FileUnicode; if (std::error_code ec = windows::UTF8ToUTF16(File, FileUnicode)) { SetLastError(ec.value()); - MakeErrMsg(Err, std::string(File) + ": Can't convert to UTF-16"); - return &DynamicLibrary::Invalid; + E = "Can't convert to UTF-16"; + } else { + Handle = LoadLibraryW(FileUnicode.data()); + if (Handle == NULL) { + E = "Can't open"; + } } - HMODULE Handle = LoadLibraryW(FileUnicode.data()); - if (Handle == NULL) { - MakeErrMsg(Err, std::string(File) + ": Can't open"); + if (E) { + Optional ErrMsg; + MakeErrMsg(ErrMsg, std::string(File) + ": " + E); + if (Err) + *Err = ErrMsg.getValue(); return &DynamicLibrary::Invalid; } - return reinterpret_cast(Handle); } @@ -79,9 +87,9 @@ !EnumProcessModules(H, Data, Bytes, &Bytes) #endif ) { - std::string Err; - if (MakeErrMsg(&Err, "EnumProcessModules failure")) - llvm::errs() << Err << "\n"; + Optional Err; + if (MakeErrMsg(Err, "EnumProcessModules failure")) + llvm::errs() << Err.getValue() << "\n"; return false; } return true; @@ -96,7 +104,7 @@ if (!HS->Process) return nullptr; - // Trials indicate EnumProcessModulesEx is consistantly faster than using + // Trials indicate EnumProcessModulesEx is consistently faster than using // EnumerateLoadedModules64 or CreateToolhelp32Snapshot. // // | Handles | DbgHelp.dll | CreateSnapshot | EnumProcessModulesEx Index: llvm/trunk/lib/Support/Windows/Process.inc =================================================================== --- llvm/trunk/lib/Support/Windows/Process.inc +++ llvm/trunk/lib/Support/Windows/Process.inc @@ -452,9 +452,9 @@ // Include GetLastError() in a fatal error message. static void ReportLastErrorFatal(const char *Msg) { - std::string ErrMsg; - MakeErrMsg(&ErrMsg, Msg); - report_fatal_error(ErrMsg); + llvm::Optional ErrMsg; + MakeErrMsg(ErrMsg, Msg); + report_fatal_error(ErrMsg.getValue()); } unsigned Process::GetRandomNumber() { Index: llvm/trunk/lib/Support/Windows/Program.inc =================================================================== --- llvm/trunk/lib/Support/Windows/Program.inc +++ llvm/trunk/lib/Support/Windows/Program.inc @@ -32,7 +32,7 @@ namespace llvm { -ProcessInfo::ProcessInfo() : Pid(0), Process(0), ReturnCode(0) {} +ProcessInfo::ProcessInfo() : Pid(), Process(), Status(ExecutionStatus::NotStarted) {} ErrorOr sys::findProgramByName(StringRef Name, ArrayRef Paths) { @@ -106,7 +106,7 @@ } static HANDLE RedirectIO(Optional Path, int fd, - std::string *ErrMsg) { + Optional& ErrMsg) { HANDLE h; if (!Path) { if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)_get_osfhandle(fd), @@ -140,10 +140,9 @@ FILE_SHARE_READ, &sa, fd == 0 ? OPEN_EXISTING : CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if (h == INVALID_HANDLE_VALUE) { - MakeErrMsg(ErrMsg, fname + ": Can't open file for " + - (fd ? "input" : "output")); + MakeErrMsg(ErrMsg, + fname + ": Can't open file for " + (fd ? "input" : "output")); } - return h; } @@ -152,10 +151,11 @@ static bool Execute(ProcessInfo &PI, StringRef Program, ArrayRef Args, Optional> Env, ArrayRef> Redirects, - unsigned MemoryLimit, std::string *ErrMsg) { + unsigned MemoryLimit) { + assert(PI.notStarted()); if (!sys::fs::can_execute(Program)) { - if (ErrMsg) - *ErrMsg = "program not executable"; + PI.Status = sys::ExecutionStatus::ExecutionFailure; + PI.ErrMsg.emplace("program not executable or not found"); return false; } @@ -183,7 +183,8 @@ SmallVector EnvString; if (std::error_code ec = windows::UTF8ToUTF16(E, EnvString)) { SetLastError(ec.value()); - MakeErrMsg(ErrMsg, "Unable to convert environment variable to UTF-16"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "unable to convert environment variable to UTF-16"); return false; } @@ -204,15 +205,17 @@ if (!Redirects.empty()) { si.dwFlags = STARTF_USESTDHANDLES; - si.hStdInput = RedirectIO(Redirects[0], 0, ErrMsg); + si.hStdInput = RedirectIO(Redirects[0], 0, PI.ErrMsg); if (si.hStdInput == INVALID_HANDLE_VALUE) { - MakeErrMsg(ErrMsg, "can't redirect stdin"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "can't redirect stdin"); return false; } - si.hStdOutput = RedirectIO(Redirects[1], 1, ErrMsg); + si.hStdOutput = RedirectIO(Redirects[1], 1, PI.ErrMsg); if (si.hStdOutput == INVALID_HANDLE_VALUE) { CloseHandle(si.hStdInput); - MakeErrMsg(ErrMsg, "can't redirect stdout"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "can't redirect stdout"); return false; } if (Redirects[1] && Redirects[2] && *Redirects[1] == *Redirects[2]) { @@ -223,16 +226,18 @@ 0, TRUE, DUPLICATE_SAME_ACCESS)) { CloseHandle(si.hStdInput); CloseHandle(si.hStdOutput); - MakeErrMsg(ErrMsg, "can't dup stderr to stdout"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "can't dup stderr to stdout"); return false; } } else { // Just redirect stderr - si.hStdError = RedirectIO(Redirects[2], 2, ErrMsg); + si.hStdError = RedirectIO(Redirects[2], 2, PI.ErrMsg); if (si.hStdError == INVALID_HANDLE_VALUE) { CloseHandle(si.hStdInput); CloseHandle(si.hStdOutput); - MakeErrMsg(ErrMsg, "can't redirect stderr"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "can't redirect stderr"); return false; } } @@ -247,16 +252,16 @@ SmallVector ProgramUtf16; if (std::error_code ec = path::widenPath(Program, ProgramUtf16)) { SetLastError(ec.value()); - MakeErrMsg(ErrMsg, - std::string("Unable to convert application name to UTF-16")); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "unable to convert application name to UTF-16"); return false; } SmallVector CommandUtf16; if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) { SetLastError(ec.value()); - MakeErrMsg(ErrMsg, - std::string("Unable to convert command-line to UTF-16")); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg,"unable to convert command-line to UTF-16"); return false; } @@ -275,8 +280,9 @@ // Now return an error if the process didn't get created. if (!rc) { SetLastError(err); - MakeErrMsg(ErrMsg, std::string("Couldn't execute program '") + - Program.str() + "'"); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, + std::string("couldn't execute program '") + Program.str() + "'"); return false; } @@ -304,13 +310,17 @@ } if (!success) { SetLastError(GetLastError()); - MakeErrMsg(ErrMsg, std::string("Unable to set memory limit")); + PI.Status = sys::ExecutionStatus::ExecutionFailure; + MakeErrMsg(PI.ErrMsg, "unable to set memory limit"); TerminateProcess(pi.hProcess, 1); WaitForSingleObject(pi.hProcess, INFINITE); + PI.Pid = procid_t(); + PI.Process = process_t(); return false; } } + PI.Status = sys::ExecutionStatus::StartedAndExecuting; return true; } @@ -356,6 +366,46 @@ return Result; } +bool CompleteProcess(sys::ProcessInfo &PI) { + DWORD status; + BOOL rc = GetExitCodeProcess(PI.Process, &status); + if (rc && status == STILL_ACTIVE) + return true; + + DWORD err = GetLastError(); + if (err != ERROR_INVALID_HANDLE) { + CloseHandle(PI.Process); + } + PI.Pid = procid_t(); + PI.Process = process_t(); + + if (!rc) { + SetLastError(err); + + // indicates a crash or timeout as opposed to failure to execute. + PI.Status = ExecutionStatus::Crashed; + MakeErrMsg(PI.ErrMsg, "Failed getting status for program"); + return false; + } + + if (!status) { + PI.ReturnCode = 0; + PI.Status = ExecutionStatus::CompletedSuccess; + return true; + } + + // Pass 10(Warning) and 11(Error) to the callee as negative value. + if ((status & 0xBFFF0000U) == 0x80000000U) + PI.ReturnCode = static_cast(status); + else if (status & 0xFF) + PI.ReturnCode = status & 0x7FFFFFFF; + else + PI.ReturnCode = 1; + + PI.Status = ExecutionStatus::CompletedWithError; + return true; +} + namespace llvm { std::string sys::flattenWindowsCommandLine(ArrayRef Args) { std::string Command; @@ -371,67 +421,103 @@ return Command; } -ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait, - bool WaitUntilChildTerminates, std::string *ErrMsg) { - assert(PI.Pid && "invalid pid to wait on, process not started?"); - assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) && - "invalid process handle to wait on, process not started?"); +bool sys::WaitMany(MutableArrayRef PIArray, bool WaitOnAll, + unsigned SecondsToWait, bool WaitUntilProcessTerminates) { DWORD milliSecondsToWait = 0; - if (WaitUntilChildTerminates) + if (WaitUntilProcessTerminates) milliSecondsToWait = INFINITE; else if (SecondsToWait > 0) milliSecondsToWait = SecondsToWait * 1000; - ProcessInfo WaitResult = PI; - DWORD WaitStatus = WaitForSingleObject(PI.Process, milliSecondsToWait); - if (WaitStatus == WAIT_TIMEOUT) { - if (SecondsToWait) { - if (!TerminateProcess(PI.Process, 1)) { - if (ErrMsg) - MakeErrMsg(ErrMsg, "Failed to terminate timed-out program"); - - // -2 indicates a crash or timeout as opposed to failure to execute. - WaitResult.ReturnCode = -2; - CloseHandle(PI.Process); - return WaitResult; + SmallVector Handles; + Handles.reserve(PIArray.size()); + + for (auto &PI : PIArray) { + if (!PI->running()) + continue; + assert(PI->Pid); + assert(PI->Process && PI->Process != INVALID_HANDLE_VALUE); + Handles.push_back(PI->Process); + } + + if (Handles.empty()) + return false; + + if (Handles.size() == 1) + WaitOnAll = true; + + DWORD WaitStatus = WaitForMultipleObjects(Handles.size(), Handles.begin(), + WaitOnAll, milliSecondsToWait); + + if (WaitStatus >= WAIT_OBJECT_0 && + WaitStatus < (WAIT_OBJECT_0 + Handles.size())) { + // success + } else { + if (WaitStatus == WAIT_TIMEOUT) { + if (!SecondsToWait) + return true; // Non-blocking wait + Terminate(PIArray); + } + else + { + for (auto &PI : PIArray) { + if (!PI->running()) + continue; + MakeErrMsg(PI->ErrMsg, "Failed to wait for processes"); } - WaitForSingleObject(PI.Process, INFINITE); - CloseHandle(PI.Process); - } else { - // Non-blocking wait. - return ProcessInfo(); } + return false; } - // Get its exit status. - DWORD status; - BOOL rc = GetExitCodeProcess(PI.Process, &status); - DWORD err = GetLastError(); - if (err != ERROR_INVALID_HANDLE) - CloseHandle(PI.Process); + bool Success = true; + // Finish collecting information from completed processes + for (auto &PI : PIArray) { + if (!PI->running()) + continue; + + if (!CompleteProcess(*PI)) + Success = false; + } + return Success; +} + +bool sys::Terminate(MutableArrayRef PIArray) { + bool AllSuccess = true; + + SmallVector Handles; + Handles.reserve(PIArray.size()); + + // First, terminate all process asynchronously to give them time to close the + // file streams + for (auto &PI : PIArray) { + if (!PI->running()) + continue; + + if (!TerminateProcess(PI->Process, (int)ProgramReturnCode::Failure)) { + // Failure - the process might have finished in the meanwhile + if (!CompleteProcess(*PI)) + AllSuccess = false; + continue; + } + Handles.push_back(PI->Process); + } - if (!rc) { - SetLastError(err); - if (ErrMsg) - MakeErrMsg(ErrMsg, "Failed getting status for program"); + if (Handles.size() == 0) + return false; - // -2 indicates a crash or timeout as opposed to failure to execute. - WaitResult.ReturnCode = -2; - return WaitResult; - } + WaitForMultipleObjects(Handles.size(), Handles.begin(), TRUE, INFINITE); - if (!status) - return WaitResult; + for (auto &PI : PIArray) { + if (!PI->running()) + continue; - // Pass 10(Warning) and 11(Error) to the callee as negative value. - if ((status & 0xBFFF0000U) == 0x80000000U) - WaitResult.ReturnCode = static_cast(status); - else if (status & 0xFF) - WaitResult.ReturnCode = status & 0x7FFFFFFF; - else - WaitResult.ReturnCode = 1; + // Collect the process' return code + if (!CompleteProcess(*PI)) + AllSuccess = false; - return WaitResult; + PI->Status = sys::ExecutionStatus::Killed; + } + return AllSuccess; } std::error_code sys::ChangeStdinToBinary() { Index: llvm/trunk/lib/Support/Windows/WindowsSupport.h =================================================================== --- llvm/trunk/lib/Support/Windows/WindowsSupport.h +++ llvm/trunk/lib/Support/Windows/WindowsSupport.h @@ -71,9 +71,7 @@ Mask) != FALSE; } -inline bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) { - if (!ErrMsg) - return true; +inline bool MakeErrMsg(llvm::Optional &ErrMsg, const std::string &prefix) { char *buffer = NULL; DWORD LastError = GetLastError(); DWORD R = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | @@ -81,9 +79,9 @@ FORMAT_MESSAGE_MAX_WIDTH_MASK, NULL, LastError, 0, (LPSTR)&buffer, 1, NULL); if (R) - *ErrMsg = prefix + ": " + buffer; + ErrMsg.emplace(prefix + ": " + buffer); else - *ErrMsg = prefix + ": Unknown error"; + ErrMsg.emplace(prefix + ": Unknown error"); *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")"; LocalFree(buffer); Index: llvm/trunk/tools/bugpoint/ExecutionDriver.cpp =================================================================== --- llvm/trunk/tools/bugpoint/ExecutionDriver.cpp +++ llvm/trunk/tools/bugpoint/ExecutionDriver.cpp @@ -337,13 +337,13 @@ if (!SharedObj.empty()) SharedObjs.push_back(SharedObj); - Expected RetVal = AI->ExecuteProgram(BitcodeFile, InputArgv, InputFile, - OutputFile, AdditionalLinkerArgs, - SharedObjs, Timeout, MemoryLimit); - if (Error E = RetVal.takeError()) + Expected R = AI->ExecuteProgram( + BitcodeFile, InputArgv, InputFile, OutputFile, AdditionalLinkerArgs, + SharedObjs, Timeout, MemoryLimit); + if (Error E = R.takeError()) return std::move(E); - if (*RetVal == -1) { + if (R->killed()) { errs() << ""; static bool FirstTimeout = true; if (FirstTimeout) { @@ -362,7 +362,7 @@ if (AppendProgramExitCode) { std::ofstream outFile(OutputFile.c_str(), std::ios_base::app); - outFile << "exit " << *RetVal << '\n'; + outFile << "exit " << R->ErrMsg.getValueOr("unknown error") << '\n'; outFile.close(); } Index: llvm/trunk/tools/bugpoint/OptimizerDriver.cpp =================================================================== --- llvm/trunk/tools/bugpoint/OptimizerDriver.cpp +++ llvm/trunk/tools/bugpoint/OptimizerDriver.cpp @@ -237,32 +237,30 @@ Redirects[2] = ""; } - std::string ErrMsg; - int result = sys::ExecuteAndWait(Prog, Args, None, Redirects, Timeout, - MemoryLimit, &ErrMsg); + sys::ProcessInfo PI; + bool R = sys::ExecuteAndWait(Prog, Args, None, Redirects, Timeout, + MemoryLimit, &PI); // If we are supposed to delete the bitcode file or if the passes crashed, // remove it now. This may fail if the file was never created, but that's ok. - if (DeleteOutput || result != 0) + if (DeleteOutput || PI.completedError()) sys::fs::remove(OutputFilename); if (!Quiet) { - if (result == 0) + if (PI.killed()) + outs() << "Killed or timed out!\n"; + else if (PI.crashed()) + outs() << "Crashed: " << PI.ErrMsg.getValue() << "\n"; + else if (PI.notCompletedOrFailed()) + outs() << "Execute failed: " << PI.ErrMsg.getValue() << "\n"; + else if (PI.completedError()) + outs() << "Exited with error code '" << PI.ReturnCode.getValue() << "'\n"; + else if (PI.completedSuccess()) outs() << "Success!\n"; - else if (result > 0) - outs() << "Exited with error code '" << result << "'\n"; - else if (result < 0) { - if (result == -1) - outs() << "Execute failed: " << ErrMsg << "\n"; - else - outs() << "Crashed: " << ErrMsg << "\n"; - } - if (result & 0x01000000) - outs() << "Dumped core\n"; } // Was the child successful? - return result != 0; + return !R; } std::unique_ptr @@ -278,7 +276,7 @@ if (!Ret) { errs() << getToolName() << ": Error reading bitcode file '" << BitcodeResult << "'!\n"; - exit(1); + exit((int)sys::ProgramReturnCode::Failure); } sys::fs::remove(BitcodeResult); return Ret; Index: llvm/trunk/tools/bugpoint/ToolRunner.h =================================================================== --- llvm/trunk/tools/bugpoint/ToolRunner.h +++ llvm/trunk/tools/bugpoint/ToolRunner.h @@ -21,6 +21,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Error.h" #include "llvm/Support/Path.h" +#include "llvm/Support/Program.h" #include "llvm/Support/SystemUtils.h" #include #include @@ -59,7 +60,7 @@ /// option specifies optional native shared objects that can be loaded into /// the program for execution. /// - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &ProgramFile, const std::vector &Args, FileType fileType, const std::string &InputFile, const std::string &OutputFile, @@ -132,7 +133,7 @@ /// returns an Error if a problem was encountered that prevented execution of /// the program. /// - virtual Expected ExecuteProgram( + virtual Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs = std::vector(), @@ -166,7 +167,7 @@ Error compileProgram(const std::string &Bitcode, unsigned Timeout = 0, unsigned MemoryLimit = 0) override; - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs = std::vector(), Index: llvm/trunk/tools/bugpoint/ToolRunner.cpp =================================================================== --- llvm/trunk/tools/bugpoint/ToolRunner.cpp +++ llvm/trunk/tools/bugpoint/ToolRunner.cpp @@ -53,15 +53,16 @@ /// RunProgramWithTimeout - This function provides an alternate interface /// to the sys::Program::ExecuteAndWait interface. /// @see sys::Program::ExecuteAndWait -static int RunProgramWithTimeout(StringRef ProgramPath, +static sys::ProcessInfo RunProgramWithTimeout(StringRef ProgramPath, ArrayRef Args, StringRef StdInFile, StringRef StdOutFile, StringRef StdErrFile, unsigned NumSeconds = 0, - unsigned MemoryLimit = 0, - std::string *ErrMsg = nullptr) { + unsigned MemoryLimit = 0) { Optional Redirects[3] = {StdInFile, StdOutFile, StdErrFile}; - return sys::ExecuteAndWait(ProgramPath, Args, None, Redirects, NumSeconds, - MemoryLimit, ErrMsg); + sys::ProcessInfo PI; + sys::ExecuteAndWait(ProgramPath, Args, None, Redirects, NumSeconds, + MemoryLimit, &PI); + return PI; } /// RunProgramRemotelyWithTimeout - This function runs the given program @@ -70,18 +71,20 @@ /// fails. Remote client is required to return 255 if it failed or program exit /// code otherwise. /// @see sys::Program::ExecuteAndWait -static int RunProgramRemotelyWithTimeout( +static sys::ProcessInfo RunProgramRemotelyWithTimeout( StringRef RemoteClientPath, ArrayRef Args, StringRef StdInFile, StringRef StdOutFile, StringRef StdErrFile, unsigned NumSeconds = 0, unsigned MemoryLimit = 0) { Optional Redirects[3] = {StdInFile, StdOutFile, StdErrFile}; + sys::ProcessInfo PI; // Run the program remotely with the remote client - int ReturnCode = sys::ExecuteAndWait(RemoteClientPath, Args, None, Redirects, - NumSeconds, MemoryLimit); + bool R = sys::ExecuteAndWait(RemoteClientPath, Args, None, Redirects, + NumSeconds, MemoryLimit, &PI); // Has the remote client fail? - if (255 == ReturnCode) { + if (!R || PI.notCompletedOrFailed() || + (PI.completedError() && PI.hasRet(sys::ProgramReturnCode::BugpointClientFail))) { std::ostringstream OS; OS << "\nError running remote client:\n "; for (StringRef Arg : Args) @@ -101,7 +104,7 @@ errs() << OS.str(); } - return ReturnCode; + return PI; } static Error ProcessFailure(StringRef ProgPath, ArrayRef Args, @@ -118,7 +121,7 @@ "bugpoint.program_error_messages", "", ErrorFilename); if (EC) { errs() << "Error making unique filename: " << EC.message() << "\n"; - exit(1); + exit((int)llvm::sys::ProgramReturnCode::Failure); } RunProgramWithTimeout(ProgPath, Args, "", ErrorFilename.str(), @@ -154,7 +157,7 @@ } } - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs, @@ -163,7 +166,7 @@ }; } -Expected LLI::ExecuteProgram(const std::string &Bitcode, +Expected LLI::ExecuteProgram(const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, @@ -212,7 +215,7 @@ /// to \p ExeName, given the value of argv[0] and the address of main() /// itself. This allows us to find another LLVM tool if it is built in the same /// directory. An empty string is returned on error; note that this function -/// just mainpulates the path and doesn't check for executability. +/// just manipulates the path and doesn't check for executability. /// Find a named executable. static std::string PrependMainExecutablePath(const std::string &ExeName, const char *Argv0, @@ -267,7 +270,7 @@ Error compileProgram(const std::string &Bitcode, unsigned Timeout = 0, unsigned MemoryLimit = 0) override; - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs = std::vector(), @@ -294,7 +297,7 @@ for (unsigned i = 0, e = CompilerArgs.size(); i != e; ++i) ProgramArgs.push_back(CompilerArgs[i].c_str()); - if (RunProgramWithTimeout(CompilerCommand, ProgramArgs, "", "", "", Timeout, + if (!RunProgramWithTimeout(CompilerCommand, ProgramArgs, "", "", "", Timeout, MemoryLimit)) return ProcessFailure(CompilerCommand, ProgramArgs, Timeout, MemoryLimit); return Error::success(); @@ -316,7 +319,7 @@ std::vector ExecArgs) : ExecutionCommand(ExecutionCmd), ExecutorArgs(std::move(ExecArgs)) {} - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs, @@ -325,7 +328,7 @@ }; } -Expected CustomExecutor::ExecuteProgram( +Expected CustomExecutor::ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs, @@ -455,7 +458,7 @@ sys::fs::createUniqueFile(Bitcode + "-%%%%%%%" + Suffix, UniqueFile); if (EC) { errs() << "Error making unique filename: " << EC.message() << "\n"; - exit(1); + exit((int)llvm::sys::ProgramReturnCode::Failure); } OutputAsmFile = UniqueFile.str(); std::vector LLCArgs; @@ -478,7 +481,7 @@ for (unsigned i = 0, e = LLCArgs.size() - 1; i != e; ++i) errs() << " " << LLCArgs[i]; errs() << "\n";); - if (RunProgramWithTimeout(LLCPath, LLCArgs, "", "", "", Timeout, MemoryLimit)) + if (!RunProgramWithTimeout(LLCPath, LLCArgs, "", "", "", Timeout, MemoryLimit)) return ProcessFailure(LLCPath, LLCArgs, Timeout, MemoryLimit); return UseIntegratedAssembler ? CC::ObjectFile : CC::AsmFile; } @@ -494,7 +497,7 @@ return Error::success(); } -Expected LLC::ExecuteProgram(const std::string &Bitcode, +Expected LLC::ExecuteProgram(const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, @@ -534,7 +537,7 @@ CC *cc = CC::create(Message, CCBinary, CCArgs); if (!cc) { errs() << Message << "\n"; - exit(1); + exit((int)llvm::sys::ProgramReturnCode::Failure); } Message = "Found llc: " + LLCPath + "\n"; return new LLC(LLCPath, cc, Args, UseIntegratedAssembler); @@ -556,7 +559,7 @@ } } - Expected ExecuteProgram( + Expected ExecuteProgram( const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, const std::vector &CCArgs = std::vector(), @@ -565,7 +568,7 @@ }; } -Expected JIT::ExecuteProgram(const std::string &Bitcode, +Expected JIT::ExecuteProgram(const std::string &Bitcode, const std::vector &Args, const std::string &InputFile, const std::string &OutputFile, @@ -635,7 +638,7 @@ return false; } -Expected CC::ExecuteProgram(const std::string &ProgramFile, +Expected CC::ExecuteProgram(const std::string &ProgramFile, const std::vector &Args, FileType fileType, const std::string &InputFile, @@ -682,7 +685,7 @@ sys::fs::createUniqueFile(ProgramFile + "-%%%%%%%.cc.exe", OutputBinary); if (EC) { errs() << "Error making unique filename: " << EC.message() << "\n"; - exit(1); + exit((int)llvm::sys::ProgramReturnCode::Failure); } CCArgs.push_back(OutputBinary); // Output to the right file... @@ -704,7 +707,7 @@ for (unsigned i = 0, e = CCArgs.size() - 1; i != e; ++i) errs() << " " << CCArgs[i]; errs() << "\n";); - if (RunProgramWithTimeout(CCPath, CCArgs, "", "", "")) + if (!RunProgramWithTimeout(CCPath, CCArgs, "", "", "")) return ProcessFailure(CCPath, CCArgs); std::vector ProgramArgs; @@ -757,18 +760,17 @@ if (RemoteClientPath.empty()) { LLVM_DEBUG(errs() << ""); - std::string Error; - int ExitCode = RunProgramWithTimeout(OutputBinary.str(), ProgramArgs, - InputFile, OutputFile, OutputFile, - Timeout, MemoryLimit, &Error); + sys::ProcessInfo PI = + RunProgramWithTimeout(OutputBinary.str(), ProgramArgs, InputFile, + OutputFile, OutputFile, Timeout, MemoryLimit); // Treat a signal (usually SIGSEGV) or timeout as part of the program output // so that crash-causing miscompilation is handled seamlessly. - if (ExitCode < -1) { + if (PI.notCompletedOrFailed() || PI.crashed()) { std::ofstream outFile(OutputFile.c_str(), std::ios_base::app); - outFile << Error << '\n'; + outFile << PI.ErrMsg.getValueOr("unknown error") << '\n'; outFile.close(); } - return ExitCode; + return PI; } else { outs() << ""; outs().flush(); @@ -786,7 +788,7 @@ InputFile + "-%%%%%%%" + LTDL_SHLIB_EXT, UniqueFilename); if (EC) { errs() << "Error making unique filename: " << EC.message() << "\n"; - exit(1); + exit((int)llvm::sys::ProgramReturnCode::Failure); } OutputFile = UniqueFilename.str(); @@ -848,7 +850,7 @@ for (unsigned i = 0, e = CCArgs.size() - 1; i != e; ++i) errs() << " " << CCArgs[i]; errs() << "\n";); - if (RunProgramWithTimeout(CCPath, CCArgs, "", "", "")) + if (!RunProgramWithTimeout(CCPath, CCArgs, "", "", "")) return ProcessFailure(CCPath, CCArgs); return Error::success(); } Index: llvm/trunk/tools/dsymutil/MachOUtils.cpp =================================================================== --- llvm/trunk/tools/dsymutil/MachOUtils.cpp +++ llvm/trunk/tools/dsymutil/MachOUtils.cpp @@ -64,10 +64,10 @@ return false; } - std::string ErrMsg; - int result = sys::ExecuteAndWait(*Path, Args, None, {}, 0, 0, &ErrMsg); - if (result) { - WithColor::error() << "lipo: " << ErrMsg << "\n"; + sys::ProcessInfo PI; + bool R = sys::ExecuteAndWait(*Path, Args, None, {}, 0, 0, &PI); + if (!R) { + WithColor::error() << "lipo: " << PI.ErrMsg.getValueOr("unknown error") << "\n"; return false; } Index: llvm/trunk/tools/llvm-cov/CodeCoverage.cpp =================================================================== --- llvm/trunk/tools/llvm-cov/CodeCoverage.cpp +++ llvm/trunk/tools/llvm-cov/CodeCoverage.cpp @@ -476,12 +476,12 @@ for (StringRef Arg : ViewOpts.DemanglerOpts) ArgsV.push_back(Arg); Optional Redirects[] = {InputPath.str(), OutputPath.str(), {""}}; - std::string ErrMsg; - int RC = sys::ExecuteAndWait(ViewOpts.DemanglerOpts[0], ArgsV, + sys::ProcessInfo PI; + bool R = sys::ExecuteAndWait(ViewOpts.DemanglerOpts[0], ArgsV, /*env=*/None, Redirects, /*secondsToWait=*/0, - /*memoryLimit=*/0, &ErrMsg); - if (RC) { - error(ErrMsg, ViewOpts.DemanglerOpts[0]); + /*memoryLimit=*/0, &PI); + if (!R) { + error(PI.ErrMsg.getValueOr("unknown error"), ViewOpts.DemanglerOpts[0]); return; } Index: llvm/trunk/unittests/Support/ProgramTest.cpp =================================================================== --- llvm/trunk/unittests/Support/ProgramTest.cpp +++ llvm/trunk/unittests/Support/ProgramTest.cpp @@ -137,14 +137,12 @@ // MAX_PATH = 260 LongPath.append(260 - TestDirectory.size(), 'a'); - std::string Error; - bool ExecutionFailed; + ProcessInfo PI; Optional Redirects[] = {None, LongPath.str(), None}; - int RC = ExecuteAndWait(MyExe, ArgV, getEnviron(), Redirects, - /*secondsToWait=*/ 10, /*memoryLimit=*/ 0, &Error, - &ExecutionFailed); - EXPECT_FALSE(ExecutionFailed) << Error; - EXPECT_EQ(0, RC); + bool R = ExecuteAndWait(MyExe, ArgV, getEnviron(), Redirects, + /*secondsToWait=*/10, /*memoryLimit=*/0, &PI); + EXPECT_TRUE(R) << PI.ErrMsg.getValueOr("execution error"); + EXPECT_EQ(0, PI.ReturnCode.getValue()); // Remove the long stdout. ASSERT_NO_ERROR(fs::remove(Twine(LongPath))); @@ -174,8 +172,7 @@ // Add LLVM_PROGRAM_TEST_CHILD to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_CHILD=1"); - std::string error; - bool ExecutionFailed; + ProcessInfo PI; // Redirect stdout and stdin to NUL, but let stderr through. #ifdef _WIN32 StringRef nul("NUL"); @@ -183,11 +180,10 @@ StringRef nul("/dev/null"); #endif Optional redirects[] = { nul, nul, None }; - int rc = ExecuteAndWait(my_exe, argv, getEnviron(), redirects, - /*secondsToWait=*/ 10, /*memoryLimit=*/ 0, &error, - &ExecutionFailed); - EXPECT_FALSE(ExecutionFailed) << error; - EXPECT_EQ(0, rc); + bool R = ExecuteAndWait(my_exe, argv, getEnviron(), redirects, + /*secondsToWait=*/ 10, /*memoryLimit=*/ 0, &PI); + EXPECT_TRUE(R) << PI.ErrMsg.getValueOr("execution error"); + EXPECT_EQ(0, PI.ReturnCode.getValue()); } TEST_F(ProgramEnvTest, TestExecuteNoWait) { @@ -206,39 +202,36 @@ // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child. addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1"); - std::string Error; - bool ExecutionFailed; - ProcessInfo PI1 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error, - &ExecutionFailed); - ASSERT_FALSE(ExecutionFailed) << Error; + ProcessInfo PI1 = ExecuteNoWait(Executable, argv, getEnviron()); + ASSERT_TRUE(PI1.running()) << PI1.ErrMsg.getValueOr("execution error"); ASSERT_NE(PI1.Pid, ProcessInfo::InvalidPid) << "Invalid process id"; unsigned LoopCount = 0; // Test that Wait() with WaitUntilTerminates=true works. In this case, // LoopCount should only be incremented once. - while (true) { + while (LoopCount < 10) { ++LoopCount; - ProcessInfo WaitResult = llvm::sys::Wait(PI1, 0, true, &Error); - ASSERT_TRUE(Error.empty()); - if (WaitResult.Pid == PI1.Pid) + bool R = llvm::sys::Wait(PI1, 0, true); + ASSERT_TRUE(R) << PI1.ErrMsg.getValueOr("wait error"); + if (PI1.completedSuccess()) break; } EXPECT_EQ(LoopCount, 1u) << "LoopCount should be 1"; - ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error, - &ExecutionFailed); - ASSERT_FALSE(ExecutionFailed) << Error; + ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron()); + ASSERT_TRUE(PI2.running()) << PI2.ErrMsg.getValueOr("execution error"); ASSERT_NE(PI2.Pid, ProcessInfo::InvalidPid) << "Invalid process id"; // Test that Wait() with SecondsToWait=0 performs a non-blocking wait. In this // cse, LoopCount should be greater than 1 (more than one increment occurs). while (true) { ++LoopCount; - ProcessInfo WaitResult = llvm::sys::Wait(PI2, 0, false, &Error); - ASSERT_TRUE(Error.empty()); - if (WaitResult.Pid == PI2.Pid) + bool R = llvm::sys::Wait(PI2, 0, false); + ASSERT_TRUE(R) << PI2.ErrMsg.getValueOr("wait error"); + ASSERT_FALSE(PI2.completedError()); + if (PI1.completedSuccess()) break; } @@ -259,14 +252,13 @@ Executable, "--gtest_filter=ProgramEnvTest.TestExecuteAndWaitTimeout"}; // Add LLVM_PROGRAM_TEST_TIMEOUT to the environment of the child. - addEnvVar("LLVM_PROGRAM_TEST_TIMEOUT=1"); + addEnvVar("LLVM_PROGRAM_TEST_TIMEOUT=1"); - std::string Error; - bool ExecutionFailed; - int RetCode = - ExecuteAndWait(Executable, argv, getEnviron(), {}, /*secondsToWait=*/1, 0, - &Error, &ExecutionFailed); - ASSERT_EQ(-2, RetCode); + ProcessInfo PI; + bool R = + ExecuteAndWait(Executable, argv, getEnviron(), {}, /*secondsToWait=*/1, 0, &PI); + ASSERT_FALSE(R); + ASSERT_TRUE(PI.killed()); } TEST(ProgramTest, TestExecuteNegative) { @@ -274,25 +266,19 @@ StringRef argv[] = {Executable}; { - std::string Error; - bool ExecutionFailed; - int RetCode = ExecuteAndWait(Executable, argv, llvm::None, {}, 0, 0, &Error, - &ExecutionFailed); - ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or " - "positive value indicating the result code"; - ASSERT_TRUE(ExecutionFailed); - ASSERT_FALSE(Error.empty()); + ProcessInfo PI; + bool R = ExecuteAndWait(Executable, argv, llvm::None, {}, 0, 0, &PI); + ASSERT_FALSE(R); + ASSERT_TRUE(PI.notCompletedOrFailed()); + ASSERT_TRUE(PI.ErrMsg.hasValue()); } { - std::string Error; - bool ExecutionFailed; - ProcessInfo PI = ExecuteNoWait(Executable, argv, llvm::None, {}, 0, &Error, - &ExecutionFailed); + ProcessInfo PI = ExecuteNoWait(Executable, argv, llvm::None); ASSERT_EQ(PI.Pid, ProcessInfo::InvalidPid) << "On error ExecuteNoWait should return an invalid ProcessInfo"; - ASSERT_TRUE(ExecutionFailed); - ASSERT_FALSE(Error.empty()); + ASSERT_TRUE(PI.completedError()); + ASSERT_FALSE(PI.ErrMsg.hasValue()); } } Index: llvm/trunk/utils/not/not.cpp =================================================================== --- llvm/trunk/utils/not/not.cpp +++ llvm/trunk/utils/not/not.cpp @@ -29,38 +29,29 @@ } if (argc == 0) - return 1; + return (int)llvm::sys::ProgramReturnCode::Failure; auto Program = sys::findProgramByName(argv[0]); if (!Program) { errs() << "Error: Unable to find `" << argv[0] << "' in PATH: " << Program.getError().message() << "\n"; - return 1; + return (int)llvm::sys::ProgramReturnCode::Failure; } std::vector Argv; Argv.reserve(argc); for (int i = 0; i < argc; ++i) Argv.push_back(argv[i]); - std::string ErrMsg; - int Result = sys::ExecuteAndWait(*Program, Argv, None, {}, 0, 0, &ErrMsg); -#ifdef _WIN32 - // Handle abort() in msvcrt -- It has exit code as 3. abort(), aka - // unreachable, should be recognized as a crash. However, some binaries use - // exit code 3 on non-crash failure paths, so only do this if we expect a - // crash. - if (ExpectCrash && Result == 3) - Result = -3; -#endif - if (Result < 0) { - errs() << "Error: " << ErrMsg << "\n"; - if (ExpectCrash) - return 0; - return 1; + + sys::ProcessInfo PI; + sys::ExecuteAndWait(*Program, Argv, None, {}, 0, 0, &PI); + + if (PI.crashed(ExpectCrash)) { + errs() << "Error: " << PI.ErrMsg.getValue() << "\n"; + return int(ExpectCrash ? llvm::sys::ProgramReturnCode::Success + : llvm::sys::ProgramReturnCode::Failure); } - if (ExpectCrash) - return 1; - - return Result == 0; + return (int)llvm::sys::ProgramReturnCode::Failure; + return PI.completedSuccess(); }