Skip to content

Commit

Permalink
(Partial) Make sure CHECK() failures crash
Browse files Browse the repository at this point in the history
This partial land excludes iOS and Windows where two tests currently
rely on aborting a LOG(FATAL). This is going in to not backslide in
platform-agnostic code.

This is trying to make sure that CHECK failures effectively don't
return before LOG(FATAL) is properly [[noreturn]]. A follow-up could
be to split out CheckError from DcheckError to make sure the
destructor can be marked [[noreturn]].

Note that this doesn't apply to the optimized version of CHECK which is
already a [[noreturn]] function call, but also doesn't involve
LOG(FATAL).

Bug: 1409729
Change-Id: I0f08668d741a7d2c0205846d2b6478876d46841d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4257852
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1107140}
  • Loading branch information
pbos authored and Chromium LUCI CQ committed Feb 18, 2023
1 parent af72fc8 commit 2cc59da
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions base/check.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,26 @@ std::ostream& CheckError::stream() {
}

CheckError::~CheckError() {
// TODO(crbug.com/1409729): Consider splitting out CHECK from DCHECK so that
// the destructor can be marked [[noreturn]] and we don't need to check
// severity in the destructor.
const bool is_fatal = log_message_->severity() == LOGGING_FATAL;
// Note: This function ends up in crash stack traces. If its full name
// changes, the crash server's magic signature logic needs to be updated.
// See cl/306632920.
delete log_message_;

// Make sure we crash even if LOG(FATAL) has been overridden.
// TODO(crbug.com/1409729): Include Windows and iOS here too. This is done in
// steps to prevent backsliding on platforms where this goes through CQ.
// Currently iOS is blocked by:
// * ListModelTest.InvalidIndexPath
// Windows is blocked by:
// * All/RenderProcessHostWriteableFileDeathTest.
// PassUnsafeWriteableExecutableFile/2
if (is_fatal && !BUILDFLAG(IS_WIN) && !BUILDFLAG(IS_IOS)) {
base::ImmediateCrash();
}
}

NotReachedError NotReachedError::NotReached(const char* file, int line) {
Expand Down

0 comments on commit 2cc59da

Please sign in to comment.