From 3c8cd7fb7e965089a91e84d9156164171f4b7bc3 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 4 Nov 2024 08:22:15 +0100 Subject: [PATCH 1/4] [winpr,assert] add WINPR_ASSERT_AT Add a version of assert that allows setting the location (useful for macros or static functions wrapping something where the location of the call is more significant than the function the macro was used in) --- winpr/include/winpr/assert.h | 64 +++++++++++++++------------------- winpr/include/winpr/platform.h | 6 ++++ 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/winpr/include/winpr/assert.h b/winpr/include/winpr/assert.h index b9257cd0b..1fb855469 100644 --- a/winpr/include/winpr/assert.h +++ b/winpr/include/winpr/assert.h @@ -30,29 +30,17 @@ #include #if defined(WITH_VERBOSE_WINPR_ASSERT) && (WITH_VERBOSE_WINPR_ASSERT != 0) +#define winpr_internal_assert(cond, file, fkt, line) \ + do \ + { \ + if (!(cond)) \ + winpr_int_assert(#cond, (file), (fkt), (line)); \ + } while (0) + #ifdef __cplusplus extern "C" { #endif -#define WINPR_ASSERT(cond) \ - do \ - { \ - WINPR_PRAGMA_DIAG_PUSH \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ - WINPR_DO_PRAGMA(coverity compliance block \x28 deviate "CONSTANT_EXPRESSION_RESULT" \ - "WINPR_ASSERT" \x29 \ - \x28 deviate "NO_EFFECT" \ - "WINPR_ASSERT" \x29) \ - \ - if (!(cond)) \ - winpr_int_assert(#cond, __FILE__, __func__, __LINE__); \ - \ - WINPR_DO_PRAGMA(coverity compliance end_block "CONSTANT_EXPRESSION_RESULT" \ - "NO_EFFECT") \ - WINPR_PRAGMA_DIAG_POP \ - } while (0) static INLINE WINPR_NORETURN(void winpr_int_assert(const char* condstr, const char* file, const char* fkt, size_t line)) @@ -68,25 +56,29 @@ extern "C" #endif #else -#define WINPR_ASSERT(cond) \ - do \ - { \ - WINPR_PRAGMA_DIAG_PUSH \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ - WINPR_DO_PRAGMA(coverity compliance block \x28 deviate "CONSTANT_EXPRESSION_RESULT" \ - "WINPR_ASSERT" \x29 \ - \x28 deviate "NO_EFFECT" \ - "WINPR_ASSERT" \x29) \ - assert(cond); \ - \ - WINPR_DO_PRAGMA(coverity compliance end_block "CONSTANT_EXPRESSION_RESULT" \ - "NO_EFFECT") \ - WINPR_PRAGMA_DIAG_POP \ - } while (0) +#define winpr_internal_assert(cond, file, fkt, line) assert(cond) #endif +#define WINPR_ASSERT_AT(cond, file, fkt, line) \ + do \ + { \ + WINPR_PRAGMA_DIAG_PUSH \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ + WINPR_DO_COVERITY_PRAGMA(coverity compliance block \(deviate "CONSTANT_EXPRESSION_RESULT" \ + "WINPR_ASSERT" \) \ + \(deviate "NO_EFFECT" \ + "WINPR_ASSERT" \)) \ + \ + winpr_internal_assert((cond), (file), (fkt), (line)); \ + \ + WINPR_DO_COVERITY_PRAGMA(coverity compliance end_block "CONSTANT_EXPRESSION_RESULT" \ + "NO_EFFECT") \ + WINPR_PRAGMA_DIAG_POP \ + } while (0) +#define WINPR_ASSERT(cond) WINPR_ASSERT_AT((cond), __FILE__, __func__, __LINE__) + #ifdef __cplusplus extern "C" { diff --git a/winpr/include/winpr/platform.h b/winpr/include/winpr/platform.h index 2b860b505..987aa9f53 100644 --- a/winpr/include/winpr/platform.h +++ b/winpr/include/winpr/platform.h @@ -32,6 +32,12 @@ #define WINPR_DO_PRAGMA(x) __pragma(#x) #endif +#if !defined(__COVERITY__) +#define WINPR_DO_COVERITY_PRAGMA(x) +#else +#define WINPR_DO_COVERITY_PRAGMA(x) WINPR_DO_PRAGMA(x) +#endif + #if defined(__GNUC__) #define WINPR_PRAGMA_WARNING(msg) WINPR_DO_PRAGMA(GCC warning #msg) #elif defined(__clang__) From 541fc3a8c67307dfbb2761a77d8e267275ca6d9a Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 14 Nov 2024 11:14:36 +0100 Subject: [PATCH 2/4] [channels,remdesk] fix const correctness of function --- channels/remdesk/common/remdesk_common.c | 2 +- channels/remdesk/common/remdesk_common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/channels/remdesk/common/remdesk_common.c b/channels/remdesk/common/remdesk_common.c index 854547798..a4600e764 100644 --- a/channels/remdesk/common/remdesk_common.c +++ b/channels/remdesk/common/remdesk_common.c @@ -23,7 +23,7 @@ #include #define TAG CHANNELS_TAG("remdesk.common") -UINT remdesk_write_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* header) +UINT remdesk_write_channel_header(wStream* s, const REMDESK_CHANNEL_HEADER* header) { WCHAR ChannelNameW[32] = { 0 }; diff --git a/channels/remdesk/common/remdesk_common.h b/channels/remdesk/common/remdesk_common.h index 76dddd0f1..98bd26b8a 100644 --- a/channels/remdesk/common/remdesk_common.h +++ b/channels/remdesk/common/remdesk_common.h @@ -25,7 +25,7 @@ #include -FREERDP_LOCAL UINT remdesk_write_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* header); +FREERDP_LOCAL UINT remdesk_write_channel_header(wStream* s, const REMDESK_CHANNEL_HEADER* header); FREERDP_LOCAL UINT remdesk_write_ctl_header(wStream* s, const REMDESK_CTL_HEADER* ctlHeader); FREERDP_LOCAL UINT remdesk_read_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* header); FREERDP_LOCAL UINT remdesk_prepare_ctl_header(REMDESK_CTL_HEADER* ctlHeader, UINT32 msgType, From 8883ccc503cd73be5a655a5e024cc6ce7b539468 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 14 Nov 2024 13:07:58 +0100 Subject: [PATCH 3/4] [cmake] remove GCC -Wunknown-pragmas workaround The offending pragma definitions are now only defined for coverity builds --- cmake/CXXCompilerFlags.cmake | 5 ----- cmake/CompilerFlags.cmake | 5 ----- 2 files changed, 10 deletions(-) diff --git a/cmake/CXXCompilerFlags.cmake b/cmake/CXXCompilerFlags.cmake index b288f8450..635c62cf7 100644 --- a/cmake/CXXCompilerFlags.cmake +++ b/cmake/CXXCompilerFlags.cmake @@ -74,10 +74,5 @@ if (WIN32) add_compile_options($<$:-DNOMINMAX>) endif() -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431 -if (CMAKE_COMPILER_IS_GNUCC AND (CMAKE_CXX_COMPILER_VERSION LESS 13)) - CheckCFlag(-Wno-unknown-pragmas) -endif() - set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} CACHE STRING "default CXXFLAGS") message("Using CXXFLAGS ${CMAKE_CXX_FLAGS}") diff --git a/cmake/CompilerFlags.cmake b/cmake/CompilerFlags.cmake index f48beb50b..b5ce375ff 100644 --- a/cmake/CompilerFlags.cmake +++ b/cmake/CompilerFlags.cmake @@ -57,11 +57,6 @@ if (ENABLE_WARNING_ERROR) CheckCFlag(-Werror) endif() -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431 -if (CMAKE_COMPILER_IS_GNUCC AND (CMAKE_C_COMPILER_VERSION LESS 13)) - CheckCFlag(-Wno-unknown-pragmas) -endif() - CheckCFlag(-fno-omit-frame-pointer) CheckCFlag(-fmacro-prefix-map="${CMAKE_SOURCE_DIR}"="./") From 9b3f5cd86e61e336bdc9e73696db760bff0786da Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 14 Nov 2024 13:33:14 +0100 Subject: [PATCH 4/4] [ci,coverity] define COVERITY_BUILD By defining this symbol the build will enable coverity related pragma definitions --- .github/workflows/coverity.yml | 1 + CMakeLists.txt | 5 +++++ winpr/CMakeLists.txt | 5 +++++ winpr/include/winpr/platform.h | 3 ++- 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml index 629a78a94..845b10a58 100644 --- a/.github/workflows/coverity.yml +++ b/.github/workflows/coverity.yml @@ -79,6 +79,7 @@ jobs: -GNinja \ -C ci/cmake-preloads/config-coverity.txt \ -DALLOW_IN_SOURCE_BUILD=true \ + -DCOVERITY_BUILD=ON \ -B. \ -S. cov-build --dir cov-int cmake --build . diff --git a/CMakeLists.txt b/CMakeLists.txt index 8eae52bcf..0dd54d379 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,6 +56,11 @@ if (NOT WIN32 AND NOT ANDROID) option(WITH_X11 "build X11 client/server" ${OPT_DEFAULT_VAL}) endif() +# Enable coverity related pragma definitions +if (COVERITY_BUILD) + add_compile_definitions(COVERITY_BUILD) +endif() + # Include our extra modules list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/) diff --git a/winpr/CMakeLists.txt b/winpr/CMakeLists.txt index 6bdff7cf2..2f3006cd9 100644 --- a/winpr/CMakeLists.txt +++ b/winpr/CMakeLists.txt @@ -33,6 +33,11 @@ if (NOT FREERDP_UNIFIED_BUILD) include(CommonConfigOptions) include(ConfigOptions) + # Enable coverity related pragma definitions + if (COVERITY_BUILD) + add_compile_definitions(COVERITY_BUILD) + endif() + # Default to build shared libs option(EXPORT_ALL_SYMBOLS "Export all symbols form library" OFF) diff --git a/winpr/include/winpr/platform.h b/winpr/include/winpr/platform.h index 987aa9f53..a3c95366e 100644 --- a/winpr/include/winpr/platform.h +++ b/winpr/include/winpr/platform.h @@ -32,7 +32,8 @@ #define WINPR_DO_PRAGMA(x) __pragma(#x) #endif -#if !defined(__COVERITY__) +/* COVERITY_BUILD must be defined by build system */ +#if !defined(COVERITY_BUILD) #define WINPR_DO_COVERITY_PRAGMA(x) #else #define WINPR_DO_COVERITY_PRAGMA(x) WINPR_DO_PRAGMA(x)