Add new macro "ANALYZER_ASSUME_TRUE" and integrate with all assertion types.

base/logging.h

Index: base/logging.h
diff --git a/base/logging.h b/base/logging.h
index 5174e6d375540cd0f0bf355034c17812a66bf37b..3f4f5423b2410dedd2124699dd9894fe1de1f01f 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -289,6 +289,29 @@ typedef bool (*LogMessageHandlerFunction)(int severity,
 BASE_EXPORT void SetLogMessageHandler(LogMessageHandlerFunction handler);
 BASE_EXPORT LogMessageHandlerFunction GetLogMessageHandler();
 
+// ANALYSIS_MARK_ASSERTION(...) adds compiler-specific annotations to an
+// asserted condition which stop static analysis if the asserted condition is
+// untrue.
+#if defined(__clang_analyzer__)
+
+inline constexpr bool AnalyzerNoReturn() __attribute__((analyzer_noreturn)) {
+  return false;
+}
+
+#define ANALYSIS_MARK_ASSERTION(condition) \
+  (!!(condition) || logging::AnalyzerNoReturn())
+
+#elif defined(_PREFAST_) && defined(OS_WIN)
+
+#define ANALYSIS_MARK_ASSERTION(condition) __analysis_assume(!!(condition))
+
+#else  // !_PREFAST_ & !__clang_analyzer__
+
+// No analyzer attached - just pass it through.
+#define ANALYSIS_MARK_ASSERTION(condition) !!(condition)

[brucedawson, 2017/02/02 21:37:57]

Why the double ! here? They are needed in the /analyze case because of a VC++
/analyze bug (which may be fixed now) but they are probably not needed in the
normal case, especially since this actually leads to a triple ! after macro
expansion in some places. The double !! should not be needed outside of
/analyze.

[Kevin M, 2017/02/02 21:46:43]

Wasn't sure if casting to a bool in this way yielded more efficient code.
Anyways, done here and on line 302.

+
+#endif  // !_PREFAST_ & !__clang_analyzer__
+
 typedef int LogSeverity;
 const LogSeverity LOG_VERBOSE = -1;  // This is level 1 verbosity
 // Note: the log severities are used to index into the array of names,
@@ -408,8 +431,9 @@ const LogSeverity LOG_0 = LOG_ERROR;
 
 // TODO(akalin): Add more VLOG variants, e.g. VPLOG.
 
-#define LOG_ASSERT(condition)  \
-  LOG_IF(FATAL, !(condition)) << "Assert failed: " #condition ". "
+#define LOG_ASSERT(condition)                        \
+  LOG_IF(FATAL, !ANALYSIS_MARK_ASSERTION(condition)) \
+      << "Assert failed: " #condition ". "
 
 #if defined(OS_WIN)
 #define PLOG_STREAM(severity) \
@@ -482,8 +506,9 @@ class CheckOpResult {
 // 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.
-#define CHECK(condition) \
-  UNLIKELY(!(condition)) ? IMMEDIATE_CRASH() : EAT_STREAM_PARAMETERS
+#define CHECK(condition)                                            \
+  UNLIKELY(!ANALYSIS_MARK_ASSERTION(condition)) ? IMMEDIATE_CRASH() \
+                                                : EAT_STREAM_PARAMETERS
 
 #define PCHECK(condition) CHECK(condition)
 
@@ -492,33 +517,29 @@ class CheckOpResult {
 #else  // !(OFFICIAL_BUILD && NDEBUG)
 
 #if defined(_PREFAST_) && defined(OS_WIN)
-// Use __analysis_assume to tell the VC++ static analysis engine that
-// assert conditions are true, to suppress warnings.  The LAZY_STREAM
-// parameter doesn't reference 'condition' in /analyze builds because
-// this evaluation confuses /analyze. The !! before condition is because
+// The LAZY_STREAM parameter doesn't reference 'condition' in /analyze builds
+// because this evaluation confuses /analyze. The !! before condition is because
 // __analysis_assume gets confused on some conditions:
 // http://randomascii.wordpress.com/2011/09/13/analyze-for-visual-studio-the-ugly-part-5/
 
-#define CHECK(condition)                \
-  __analysis_assume(!!(condition)),     \
-  LAZY_STREAM(LOG_STREAM(FATAL), false) \
-  << "Check failed: " #condition ". "
+#define CHECK(condition)             \
+  ANALYSIS_MARK_ASSERTION(condition) \
+  , LAZY_STREAM(LOG_STREAM(FATAL), false) << "Check failed: " #condition ". "
 
-#define PCHECK(condition)                \
-  __analysis_assume(!!(condition)),      \
-  LAZY_STREAM(PLOG_STREAM(FATAL), false) \
-  << "Check failed: " #condition ". "
+#define PCHECK(condition)            \
+  ANALYSIS_MARK_ASSERTION(condition) \
+  , LAZY_STREAM(PLOG_STREAM(FATAL), false) << "Check failed: " #condition ". "
 
 #else  // _PREFAST_
 
 // Do as much work as possible out of line to reduce inline code size.
 #define CHECK(condition)                                                      \
   LAZY_STREAM(::logging::LogMessage(__FILE__, __LINE__, #condition).stream(), \
-              !(condition))
+              ANALYSIS_MARK_ASSERTION(condition))

[brucedawson, 2017/02/02 21:37:57]

I think you've gone from !(condition) to !!(condition), thus reversing the sense
of the check.

[Kevin M, 2017/02/02 21:46:44]

Done.

 
-#define PCHECK(condition)                       \
-  LAZY_STREAM(PLOG_STREAM(FATAL), !(condition)) \
-  << "Check failed: " #condition ". "
+#define PCHECK(condition)                                             \
+  LAZY_STREAM(PLOG_STREAM(FATAL), ANALYSIS_MARK_ASSERTION(condition)) \

[brucedawson, 2017/02/02 21:37:57]

I think you've gone from !(condition) to !!(condition), thus reversing the sense
of the check.

[Kevin M, 2017/02/02 21:46:44]

Done.

+      << "Check failed: " #condition ". "
 
 #endif  // _PREFAST_
 
@@ -716,60 +737,27 @@ const LogSeverity LOG_DCHECK = LOG_INFO;
 // Note that the definition of the DCHECK macros depends on whether or not
 // DCHECK_IS_ON() is true. When DCHECK_IS_ON() is false, the macros use
 // EAT_STREAM_PARAMETERS to avoid expressions that would create temporaries.
-
-#if defined(_PREFAST_) && defined(OS_WIN)
-// See comments on the previous use of __analysis_assume.
-
-#define DCHECK(condition)                                               \
-  __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(__clang_analyzer__)
-
-// Keeps the static analyzer from proceeding along the current codepath,
-// otherwise false positive errors may be generated  by null pointer checks.
-inline constexpr bool AnalyzerNoReturn() __attribute__((analyzer_noreturn)) {
-  return false;
-}
-
-#define DCHECK(condition)                                                     \
-  LAZY_STREAM(                                                                \
-      LOG_STREAM(DCHECK),                                                     \
-      DCHECK_IS_ON() ? (logging::AnalyzerNoReturn() || !(condition)) : false) \
-      << "Check failed: " #condition ". "
-
-#define DPCHECK(condition)                                                    \
-  LAZY_STREAM(                                                                \
-      PLOG_STREAM(DCHECK),                                                    \
-      DCHECK_IS_ON() ? (logging::AnalyzerNoReturn() || !(condition)) : false) \
-      << "Check failed: " #condition ". "
-
-#else
-
 #if DCHECK_IS_ON()
 
-#define DCHECK(condition)                       \
-  LAZY_STREAM(LOG_STREAM(DCHECK), !(condition)) \
+#define DCHECK(condition)                                                   \
+  LAZY_STREAM(LOG_STREAM(DCHECK),                                           \
+              DCHECK_IS_ON() ? !ANALYSIS_MARK_ASSERTION(condition) : false) \
       << "Check failed: " #condition ". "
-#define DPCHECK(condition)                       \
-  LAZY_STREAM(PLOG_STREAM(DCHECK), !(condition)) \
+
+#define DPCHECK(condition)                                                  \
+  LAZY_STREAM(PLOG_STREAM(DCHECK),                                          \
+              DCHECK_IS_ON() ? !ANALYSIS_MARK_ASSERTION(condition) : false) \
       << "Check failed: " #condition ". "
 
 #else  // DCHECK_IS_ON()
 
-#define DCHECK(condition) EAT_STREAM_PARAMETERS << !(condition)
-#define DPCHECK(condition) EAT_STREAM_PARAMETERS << !(condition)
+#define DCHECK(condition) \
+  EAT_STREAM_PARAMETERS << !ANALYSIS_MARK_ASSERTION(condition)
+#define DPCHECK(condition) \
+  EAT_STREAM_PARAMETERS << !ANALYSIS_MARK_ASSERTION(condition)
 
 #endif  // DCHECK_IS_ON()
 
-#endif
-
 // Helper macro for binary operators.
 // Don't use this macro directly in your code, use DCHECK_EQ et al below.
 // The 'switch' is used to prevent the 'else' from being ambiguous when the
@@ -993,7 +981,7 @@ BASE_EXPORT void RawLog(int level, const char* message);
 
 #define RAW_CHECK(condition)                               \
   do {                                                     \
-    if (!(condition))                                      \
+    if (!ANALYSIS_MARK_ASSERTION(condition))               \
       ::logging::RawLog(::logging::LOG_FATAL,              \
                         "Check failed: " #condition "\n"); \
   } while (0)