From 7b1a5dfc587bb00154ca79f99abcdd69de0f89e7 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 18 Jul 2024 14:36:29 -0700 Subject: [PATCH] Try some crash recovery in autoupdate threads. (#4147) At present, I think if we crash from multiple threads in parallel, it can lead to the stack trace not being printed out. Using CrashRecoveryContext here seems to more successfully print a stack trace on errors, which I'm hoping will ease debugging. i.e., before: ``` ----------------------------------------------------------------------------- .Please report issues to https://github.com/carbon-language/carbon-lang/issues and include the crash backtrace. Stack dump: 0. performing autoupdate for toolchain/check/testdata/alias/no_prelude/import_order.carbon 1. Program arguments: compile --phase=check --dump-sem-ir --no-prelude-import --exclude-dump-file-prefix=/usr/local/google/home/jperkins/.cache/bazel/_bazel_jper.kins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/b.in/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/core a.carbon b.carbon .CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() external/bazel_tools/tools/test/test-setup.sh: line 328: 1151859 Aborted "${TEST_PATH}" "$@" 2>&1 ``` (EOF) after: ``` ----------------------------------------------------------------------------- Please report issues to https://github.com/carbon-language/carbon-lang/issues and include the crash backtrace. Stack dump: 0. performing autoupdate for toolchain/check/testdata/alias/no_prelude/import_order.carbon 1. Program arguments: compile --phase=check --dump-sem-ir --no-prelude-import --exclude-dump-file-prefix=/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/core a.carbon b.carbon ..CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() ......CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() .....................................................................................CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() .CHECK failure at ./toolchain/sem_ir/ids.h:140: is_valid() ............................ #0 0x000056045ee22d7d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x731dd7d) ``` (elided the full stack trace) --- testing/file_test/file_test_base.cpp | 79 ++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 8acd146ad2b8..48d4954f77ee 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -20,6 +20,7 @@ #include "common/init_llvm.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/Twine.h" +#include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/PrettyStackTrace.h" @@ -846,6 +847,61 @@ static auto GetTests() -> llvm::SmallVector { return all_tests; } +// Runs autoupdate for the given tests. This is multi-threaded to try to get a +// little extra speed. +static auto RunAutoupdate(llvm::StringRef exe_path, + llvm::ArrayRef tests, + FileTestFactory& test_factory) -> int { + llvm::CrashRecoveryContext::Enable(); + llvm::DefaultThreadPool pool( + {.ThreadsRequested = absl::GetFlag(FLAGS_threads)}); + + // Guard access to both `llvm::errs` and `crashed`. + std::mutex mutex; + bool crashed = false; + + for (const auto& test_name : tests) { + pool.async([&test_factory, &mutex, &exe_path, &crashed, test_name] { + // If any thread crashed, don't try running more. + { + std::unique_lock lock(mutex); + if (crashed) { + return; + } + } + + // Use a crash recovery context to try to get a stack trace when + // multiple threads may crash in parallel, which otherwise leads to the + // program aborting without printing a stack trace. + llvm::CrashRecoveryContext crc; + crc.DumpStackAndCleanupOnFailure = true; + bool thread_crashed = !crc.RunSafely([&] { + std::unique_ptr test( + test_factory.factory_fn(exe_path, test_name)); + auto result = test->Autoupdate(); + + std::unique_lock lock(mutex); + if (result.ok()) { + llvm::errs() << (*result ? "!" : "."); + } else { + llvm::errs() << "\n" << result.error().message() << "\n"; + } + }); + if (thread_crashed) { + std::unique_lock lock(mutex); + crashed = true; + } + }); + } + + pool.wait(); + if (crashed) { + return EXIT_FAILURE; + } + llvm::errs() << "\nDone!\n"; + return EXIT_SUCCESS; +} + // Implements main() within the Carbon::Testing namespace for convenience. static auto Main(int argc, char** argv) -> int { Carbon::InitLLVM init_llvm(argc, argv); @@ -887,28 +943,7 @@ static auto Main(int argc, char** argv) -> int { llvm::SmallVector tests = GetTests(); auto test_factory = GetFileTestFactory(); if (absl::GetFlag(FLAGS_autoupdate)) { - llvm::DefaultThreadPool pool( - {.ThreadsRequested = absl::GetFlag(FLAGS_threads)}); - std::mutex errs_mutex; - - for (const auto& test_name : tests) { - pool.async([&test_factory, &errs_mutex, &exe_path, test_name] { - std::unique_ptr test( - test_factory.factory_fn(exe_path, test_name)); - auto result = test->Autoupdate(); - - // Guard access to llvm::errs, which is not thread-safe. - std::unique_lock lock(errs_mutex); - if (result.ok()) { - llvm::errs() << (*result ? "!" : "."); - } else { - llvm::errs() << "\n" << result.error().message() << "\n"; - } - }); - } - pool.wait(); - llvm::errs() << "\nDone!\n"; - return EXIT_SUCCESS; + return RunAutoupdate(exe_path, tests, test_factory); } else if (absl::GetFlag(FLAGS_dump_output)) { for (const auto& test_name : tests) { std::unique_ptr test(