Add a flag to make CHECK failures non-fatal for debugging. (#4835)

`toolchain/autoupdate_testdata.py --allow-check-fail` can now be used to
perform an autoupdate even if some `CARBON_CHECK`s are failing. What
this does will depend on how the toolchain behaves after the `CHECK`
failure, and of course there's no guarantees there, but this can be
useful if it's easier to debug the `CHECK` failure by looking at the
produced SemIR.

Internally, this uses `bazel build --config=non-fatal-checks`, which in
turn specifies a `--per_file_copt` for `check_internal.cpp`. The intent
here is that the rebuild required to enable or disable this mode is as
small as reasonably possible.

This mode is not compatible with `-c opt`, as it's important that check
failure calls are `[[noreturn]]` in `-c opt` mode.

---------

Co-authored-by: Jon Ross-Perkins <[email protected]>

Author: zygoloid

.bazelrc

@@ -28,6 +28,9 @@ build:clang-tidy --action_env=PATH --host_action_env=PATH
 # not firing in our normal builds.
 build:clang-tidy --copt=-Wno-unknown-pragmas
 
+# --config=non-fatal-checks makes CHECK failures not terminate compilation.
+build:non-fatal-checks --per_file_copt=common/check_internal.cpp@-DCARBON_NON_FATAL_CHECKS
+
 # Provide an alias for controlling the `carbon_*` Bazel rules' configuration. We
 # enable use of the target config here to make our build and tests more
 # efficient, see the documentation in //bazel/carbon_rules/BUILD for details.

common/check_internal.cpp

@@ -9,11 +9,6 @@
 
 namespace Carbon::Internal {
 
-// Prints the buffered message.
-static auto PrintAfterStackTrace(void* str) -> void {
-  llvm::errs() << reinterpret_cast<char*>(str);
-}
-
 auto CheckFailImpl(const char* kind, const char* file, int line,
                    const char* condition_str, llvm::StringRef extra_message)
     -> void {
@@ -23,15 +18,29 @@ auto CheckFailImpl(const char* kind, const char* file, int line,
       llvm::StringRef(condition_str).empty() ? "" : ": ", condition_str,
       extra_message.empty() ? "" : ": ", extra_message);
 
+  // This macro is defined by `--config=non-fatal-checks`.
+#ifdef CARBON_NON_FATAL_CHECKS
+#ifdef NDEBUG
+#error "--config=non-fatal-checks is incompatible with -c opt"
+#endif
+  // TODO: It'd be nice to print the LLVM PrettyStackTrace, but LLVM doesn't
+  // expose functionality to do so.
+  llvm::sys::PrintStackTrace(llvm::errs());
+
+  llvm::errs() << message;
+#else
   // Register another signal handler to print the message. This is because we
   // want it at the bottom of output, after LLVM's builtin stack output, rather
   // than the top.
-  llvm::sys::AddSignalHandler(PrintAfterStackTrace,
-                              const_cast<char*>(message.c_str()));
+  llvm::sys::AddSignalHandler(
+      [](void* str) { llvm::errs() << reinterpret_cast<char*>(str); },
+      const_cast<char*>(message.c_str()));
+
   // It's useful to exit the program with `std::abort()` for integration with
   // debuggers and other tools. We also assume LLVM's exit handling is
   // installed, which will stack trace on `std::abort()`.
   std::abort();
+#endif
 }
 
 }  // namespace Carbon::Internal

common/check_internal.h

@@ -31,17 +31,22 @@ CheckCondition(bool condition)
 // Implements the check failure message printing.
 //
 // This is out-of-line and will arrange to stop the program, print any debugging
-// information and this string.
+// information and this string. In `!NDEBUG` mode (`dbg` and `fastbuild`), check
+// failures can be made non-fatal by a build flag, so this is not `[[noreturn]]`
+// in that case.
 //
 // This API uses `const char*` C string arguments rather than `llvm::StringRef`
 // because we know that these are available as C strings and passing them that
 // way lets the code size of calling it be smaller: it only needs to materialize
 // a single pointer argument for each. The runtime cost of re-computing the size
 // should be minimal. The extra message however might not be compile-time
 // guaranteed to be a C string so we use a normal `StringRef` there.
-[[noreturn]] auto CheckFailImpl(const char* kind, const char* file, int line,
-                                const char* condition_str,
-                                llvm::StringRef extra_message) -> void;
+#ifdef NDEBUG
+[[noreturn]]
+#endif
+auto CheckFailImpl(const char* kind, const char* file, int line,
+                   const char* condition_str, llvm::StringRef extra_message)
+    -> void;
 
 // Allow converting format values; the default behaviour is to just pass them
 // through.
@@ -77,8 +82,11 @@ auto ConvertFormatValue(T&& t) {
 // `llvm::formatv` which handles all of the formatting of output.
 template <TemplateString Kind, TemplateString File, int Line,
           TemplateString ConditionStr, TemplateString FormatStr, typename... Ts>
-[[noreturn, gnu::cold, clang::noinline]] auto CheckFail(Ts&&... values)
-    -> void {
+#ifdef NDEBUG
+[[noreturn]]
+#endif
+[[gnu::cold, clang::noinline]] auto
+CheckFail(Ts&&... values) -> void {
   if constexpr (llvm::StringRef(FormatStr).empty()) {
     // Skip the format string rendering if empty. Note that we don't skip it
     // even if there are no values as we want to have consistent handling of
@@ -135,9 +143,10 @@ template <TemplateString Kind, TemplateString File, int Line,
 //
 // Similar to the check failure macro, but tags the message as a fatal one and
 // leaves the stringified condition empty.
-#define CARBON_INTERNAL_FATAL(...)                 \
-  CARBON_INTERNAL_CHECK_IMPL##__VA_OPT__(_FORMAT)( \
-      "FATAL", __FILE__, __LINE__, "" __VA_OPT__(, ) __VA_ARGS__)
+#define CARBON_INTERNAL_FATAL(...)                                  \
+  (CARBON_INTERNAL_CHECK_IMPL##__VA_OPT__(_FORMAT)(                 \
+       "FATAL", __FILE__, __LINE__, "" __VA_OPT__(, ) __VA_ARGS__), \
+   CARBON_INTERNAL_FATAL_NORETURN_SUFFIX())
 
 #ifdef NDEBUG
 // For `DCHECK` in optimized builds we have a dead check that we want to
@@ -153,6 +162,11 @@ template <TemplateString Kind, TemplateString File, int Line,
 
 #define CARBON_INTERNAL_DEAD_DCHECK_IMPL_FORMAT(format_str, ...) \
   Carbon::Internal::CheckFail<"", "", 0, "", "">(__VA_ARGS__)
+
+// The CheckFail function itself is noreturn in NDEBUG.
+#define CARBON_INTERNAL_FATAL_NORETURN_SUFFIX() void()
+#else
+#define CARBON_INTERNAL_FATAL_NORETURN_SUFFIX() std::abort()
 #endif
 
 #endif  // CARBON_COMMON_CHECK_INTERNAL_H_

toolchain/autoupdate_testdata.py

@@ -8,6 +8,7 @@
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
+import argparse
 import re
 import subprocess
 import sys
@@ -16,6 +17,7 @@
 
 def main() -> None:
     bazel = str(Path(__file__).parents[1] / "scripts" / "run_bazel.py")
+    configs = []
     # Use the most recently used build mode, or `fastbuild` if missing
     # `bazel-bin`.
     build_mode = "fastbuild"
@@ -37,11 +39,26 @@ def main() -> None:
         else:
             exit(f"Build mode not found in `bazel-bin` symlink: {link}")
 
+    # Parse arguments.
+    parser = argparse.ArgumentParser(__doc__)
+    parser.add_argument("--non-fatal-checks", action="store_true")
+    parser.add_argument("files", nargs="*")
+    args = parser.parse_args()
+
+    if args.allow_check_fail:
+        if build_mode == "opt":
+            exit(
+                "`--non-fatal-checks` is incompatible with inferred "
+                "`-c opt` build mode"
+            )
+        configs.append("--config=non-fatal-checks")
+
     argv = [
         bazel,
         "run",
         "-c",
         build_mode,
+        *configs,
         "--experimental_convenience_symlinks=ignore",
         "--ui_event_filters=-info,-stdout,-stderr,-finish",
         "//toolchain/testing:file_test",
@@ -50,18 +67,19 @@ def main() -> None:
     ]
     # Support specifying tests to update, such as:
     # ./autoupdate_testdata.py lex/**/*
-    if len(sys.argv) > 1:
+    if args.files:
         repo_root = Path(__file__).parents[1]
         file_tests = []
         # Filter down to just test files.
-        for f in sys.argv[1:]:
+        for f in args.files:
             if f.endswith(".carbon"):
                 path = str(Path(f).resolve().relative_to(repo_root))
                 if path.count("/testdata/"):
                     file_tests.append(path)
         if not file_tests:
             sys.exit(
-                f"Args do not seem to be test files; for example, {sys.argv[1]}"
+                "Args do not seem to be test files; for example, "
+                f"{args.files[0]}"
             )
         argv.append("--file_tests=" + ",".join(file_tests))
     # Provide an empty stdin so that the driver tests that read from stdin