From aaab93e367c462fecb481d2b98880953d0d6d405 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Mon, 11 Mar 2019 09:49:05 -0700 Subject: [PATCH] Fix no-return false positives in static analyzer build There were about 800 false positive issues in the clang status analyzer build caused by the fact that various forms of asserts were not considered by the analyzer as not returning. This change adds __attribute__((analyzer_noreturn)) (wrapped in a macro) to those assertion functions. Commit migrated from https://github.com/dotnet/coreclr/commit/c4ca1ddb2413a354e8f49fff4680f175a02e7d8e --- src/coreclr/src/inc/check.h | 4 ++-- src/coreclr/src/inc/debugmacros.h | 2 ++ src/coreclr/src/inc/palclr.h | 2 ++ src/coreclr/src/jit/error.h | 2 ++ src/coreclr/src/jit/host.h | 1 + src/coreclr/src/pal/inc/pal.h | 6 ++++++ src/coreclr/src/pal/src/include/pal/dbgmsg.h | 14 ++++++++++---- 7 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/coreclr/src/inc/check.h b/src/coreclr/src/inc/check.h index b1fdba9..b401a25 100644 --- a/src/coreclr/src/inc/check.h +++ b/src/coreclr/src/inc/check.h @@ -473,7 +473,7 @@ CHECK CheckValue(TYPENAME &val) // in a free build they are passed through to the compiler to use in optimization. //-------------------------------------------------------------------------------- -#if defined(_PREFAST_) || defined(_PREFIX_) +#if defined(_PREFAST_) || defined(_PREFIX_) || defined(__clang_analyzer__) #define COMPILER_ASSUME_MSG(_condition, _message) if (!(_condition)) __UNREACHABLE(); #define COMPILER_ASSUME_MSGF(_condition, args) if (!(_condition)) __UNREACHABLE(); #else @@ -561,7 +561,7 @@ CHECK CheckValue(TYPENAME &val) # define __UNREACHABLE() __assume(0) #endif #else -#define __UNREACHABLE() do { } while(true) +#define __UNREACHABLE() __builtin_unreachable() #endif #ifdef _DEBUG_IMPL diff --git a/src/coreclr/src/inc/debugmacros.h b/src/coreclr/src/inc/debugmacros.h index ef809cb..655fba5 100644 --- a/src/coreclr/src/inc/debugmacros.h +++ b/src/coreclr/src/inc/debugmacros.h @@ -13,6 +13,7 @@ #include "stacktrace.h" #include "debugmacrosext.h" +#include "palclr.h" #undef _ASSERTE #undef VERIFY @@ -29,6 +30,7 @@ bool GetStackTraceAtContext(SString & s, struct _CONTEXT * pContext); void _cdecl DbgWriteEx(LPCTSTR szFmt, ...); bool _DbgBreakCheck(LPCSTR szFile, int iLine, LPCSTR szExpr, BOOL fConstrained = FALSE); +ANALYZER_NORETURN extern VOID DbgAssertDialog(const char *szFile, int iLine, const char *szExpr); #define TRACE_BUFF_SIZE (cchMaxAssertStackLevelStringLen * cfrMaxAssertStackLevels + cchMaxAssertExprLen + 1) diff --git a/src/coreclr/src/inc/palclr.h b/src/coreclr/src/inc/palclr.h index 60b9730..3b42ac5 100644 --- a/src/coreclr/src/inc/palclr.h +++ b/src/coreclr/src/inc/palclr.h @@ -53,6 +53,8 @@ #endif // !_MSC_VER #endif // !NOINLINE +#define ANALYZER_NORETURN + // // CPP_ASSERT() can be used within a class definition, to perform a // compile-time assertion involving private names within the class. diff --git a/src/coreclr/src/jit/error.h b/src/coreclr/src/jit/error.h index 944eaca..2a3b60f 100644 --- a/src/coreclr/src/jit/error.h +++ b/src/coreclr/src/jit/error.h @@ -73,11 +73,13 @@ extern void DECLSPEC_NORETURN noWayAssertBody(const char* cond, const char* file // Conditionally invoke the noway assert body. The conditional predicate is evaluated using a method on the tlsCompiler. // If a noway_assert is hit, we ask the Compiler whether to raise an exception (i.e., conditionally raise exception.) // To have backward compatibility between v4.5 and v4.0, in min-opts we take a shot at codegen rather than rethrow. +ANALYZER_NORETURN extern void noWayAssertBodyConditional( #ifdef FEATURE_TRACELOGGING const char* file, unsigned line #endif ); +ANALYZER_NORETURN extern void noWayAssertBodyConditional(const char* cond, const char* file, unsigned line); // Define MEASURE_NOWAY to 1 to enable code to count and rank individual noway_assert calls by occurrence. diff --git a/src/coreclr/src/jit/host.h b/src/coreclr/src/jit/host.h index f7cbdcf..1b090a1 100644 --- a/src/coreclr/src/jit/host.h +++ b/src/coreclr/src/jit/host.h @@ -35,6 +35,7 @@ void gcDump_logf(const char* fmt, ...); void logf(unsigned level, const char* fmt, ...); +ANALYZER_NORETURN extern "C" void __cdecl assertAbort(const char* why, const char* file, unsigned line); #undef assert diff --git a/src/coreclr/src/pal/inc/pal.h b/src/coreclr/src/pal/inc/pal.h index cdb3760..26c9e3f 100644 --- a/src/coreclr/src/pal/inc/pal.h +++ b/src/coreclr/src/pal/inc/pal.h @@ -168,6 +168,12 @@ typedef PVOID NATIVE_LIBRARY_HANDLE; #define DECLSPEC_NORETURN PAL_NORETURN +#ifdef __clang_analyzer__ +#define ANALYZER_NORETURN __attribute((analyzer_noreturn)) +#else +#define ANALYZER_NORETURN +#endif + #if !defined(_MSC_VER) || defined(SOURCE_FORMATTING) #define __assume(x) (void)0 #define __annotation(x) diff --git a/src/coreclr/src/pal/src/include/pal/dbgmsg.h b/src/coreclr/src/pal/src/include/pal/dbgmsg.h index 641de90..ad3dd54 100644 --- a/src/coreclr/src/pal/src/include/pal/dbgmsg.h +++ b/src/coreclr/src/pal/src/include/pal/dbgmsg.h @@ -355,6 +355,15 @@ bool DBG_ShouldCheckStackAlignment(); #else /* defined(_DEBUG) */ +ANALYZER_NORETURN +inline void AssertBreak() +{ + if(g_Dbg_asserts_enabled) + { + DebugBreak(); + } +} + #define ASSERT(...) \ { \ __ASSERT_ENTER(); \ @@ -362,10 +371,7 @@ bool DBG_ShouldCheckStackAlignment(); { \ DBG_printf(defdbgchan,DLI_ASSERT,TRUE,__FUNCTION__,__FILE__,__LINE__,__VA_ARGS__); \ } \ - if(g_Dbg_asserts_enabled) \ - { \ - DebugBreak(); \ - } \ + AssertBreak(); \ } #define _ASSERT(expr) do { if (!(expr)) { ASSERT(""); } } while(0) -- 2.7.4