aganea updated this revision to Diff 228729. aganea marked 2 inline comments as done. aganea added a comment.
Addressed comments & finished the Linux part. All tests pass. @Meinersbur : Just to show the difference between Windows & Linux, here's some timings for running the tests over the same `git checkout`, compiled with clang-9. Everything was already built, I used `ninja check-all` and only the tests ran. `LLVM_ENABLE_PROJECTS = "clang;lld;clang-tools-extra;compiler-rt"` | 6-core Intel(R) Xeon(R) CPU E5-1650 0 @ 3.20GHz | Ubuntu 18.04 | 770.07s (12 min 50 sec) | 59182 tests ran | | 6-core Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz | Windows 10 build 1903 | 1779.83s (29 min 39 sec) | 56589 tests ran | | This patch does not make much difference for running the tests on Windows (it's saving only 30 sec). The friction in the OS is so high, improving just clang.exe does not change much. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 Files: clang/include/clang/Driver/Driver.h clang/include/clang/Driver/Job.h clang/lib/Driver/Driver.cpp clang/lib/Driver/Job.cpp clang/tools/driver/driver.cpp llvm/include/llvm/Support/CrashRecoveryContext.h llvm/lib/Support/CrashRecoveryContext.cpp llvm/lib/Support/Unix/Signals.inc llvm/lib/Support/Windows/Signals.inc
Index: llvm/lib/Support/Windows/Signals.inc =================================================================== --- llvm/lib/Support/Windows/Signals.inc +++ llvm/lib/Support/Windows/Signals.inc @@ -187,7 +187,7 @@ using namespace llvm; // Forward declare. -static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep); +LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep); static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType); // The function to call if ctrl-c is pressed. @@ -521,10 +521,13 @@ extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord); #endif -void llvm::sys::PrintStackTrace(raw_ostream &OS) { - STACKFRAME64 StackFrame = {}; - CONTEXT Context = {}; - ::RtlCaptureContext(&Context); +static void LocalPrintStackTrace(raw_ostream &OS, PCONTEXT C) { + STACKFRAME64 StackFrame{}; + CONTEXT Context{}; + if (!C) { + ::RtlCaptureContext(&Context); + C = &Context; + } #if defined(_M_X64) StackFrame.AddrPC.Offset = Context.Rip; StackFrame.AddrStack.Offset = Context.Rsp; @@ -546,9 +549,12 @@ StackFrame.AddrStack.Mode = AddrModeFlat; StackFrame.AddrFrame.Mode = AddrModeFlat; PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(), - StackFrame, &Context); + StackFrame, C); } +void llvm::sys::PrintStackTrace(raw_ostream &OS) { + LocalPrintStackTrace(OS, nullptr); +} void llvm::sys::SetInterruptFunction(void (*IF)()) { RegisterHandler(); @@ -785,7 +791,7 @@ return std::error_code(); } -static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) { +LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) { Cleanup(); // We'll automatically write a Minidump file here to help diagnose @@ -803,42 +809,9 @@ << "\n"; } - // Initialize the STACKFRAME structure. - STACKFRAME64 StackFrame = {}; - -#if defined(_M_X64) - StackFrame.AddrPC.Offset = ep->ContextRecord->Rip; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp; - StackFrame.AddrStack.Mode = AddrModeFlat; - StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp; - StackFrame.AddrFrame.Mode = AddrModeFlat; -#elif defined(_M_IX86) - StackFrame.AddrPC.Offset = ep->ContextRecord->Eip; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Esp; - StackFrame.AddrStack.Mode = AddrModeFlat; - StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp; - StackFrame.AddrFrame.Mode = AddrModeFlat; -#elif defined(_M_ARM64) || defined(_M_ARM) - StackFrame.AddrPC.Offset = ep->ContextRecord->Pc; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Sp; - StackFrame.AddrStack.Mode = AddrModeFlat; -#if defined(_M_ARM64) - StackFrame.AddrFrame.Offset = ep->ContextRecord->Fp; -#else - StackFrame.AddrFrame.Offset = ep->ContextRecord->R11; -#endif - StackFrame.AddrFrame.Mode = AddrModeFlat; -#endif - - HANDLE hProcess = GetCurrentProcess(); - HANDLE hThread = GetCurrentThread(); - PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame, - ep->ContextRecord); + LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr); - _exit(ep->ExceptionRecord->ExceptionCode); + return EXCEPTION_EXECUTE_HANDLER; } static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) { Index: llvm/lib/Support/Unix/Signals.inc =================================================================== --- llvm/lib/Support/Unix/Signals.inc +++ llvm/lib/Support/Unix/Signals.inc @@ -79,7 +79,7 @@ using namespace llvm; -static RETSIGTYPE SignalHandler(int Sig); // defined below. +RETSIGTYPE SignalHandler(int Sig); // defined below. static RETSIGTYPE InfoSignalHandler(int Sig); // defined below. using SignalHandlerFunctionType = void (*)(); @@ -341,7 +341,7 @@ } // The signal handler that runs. -static RETSIGTYPE SignalHandler(int Sig) { +RETSIGTYPE SignalHandler(int Sig) { // Restore the signal behavior to default, so that the program actually // crashes when we return and the signal reissues. This also ensures that if // we crash in our signal handler that the program will terminate immediately @@ -356,8 +356,8 @@ { RemoveFilesToRemove(); - if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) - != std::end(IntSigs)) { + if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) != + std::end(IntSigs)) { if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr)) return OldInterruptFunction(); @@ -365,9 +365,9 @@ if (Sig == SIGPIPE) exit(EX_IOERR); - raise(Sig); // Execute the default handler. + raise(Sig); // Execute the default handler. return; - } + } } // Otherwise if it is a fault (like SEGV) run any handler. Index: llvm/lib/Support/CrashRecoveryContext.cpp =================================================================== --- llvm/lib/Support/CrashRecoveryContext.cpp +++ llvm/lib/Support/CrashRecoveryContext.cpp @@ -13,8 +13,20 @@ #include "llvm/Support/ThreadLocal.h" #include <mutex> #include <setjmp.h> +#ifdef _WIN32 +#include <windows.h> +#endif using namespace llvm; +#ifdef _WIN32 +// In llvm/lib/Support/Windows/Signals.inc +LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep); +#endif +#ifdef LLVM_ON_UNIX +// In llvm/lib/Support/Unix/Signals.inc +void SignalHandler(int Sig); +#endif + namespace { struct CrashRecoveryContextImpl; @@ -54,7 +66,7 @@ #endif } - void HandleCrash() { + void HandleCrash(int RetCode, void *ExceptionInfo) { // Eliminate the current context entry, to avoid re-entering in case the // cleanup code crashes. CurrentContext->set(Next); @@ -62,7 +74,20 @@ assert(!Failed && "Crash recovery context already failed!"); Failed = true; - // FIXME: Stash the backtrace. + if (CRC->EnableExceptionHandler) { +#ifdef _WIN32 + LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)ExceptionInfo); +#endif +#ifdef LLVM_ON_UNIX + SignalHandler(RetCode); + // llvm/lib/Support/Unix/Program.inc:Wait() returns -2 if a crash occurs, + // not the actual error code. + if (WIFSIGNALED(RetCode)) + RetCode = -2; +#endif + } + + CRC->RetCode = RetCode; // Jump back to the RunSafely we were called under. longjmp(JumpBuffer, 1); @@ -178,9 +203,14 @@ } bool Result = true; + // FIXME error: cannot compile this 'this' captured by SEH yet + CrashRecoveryContext *This = this; __try { Fn(); - } __except (1) { // Catch any exception. + } __except (This->EnableExceptionHandler + ? LLVMUnhandledExceptionFilter(GetExceptionInformation()) + : 1) { + This->RetCode = GetExceptionCode(); Result = false; } return Result; @@ -237,7 +267,8 @@ // implementation if we so choose. // Handle the crash - const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash(); + const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash( + (int)ExceptionInfo->ExceptionRecord->ExceptionCode, ExceptionInfo); // Note that we don't actually get here because HandleCrash calls // longjmp, which means the HandleCrash function never returns. @@ -320,7 +351,7 @@ sigprocmask(SIG_UNBLOCK, &SigMask, nullptr); if (CRCI) - const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash(); + const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(Signal, nullptr); } static void installExceptionOrSignalHandlers() { @@ -364,7 +395,7 @@ void CrashRecoveryContext::HandleCrash() { CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl; assert(CRCI && "Crash recovery context never initialized!"); - CRCI->HandleCrash(); + CRCI->HandleCrash(-1, nullptr); } // FIXME: Portability. Index: llvm/include/llvm/Support/CrashRecoveryContext.h =================================================================== --- llvm/include/llvm/Support/CrashRecoveryContext.h +++ llvm/include/llvm/Support/CrashRecoveryContext.h @@ -100,6 +100,15 @@ /// Explicitly trigger a crash recovery in the current process, and /// return failure from RunSafely(). This function does not return. void HandleCrash(); + + /// In case of a crash, this is the crash identifier + int RetCode{}; + + /// Selects whether the global exception handler should be called. When this + /// is active, a crash has the same side-effect as uncatched code (callstack + /// print and coredump/minidump), except that here we recover the execution + /// flow after the call to RunSafely(). + bool EnableExceptionHandler{}; }; /// Abstract base class of cleanup handlers. Index: clang/tools/driver/driver.cpp =================================================================== --- clang/tools/driver/driver.cpp +++ clang/tools/driver/driver.cpp @@ -30,6 +30,7 @@ #include "llvm/Option/OptTable.h" #include "llvm/Option/Option.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Host.h" @@ -318,6 +319,8 @@ return 1; } +int ClangDriverMain(SmallVectorImpl<const char *> &argv); + int main(int argc_, const char **argv_) { noteBottomOfStack(); llvm::InitLLVM X(argc_, argv_); @@ -327,6 +330,20 @@ return 1; llvm::InitializeAllTargets(); + return ClangDriverMain(argv); +} + +// When calling any clang driver other than -cc1, we optimize the +// clangDriver lib to call back into this function, instead starting a new +// clang.exe. This saves a huge amount of time of Windows, as process creation +// can be expensive on that platform. +int ClangDriverMain(SmallVectorImpl<const char *> &argv) { + + if (!Driver::Main) + Driver::Main = &ClangDriverMain; + else + llvm::cl::ResetAllOptionOccurrences(); + auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]); llvm::BumpPtrAllocator A; @@ -503,7 +520,7 @@ #ifdef _WIN32 // Exit status should not be negative on Win32, unless abnormal termination. - // Once abnormal termiation was caught, negative status should not be + // Once abnormal termination was caught, negative status should not be // propagated. if (Res < 0) Res = 1; Index: clang/lib/Driver/Job.cpp =================================================================== --- clang/lib/Driver/Job.cpp +++ clang/lib/Driver/Job.cpp @@ -19,8 +19,10 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" +#include "llvm/Support/PrettyStackTrace.h" #include "llvm/Support/Program.h" #include "llvm/Support/raw_ostream.h" #include <algorithm> @@ -313,6 +315,8 @@ Environment.push_back(nullptr); } +LLVM_THREAD_LOCAL bool Command::ReenterTool = true; + int Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects, std::string *ErrMsg, bool *ExecutionFailed) const { if (PrintInputFilenames) { @@ -321,7 +325,7 @@ llvm::outs().flush(); } - SmallVector<const char*, 128> Argv; + SmallVector<const char *, 128> Argv; Optional<ArrayRef<StringRef>> Env; std::vector<StringRef> ArgvVectorStorage; @@ -332,42 +336,79 @@ Env = makeArrayRef(ArgvVectorStorage); } + Driver::ClangDriverMainFunc ClangDriverMain{}; + + // If we can re-use the same process, we reenter a variant of main() through a + // callback. This is an optimization for speed on Windows. + if (ReenterTool) { + const Driver &D = getCreator().getToolChain().getDriver(); + StringRef CommandExe = llvm::sys::path::stem(Executable); + StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable); + if (CommandExe.equals_lower(DriverExe)) + ClangDriverMain = Driver::Main; + } + if (ResponseFile == nullptr) { Argv.push_back(Executable); Argv.append(Arguments.begin(), Arguments.end()); - Argv.push_back(nullptr); - - auto Args = llvm::toStringRefArray(Argv.data()); - return llvm::sys::ExecuteAndWait( - Executable, Args, Env, Redirects, /*secondsToWait*/ 0, - /*memoryLimit*/ 0, ErrMsg, ExecutionFailed); + if (!ClangDriverMain) + Argv.push_back(nullptr); + } else { + // If the command is too large, we need to put arguments in a response file. + std::string RespContents; + llvm::raw_string_ostream SS(RespContents); + + // Write file contents and build the Argv vector + writeResponseFile(SS); + buildArgvForResponseFile(Argv); + if (!ClangDriverMain) + 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; + } } - // 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 (ClangDriverMain) { + // This simply indicates that the program couldn't start, which isn't + // applicable here. if (ExecutionFailed) - *ExecutionFailed = true; - return -1; - } + *ExecutionFailed = false; - auto Args = llvm::toStringRefArray(Argv.data()); - return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects, - /*secondsToWait*/ 0, - /*memoryLimit*/ 0, ErrMsg, ExecutionFailed); + static bool CRCEnabled{}; + if (!CRCEnabled) { + llvm::CrashRecoveryContext::Enable(); + CRCEnabled = true; + } + + llvm::CrashRecoveryContext CRC; + CRC.EnableExceptionHandler = true; + + const void *PrettyState = llvm::SavePrettyStackState(); + + int Ret = 0; + auto ExecuteClangMain = [&]() { Ret = ClangDriverMain(Argv); }; + + // Reenter a variant of main() instead of starting up a new process + if (!CRC.RunSafely(ExecuteClangMain)) { + llvm::RestorePrettyStackState(PrettyState); + return CRC.RetCode; + } + return Ret; + } else { + auto Args = llvm::toStringRefArray(Argv.data()); + return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects, + /*secondsToWait*/ 0, + /*memoryLimit*/ 0, ErrMsg, + ExecutionFailed); + } } FallbackCommand::FallbackCommand(const Action &Source_, const Tool &Creator_, Index: clang/lib/Driver/Driver.cpp =================================================================== --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -91,6 +91,8 @@ using namespace clang; using namespace llvm::opt; +LLVM_THREAD_LOCAL Driver::ClangDriverMainFunc Driver::Main; + // static std::string Driver::GetResourcesPath(StringRef BinaryPath, StringRef CustomResourceDir) { @@ -1333,6 +1335,10 @@ BuildJobs(C); + // Call a new clang.exe for rendering the preprocessed output, to minimize the + // chances of a crash (in case this function was called due to an exception) + Command::ReenterTool = false; + // If there were errors building the compilation, quit now. if (Trap.hasErrorOccurred()) { Diag(clang::diag::note_drv_command_failed_diag_msg) Index: clang/include/clang/Driver/Job.h =================================================================== --- clang/include/clang/Driver/Job.h +++ clang/include/clang/Driver/Job.h @@ -130,6 +130,11 @@ /// Set whether to print the input filenames when executing. void setPrintInputFilenames(bool P) { PrintInputFilenames = P; } + + /// When set, we will try re-entering this process for the CC1 command + /// execution, instead of creating a new process. This is an optimization for + /// speed. + static LLVM_THREAD_LOCAL bool ReenterTool; }; /// Like Command, but with a fallback which is executed in case Index: clang/include/clang/Driver/Driver.h =================================================================== --- clang/include/clang/Driver/Driver.h +++ clang/include/clang/Driver/Driver.h @@ -204,6 +204,10 @@ /// Whether the driver is generating diagnostics for debugging purposes. unsigned CCGenDiagnostics : 1; + /// Callback to an execution point in main(), after InitLLVM called + typedef int(*ClangDriverMainFunc)(SmallVectorImpl<const char *> &Args); + static LLVM_THREAD_LOCAL ClangDriverMainFunc Main; + private: /// Raw target triple. std::string TargetTriple;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits