Skip to content

Commit

Permalink
Revert of In official builds, let CHECK(false) crash instead of calli…
Browse files Browse the repository at this point in the history
…ng BreakDebugger. (patchset chromium#2 id:20001 of https://codereview.chromium.org/1982123002/ )

Reason for revert:
Unfortunately crrev.com/1982123002 causes loss of
crash reports on Android arm64 (and supposedly also CrOS).
This is because __builtin_trap() raises a SIGILL on x86 and
arm but SIGTRAP on arm64. Breakpad does not handle SIGTRAP (yet).
Temporarily reverting this CL until SIGTRAP support for breakpad lands.

BUG=599867,614865

Original issue's description:
> In official builds, let CHECK(false) crash instead of calling BreakDebugger.
>
> This should save some binary size and make things a bit faster, without ill
> effects.
>
> See bug comment 15, and brettw's and my comments on
> "[blink-dev] Update of wtf/Assertions.h, and ASSERT macros deprecation"
>
> BUG=599867
>
> Committed: https://crrev.com/481a8ec8b24df24795c63fd4ec26f3670d516db8
> Cr-Commit-Position: refs/heads/master@{#394035}

TBR=danakj@chromium.org,thakis@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=599867

Review-Url: https://codereview.chromium.org/2046593002
Cr-Commit-Position: refs/heads/master@{#398084}
  • Loading branch information
primiano authored and Commit bot committed Jun 6, 2016
1 parent 42e063e commit 6ff194f
Showing 1 changed file with 4 additions and 10 deletions.
14 changes: 4 additions & 10 deletions base/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,17 +456,11 @@ class CheckOpResult {
// Make all CHECK functions discard their log strings to reduce code
// bloat, and improve performance, for official release builds.

#if defined(COMPILER_GCC) || __clang__
#define LOGGING_CRASH() __builtin_trap()
#else
#define LOGGING_CRASH() ((void)(*(volatile char*)0 = 0))
#endif

// This is not calling BreakDebugger since this is called frequently, and
// calling an out-of-line function instead of a noreturn inline macro prevents
// compiler optimizations.
// TODO(akalin): This would be more valuable if there were some way to
// remove BreakDebugger() from the backtrace, perhaps by turning it
// into a macro (like __debugbreak() on Windows).
#define CHECK(condition) \
!(condition) ? LOGGING_CRASH() : EAT_STREAM_PARAMETERS
!(condition) ? ::base::debug::BreakDebugger() : EAT_STREAM_PARAMETERS

#define PCHECK(condition) CHECK(condition)

Expand Down

0 comments on commit 6ff194f

Please sign in to comment.