Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

Unified Diff: base/logging.h

Issue 2729503004: Add Clang static analysis control to all assert functions in logging.h (Closed)
Patch Set: Fixed up the code comments a bit Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/logging.h
diff --git a/base/logging.h b/base/logging.h
index 1dcb0f71f12ed6b9854524161e7555ea3ac609e1..f9ff4dbd7cfc4c20905d7a71b3d8efba195a3468 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -289,6 +289,30 @@ typedef bool (*LogMessageHandlerFunction)(int severity,
BASE_EXPORT void SetLogMessageHandler(LogMessageHandlerFunction handler);
BASE_EXPORT LogMessageHandlerFunction GetLogMessageHandler();
+// The ANALYZER_ASSUME_TRUE(bool arg) macro adds compiler-specific hints
+// to Clang which control what code paths are statically analyzed,
+// and is meant to be used in conjunction with assert & assert-like functions.
+// The expression is passed straight through if analysis isn't enabled.
+#if defined(__clang_analyzer__)
+
+inline constexpr bool AnalyzerNoReturn() __attribute__((analyzer_noreturn)) {
+ return false;
+}
+
+inline constexpr bool AnalyzerAssumeTrue(bool arg) {
+ // AnalyzerNoReturn() is invoked and analysis is terminated if |arg| is
+ // false.
+ return arg || AnalyzerNoReturn();
+}
+
+#define ANALYZER_ASSUME_TRUE(arg) ::logging::AnalyzerAssumeTrue(!!(arg))
+
+#else // !defined(__clang_analyzer__)
+
+#define ANALYZER_ASSUME_TRUE(arg) (arg)
+
+#endif // defined(__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 +432,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, !(ANALYZER_ASSUME_TRUE(condition))) \
+ << "Assert failed: " #condition ". "
#if defined(OS_WIN)
#define PLOG_STREAM(severity) \
@@ -585,10 +610,10 @@ class CheckOpResult {
// 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))
+ !ANALYZER_ASSUME_TRUE(condition))
-#define PCHECK(condition) \
- LAZY_STREAM(PLOG_STREAM(FATAL), !(condition)) \
+#define PCHECK(condition) \
+ LAZY_STREAM(PLOG_STREAM(FATAL), !ANALYZER_ASSUME_TRUE(condition)) \
<< "Check failed: " #condition ". "
#endif // _PREFAST_
@@ -685,17 +710,21 @@ std::string* MakeCheckOpString<std::string, std::string>(
// The (int, int) specialization works around the issue that the compiler
// will not instantiate the template version of the function on values of
// unnamed enum type - see comment below.
+//
+// The checked condition is wrapped with ANALYZER_ASSUME_TRUE, which under
+// static analysis builds, blocks analysis of the current path if the
+// condition is false.
#define DEFINE_CHECK_OP_IMPL(name, op) \
template <class t1, class t2> \
inline std::string* Check##name##Impl(const t1& v1, const t2& v2, \
const char* names) { \
- if (v1 op v2) \
+ if (ANALYZER_ASSUME_TRUE(v1 op v2)) \
return NULL; \
else \
return ::logging::MakeCheckOpString(v1, v2, names); \
} \
inline std::string* Check##name##Impl(int v1, int v2, const char* names) { \
- if (v1 op v2) \
+ if (ANALYZER_ASSUME_TRUE(v1 op v2)) \
return NULL; \
else \
return ::logging::MakeCheckOpString(v1, v2, names); \
@@ -798,35 +827,15 @@ const LogSeverity LOG_DCHECK = LOG_INFO;
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
+#else // !(defined(_PREFAST_) && defined(OS_WIN))
#if DCHECK_IS_ON()
-#define DCHECK(condition) \
- LAZY_STREAM(LOG_STREAM(DCHECK), !(condition)) \
+#define DCHECK(condition) \
+ LAZY_STREAM(LOG_STREAM(DCHECK), !ANALYZER_ASSUME_TRUE(condition)) \
<< "Check failed: " #condition ". "
-#define DPCHECK(condition) \
- LAZY_STREAM(PLOG_STREAM(DCHECK), !(condition)) \
+#define DPCHECK(condition) \
+ LAZY_STREAM(PLOG_STREAM(DCHECK), !ANALYZER_ASSUME_TRUE(condition)) \
<< "Check failed: " #condition ". "
#else // DCHECK_IS_ON()
@@ -836,7 +845,7 @@ inline constexpr bool AnalyzerNoReturn() __attribute__((analyzer_noreturn)) {
#endif // DCHECK_IS_ON()
-#endif
+#endif // defined(_PREFAST_) && defined(OS_WIN)
// Helper macro for binary operators.
// Don't use this macro directly in your code, use DCHECK_EQ et al below.
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698