Remove NOTREACHED() dumping for official builds

This is to be merged to M111. A follow-up change will enable `enable_log_error_not_reached` that will allow us to dump again (with more data), and we can then decide if that should follow M112 all the way to stable before we make this fatal, or if we need to strip the useful debug info from reporting before we hit stable. Bug: 851128 Change-Id: Iec9bfe61ec8988d6e6017f7a64ec0401d0f541d1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4200079 Auto-Submit: Peter Boström <pbos@chromium.org> Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/main@{#1098455}
chromium · Jan 30, 2023 · a1b3c71 · a1b3c71
1 parent ee80a21
commit a1b3c71
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 19 deletions.
diff --git a/base/check.cc b/base/check.cc
@@ -197,10 +197,6 @@ NotReachedError NotReachedError::NotReached(const char* file, int line) {
   return NotReachedError(log_message);
 }
 
-void NotReachedError::TriggerNotReached() {
-  NotReached("", -1);
-}
-
 NotReachedError::~NotReachedError() = default;
 
 void RawCheck(const char* message) {

diff --git a/base/check.h b/base/check.h
@@ -101,10 +101,6 @@ class BASE_EXPORT NotReachedError : public CheckError {
  public:
   static NotReachedError NotReached(const char* file, int line);
 
-  // Used to trigger a NOTREACHED() without providing file or line while also
-  // discarding log-stream arguments. See base/notreached.h.
-  NOMERGE NOINLINE NOT_TAIL_CALLED static void TriggerNotReached();
-
   // TODO(crbug.com/851128): Mark [[noreturn]] once this is CHECK-fatal on all
   // builds.
   NOMERGE NOINLINE NOT_TAIL_CALLED ~NotReachedError();

diff --git a/base/check_unittest.cc b/base/check_unittest.cc
@@ -457,11 +457,6 @@ TEST(CheckDeathTest, NotReached) {
   // Expect LOG(ERROR) that looks like CHECK(false) with streamed params intact.
   EXPECT_LOG_ERROR(__LINE__, NOTREACHED() << "foo",
                    "Check failed: false. foo\n");
-#else
-  // Expect LOG(ERROR) that looks like CHECK(false) without file, line or
-  // streamed params.
-  EXPECT_LOG_ERROR_WITH_FILENAME("", -1, NOTREACHED() << "foo",
-                                 "Check failed: false. \n");
 #endif
 }
 

diff --git a/base/notreached.h b/base/notreached.h
@@ -16,19 +16,21 @@ namespace logging {
 // non-FATAL a crash report will be generated for the first NOTREACHED() that
 // hits per process.
 //
-// Outside DCHECK builds NOTREACHED() will LOG(ERROR) and also upload a crash
-// report without crashing in order to weed out prevalent NOTREACHED()s in the
-// wild before always turning NOTREACHED()s FATAL.
+// If `enable_log_error_not_reached` is enabled in the build then NOTREACHED()
+// builds will LOG(ERROR) and also upload a crash report without crashing in
+// order to weed out prevalent NOTREACHED()s in the wild before always turning
+// NOTREACHED()s FATAL.
 //
+// Without either of the above this currently does nothing. In the future (TODO
+// below) we should replace the #else branch here with base::ImmediateCrash() or
+// equivalent (and make ~NotReachedError()) FATAL and [[noreturn]].
 // TODO(crbug.com/851128): Turn NOTREACHED() FATAL and mark them [[noreturn]].
 #if CHECK_WILL_STREAM() || BUILDFLAG(ENABLE_LOG_ERROR_NOT_REACHED)
 #define NOTREACHED()   \
   CHECK_FUNCTION_IMPL( \
       ::logging::NotReachedError::NotReached(__FILE__, __LINE__), false)
 #else
-#define NOTREACHED()                                       \
-  (true) ? ::logging::NotReachedError::TriggerNotReached() \
-         : EAT_CHECK_STREAM_PARAMS()
+#define NOTREACHED() EAT_CHECK_STREAM_PARAMS()
 #endif
 
 // The NOTIMPLEMENTED() macro annotates codepaths which have not been