Patch to try dump-on-DCHECK.

base/logging.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_LOGGING_H_
 #define BASE_LOGGING_H_
 
 #include <stddef.h>
 
 #include <cassert>
@@ skipping 619 matching lines @@
   LAZY_STREAM(PLOG_STREAM(severity), DLOG_IS_ON(severity))
 
 #define DVLOG(verboselevel) DVLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
 
 #define DVPLOG(verboselevel) DVPLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
 
 // Definitions for DCHECK et al.
 
 #if DCHECK_IS_ON()
 
+// If DCHECK is configured to dump-without-crashing then omit these, so that
+// anything using them directly will break the build. See crbug.com/596231.
+#if !defined(DCHECK_IS_DUMP_WITHOUT_CRASH)
+
 #define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \
   COMPACT_GOOGLE_LOG_EX_FATAL(ClassName , ##__VA_ARGS__)
 #define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_FATAL
 const LogSeverity LOG_DCHECK = LOG_FATAL;
 
+#endif  // !defined(DCHECK_IS_DUMP_WITHOUT_CRASH)
+
 #else  // DCHECK_IS_ON()
 
 // These are just dummy values.
 #define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \
   COMPACT_GOOGLE_LOG_EX_INFO(ClassName , ##__VA_ARGS__)
 #define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_INFO
 const LogSeverity LOG_DCHECK = LOG_INFO;
 
 #endif  // DCHECK_IS_ON()
 
 // DCHECK et al. make sure to reference |condition| regardless of
 // whether DCHECKs are enabled; this is so that we don't get unused
 // variable warnings if the only use of a variable is in a DCHECK.
 // This behavior is different from DLOG_IF et al.
 
 #if defined(_PREFAST_) && defined(OS_WIN)

[danakj, 2016/03/25 23:21:32]

Seems like dumping should win here.

[Wez, 2016/03/25 23:57:29]

I've added an #error to break things explicitly if both flags are set, since we
don't expect that will happen; better than going ahead and compiling and having
weird Prefast output.

 // See comments on the previous use of __analysis_assume.
 
 #define DCHECK(condition)                                               \

[danakj, 2016/03/24 00:10:34]

But in this case it won't log?

[Wez, 2016/03/25 23:57:30]

Not sure what you mean, but PREFAST + DCHECK_IS_DUMP is now officially an
unsupported config!

   __analysis_assume(!!(condition)),                                     \
   LAZY_STREAM(LOG_STREAM(DCHECK), false)                                \
   << "Check failed: " #condition ". "
 
 #define DPCHECK(condition)                                              \
   __analysis_assume(!!(condition)),                                     \
   LAZY_STREAM(PLOG_STREAM(DCHECK), false)                               \
   << "Check failed: " #condition ". "
 
+#elif defined(DCHECK_IS_DUMP_WITHOUT_CRASH)
+// DCHECK is configured to dump-without-crashing, rather than logging.
+// See crbug.com/596231.
+
+BASE_EXPORT void DCheckDumpWithoutCrashing();
+
+#define DCHECK(condition)                                       \
+  (condition) ? (void)0 : logging::DCheckDumpWithoutCrashing(), \

[danakj, 2016/03/24 00:10:34]

No DCHECK_IS_ON() check here? This will make DCHECK always run.

[danakj, 2016/03/25 23:21:32]

I still think this should be

DCHECK_IS_ON() && (condition) ? (void)0 : logging::DCheckDump

We talked about this a bunch yesterday, and I thought we agreed that this
shouldn't change the enabled-ness of DCHECK(). Am I misremembering? 

[Wez, 2016/03/25 23:57:29]

The patch forces DCHECK on, so the effect is the same, but I've added a
DCHECK_IS_ON condition to the #elif, so that in the off case we compile the
normal way, rather than risk DCHECK_IS_DUMP changing the behaviour of the "off"
case in some way.

+      EAT_STREAM_PARAMETERS
+
+// Since we're uploading a crash dump, not logging, DPCHECK behaves identically.
+#define DPCHECK(condition) DCHECK(condition)
+
 #else  // _PREFAST_
 
 #define DCHECK(condition)                                                \
   LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \
       << "Check failed: " #condition ". "
 
 #define DPCHECK(condition)                                                \
   LAZY_STREAM(PLOG_STREAM(DCHECK), DCHECK_IS_ON() ? !(condition) : false) \
       << "Check failed: " #condition ". "
 
 #endif  // _PREFAST_
 
 // Helper macro for binary operators.
 // Don't use this macro directly in your code, use DCHECK_EQ et al below.
+
+#if defined(DCHECK_IS_DUMP_WITHOUT_CRASH)
+// DCHECK is configured to dump-without-crashing, rather than logging.
+// See crbug.com/596231.
+
+// Use logging::Check*Impl() to ensure that operator<<()s don't go unused.

[danakj, 2016/03/25 23:21:32]

Didn't you use DCHECK() to ensure that rather? DCHECK has the
EAT_STREAM_PARAMETERS.

You used Check*Impl because that's how you do DCHECK_OP, to compare the two
inputs, no?

[Wez, 2016/03/25 23:57:29]

No; you could implement this as DCHECK((val1) op (val2)) but I found that that
in one case the operator<<() for a particular DCHECK() << value logging case
went unused - I think DCHECK guarantees to evaluate (condition) even if it is
off, but not to evaluate the trailing logging, necessarily.

+#define DCHECK_OP(name, op, val1, val2) \
+  DCHECK(logging::Check##name##Impl((val1), (val2), #val1 " " #op " " #val2))

[danakj, 2016/03/24 00:10:34]

No DCHECK_IS_ON() check here?

[Wez, 2016/03/25 23:57:29]

Added a #if block check instead.

+
+#else
+
 // The 'switch' is used to prevent the 'else' from being ambiguous when the
 // macro is used in an 'if' clause such as:
 // if (a == 1)
 //   DCHECK_EQ(2, a);
 #define DCHECK_OP(name, op, val1, val2)                               \
   switch (0) case 0: default:                                         \
   if (logging::CheckOpResult true_if_passed =                         \
       DCHECK_IS_ON() ?                                                \
       logging::Check##name##Impl((val1), (val2),                      \
                                  #val1 " " #op " " #val2) : nullptr)  \
    ;                                                                  \
   else                                                                \
     logging::LogMessage(__FILE__, __LINE__, ::logging::LOG_DCHECK,    \
                         true_if_passed.message()).stream()
 
+#endif

[danakj, 2016/03/25 23:21:32]

comment what you're ending

[Wez, 2016/03/25 23:57:29]

Done.

+
 // Equality/Inequality checks - compare two values, and log a
 // LOG_DCHECK message including the two values when the result is not
 // as expected.  The values must have operator<<(ostream, ...)
 // defined.
 //
 // You may append to the error message like so:
 //   DCHECK_NE(1, 2) << ": The world must be ending!";
 //
 // We are very careful to ensure that each argument is evaluated exactly
 // once, and that anything which is legal to pass as a function argument is
@@ skipping 244 matching lines @@
 #elif NOTIMPLEMENTED_POLICY == 5
 #define NOTIMPLEMENTED() do {\
   static bool logged_once = false;\
   LOG_IF(ERROR, !logged_once) << NOTIMPLEMENTED_MSG;\
   logged_once = true;\
 } while(0);\
 EAT_STREAM_PARAMETERS
 #endif
 
 #endif  // BASE_LOGGING_H_