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

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 271 matching lines @@
 
 // Sets the Log Message Handler that gets passed every log message before
 // it's sent to other log destinations (if any).
 // Returns true to signal that it handled the message and the message
 // should not be sent to other log destinations.
 typedef bool (*LogMessageHandlerFunction)(int severity,
     const char* file, int line, size_t message_start, const std::string& str);
 BASE_EXPORT void SetLogMessageHandler(LogMessageHandlerFunction handler);
 BASE_EXPORT LogMessageHandlerFunction GetLogMessageHandler();
 
+// ANALYSIS_MARK_ASSERTION(...) adds compiler-specific annotations to an

[Wez, 2017/02/03 20:51:52]

nit: Can we give this a more specific name; it's not immediately clear to me
what it means to "mark assertion".

Perhaps ANALYSIS_ASSUME_TRUE()?

femto-nit: I'd also suggest calling it ANALYZE[R]_<foo> rather than
ANALYSIS_<foo>, so that a trivial code-search for "analyz*" will match these as
well as the analyzer build flags in GN, etc.

+// 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)
+
+#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,
 // see log_severity_names.
 const LogSeverity LOG_INFO = 0;
 const LogSeverity LOG_WARNING = 1;
 const LogSeverity LOG_ERROR = 2;
 const LogSeverity LOG_FATAL = 3;
 const LogSeverity LOG_NUM_SEVERITIES = 4;
 
@@ skipping 99 matching lines @@
 
 #define VPLOG(verbose_level) \
   LAZY_STREAM(VPLOG_STREAM(verbose_level), VLOG_IS_ON(verbose_level))
 
 #define VPLOG_IF(verbose_level, condition) \
   LAZY_STREAM(VPLOG_STREAM(verbose_level), \
     VLOG_IS_ON(verbose_level) && (condition))
 
 // 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) \
   COMPACT_GOOGLE_LOG_EX_ ## severity(Win32ErrorLogMessage, \
       ::logging::GetLastSystemErrorCode()).stream()
 #elif defined(OS_POSIX)
 #define PLOG_STREAM(severity) \
   COMPACT_GOOGLE_LOG_EX_ ## severity(ErrnoLogMessage, \
       ::logging::GetLastSystemErrorCode()).stream()
 #endif
@@ skipping 52 matching lines @@
 // doing CHECK(FunctionWithSideEffect()) is a common idiom.
 
 #if defined(OFFICIAL_BUILD) && defined(NDEBUG)
 
 // Make all CHECK functions discard their log strings to reduce code bloat, and
 // improve performance, for official release builds.
 //
 // 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)
 
 #define CHECK_OP(name, op, val1, val2) CHECK((val1) op (val2))
 
 #else  // !(OFFICIAL_BUILD && NDEBUG)
 
 #if defined(_PREFAST_) && defined(OS_WIN)

[Wez, 2017/02/03 20:51:52]

Isn't the point of the ANALYSIS_... macro to allow us to have a single
definition of CHECK etc, which looks like the non-PREFAST case below?

-// 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 ". "

[Wez, 2017/02/03 20:51:52]

Not this CL: I'm confused; does CHECK ever actually fire in these PreFast
builds, given that we pass |false| to LAZY_STREAM()?

[Kevin M, 2017/02/03 22:37:23]

It doesn't, but the analysis output should be correct.

[Wez, 2017/02/06 21:04:39]

Feels cleaner to replace false with condition here, then, so that a PREFAST
build will still crash as intended.

But, as noted above, surely the point of ANALYZER_ASSUME_TRUE is to let us use
the non-PREFAST definition below, all the time, and not have this special-case
definition of CHECK et al at all?

 
-#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))
 
-#define PCHECK(condition)                       \
-  LAZY_STREAM(PLOG_STREAM(FATAL), !(condition)) \
-  << "Check failed: " #condition ". "
+#define PCHECK(condition)                                             \
+  LAZY_STREAM(PLOG_STREAM(FATAL), !ANALYSIS_MARK_ASSERTION(condition)) \
+      << "Check failed: " #condition ". "
 
 #endif  // _PREFAST_
 
 // Helper macro for binary operators.
 // Don't use this macro directly in your code, use CHECK_EQ et al below.
 // 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)
 //   CHECK_EQ(2, a);
 #define CHECK_OP(name, op, val1, val2)                                         \
@@ skipping 177 matching lines @@
 #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.
 //
 // 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 DCHECK_IS_ON()
 
-#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) \
+#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),                                                    \
-      DCHECK_IS_ON() ? (logging::AnalyzerNoReturn() || !(condition)) : false) \
-      << "Check failed: " #condition ". "
-
-#else
-
-#if DCHECK_IS_ON()
-
-#define DCHECK(condition)                       \
-  LAZY_STREAM(LOG_STREAM(DCHECK), !(condition)) \
-      << "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
 // macro is used in an 'if' clause such as:
 // if (a == 1)
 //   DCHECK_EQ(2, a);
 #if DCHECK_IS_ON()
 
 #define DCHECK_OP(name, op, val1, val2)                                \
   switch (0) case 0: default:                                          \
@@ skipping 203 matching lines @@
 BASE_EXPORT void CloseLogFile();
 
 // Async signal safe logging mechanism.
 BASE_EXPORT void RawLog(int level, const char* message);
 
 #define RAW_LOG(level, message) \
   ::logging::RawLog(::logging::LOG_##level, message)
 
 #define RAW_CHECK(condition)                               \
   do {                                                     \
-    if (!(condition))                                      \
+    if (!ANALYSIS_MARK_ASSERTION(condition))               \
       ::logging::RawLog(::logging::LOG_FATAL,              \
                         "Check failed: " #condition "\n"); \
   } while (0)
 
 #if defined(OS_WIN)
 // Returns true if logging to file is enabled.
 BASE_EXPORT bool IsLoggingToFileEnabled();
 
 // Returns the default log file path.
 BASE_EXPORT std::wstring GetLogFileFullPath();
@@ skipping 64 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_