From 42759b94bd1ee414c4c36d81e4d0a9cac22f1b9a Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Wed, 8 May 2024 15:12:57 +0200 Subject: [PATCH] all: more clang-tidy cleanups --- .clang-tidy | 2 ++ CMakeLists.txt | 2 +- misc/cmake/try_compile/types_abi.cpp | 14 ++++++++++++++ misc/make/Makefile-extra.mk | 2 +- src/check/dt_cxxlib.cpp | 10 ++++++++++ src/util/cxxlib.h | 14 +++++++++----- 6 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 misc/cmake/try_compile/types_abi.cpp diff --git a/.clang-tidy b/.clang-tidy index b3a50607..ec0a863e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -23,11 +23,13 @@ Checks: > clang-analyzer-*, -clang-analyzer-optin.performance.Padding, -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling, + -clang-analyzer-security.insecureAPI.rand, -clang-analyzer-security.insecureAPI.strcpy, clang-diagnostic-*, performance-*, -performance-avoid-endl, -performance-enum-size, + -performance-no-int-to-ptr, -performance-unnecessary-value-param, FormatStyle: file HeaderFilterRegex: '.*' diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e3e33e9..9b27613f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -259,7 +259,7 @@ if(NOT UPX_CONFIG_DISABLE_THREADS) endif() # make sure that threads are indeed fully supported in C++ if(Threads_FOUND) - foreach(f std_lock_guard.cpp) + foreach(f std_lock_guard.cpp types_abi.cpp) set(CMAKE_TRY_COMPILE_TARGET_TYPE "EXECUTABLE") if(NOT UPX_CONFIG_DISABLE_CXX_STANDARD) try_compile(result "${CMAKE_CURRENT_BINARY_DIR}" diff --git a/misc/cmake/try_compile/types_abi.cpp b/misc/cmake/try_compile/types_abi.cpp new file mode 100644 index 00000000..51393c5f --- /dev/null +++ b/misc/cmake/try_compile/types_abi.cpp @@ -0,0 +1,14 @@ +// Copyright (C) Markus Franz Xaver Johannes Oberhumer + +#include + +// check ABI - require proper alignment of fundamental types for use in atomics +static_assert(alignof(char) == sizeof(char), ""); +static_assert(alignof(short) == sizeof(short), ""); +static_assert(alignof(int) == sizeof(int), ""); +static_assert(alignof(long) == sizeof(long), ""); +static_assert(alignof(ptrdiff_t) == sizeof(ptrdiff_t), ""); +static_assert(alignof(size_t) == sizeof(size_t), ""); +static_assert(alignof(void *) == sizeof(void *), ""); + +int main() { return 0; } diff --git a/misc/make/Makefile-extra.mk b/misc/make/Makefile-extra.mk index b09aee33..41b5c131 100644 --- a/misc/make/Makefile-extra.mk +++ b/misc/make/Makefile-extra.mk @@ -225,7 +225,7 @@ build/analyze/clang-tidy-bzip2/debug build/analyze/clang-tidy-bzip2/release: $$( build/analyze/clang-tidy-ucl/debug build/analyze/clang-tidy-ucl/release: $$(CLANG_TIDY_BUILD_BASE)/$$(notdir $$@) PHONY $(RUN_CLANG_TIDY_WERROR) -config-file ./misc/analyze/clang-tidy/clang-tidy-ucl.yml /vendor/ucl/ build/analyze/clang-tidy-zlib/debug build/analyze/clang-tidy-zlib/release: $$(CLANG_TIDY_BUILD_BASE)/$$(notdir $$@) PHONY - $(RUN_CLANG_TIDY_WERROR) -config-file ./misc/analyze/clang-tidy/clang-tidy-zlib.yml /vendor/zlib/ + $(RUN_CLANG_TIDY) -config-file ./misc/analyze/clang-tidy/clang-tidy-zlib.yml /vendor/zlib/ build/analyze/clang-tidy-zstd/debug build/analyze/clang-tidy-zstd/release: $$(CLANG_TIDY_BUILD_BASE)/$$(notdir $$@) PHONY $(RUN_CLANG_TIDY) -config-file ./misc/analyze/clang-tidy/clang-tidy-zstd.yml /vendor/zstd/ build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-upx/$$(notdir $$@) diff --git a/src/check/dt_cxxlib.cpp b/src/check/dt_cxxlib.cpp index 6e86e671..d5f0533e 100644 --- a/src/check/dt_cxxlib.cpp +++ b/src/check/dt_cxxlib.cpp @@ -257,35 +257,44 @@ TEST_CASE("upx::ptr_std_atomic_cast") { (void) upx::ptr_std_atomic_cast((upx_int32_t *) nullptr); (void) upx::ptr_std_atomic_cast((int *) nullptr); (void) upx::ptr_std_atomic_cast((long *) nullptr); + (void) upx::ptr_std_atomic_cast((ptrdiff_t *) nullptr); (void) upx::ptr_std_atomic_cast((size_t *) nullptr); (void) upx::ptr_std_atomic_cast((void **) nullptr); } #endif TEST_CASE("upx::atomic_exchange") { +#if defined(__riscv) +// RISC-V has no support for subword atomic operations and needs libatomic to emulate +#else { upx_int8_t x = -1; upx_int8_t y = upx::atomic_exchange(&x, (upx_int8_t) 2); CHECK_EQ(x, 2); CHECK_EQ(y, -1); + UNUSED(y); } { upx_int16_t x = -1; upx_int16_t y = upx::atomic_exchange(&x, (upx_int16_t) 2); CHECK_EQ(x, 2); CHECK_EQ(y, -1); + UNUSED(y); } { upx_int32_t x = -1; upx_int32_t y = upx::atomic_exchange(&x, (upx_int32_t) 2); CHECK_EQ(x, 2); CHECK_EQ(y, -1); + UNUSED(y); } +#endif // __riscv { size_t x = (size_t) 0 - 1; size_t y = upx::atomic_exchange(&x, (size_t) 2); CHECK_EQ(x, 2); CHECK_EQ(y, (size_t) 0 - 1); + UNUSED(y); } { const int buf[2] = {101, 202}; @@ -297,6 +306,7 @@ TEST_CASE("upx::atomic_exchange") { p = upx::atomic_exchange(&ptr_array[1], p); CHECK_EQ(p, buf + 1); assert_noexcept(*ptr_array[0] == 202 && *ptr_array[1] == 101); + UNUSED(p); } } diff --git a/src/util/cxxlib.h b/src/util/cxxlib.h index 739d08f5..0cfdbc5d 100644 --- a/src/util/cxxlib.h +++ b/src/util/cxxlib.h @@ -206,16 +206,20 @@ template forceinline T atomic_exchange(T *ptr, T new_value) noexcept { static_assert(std::is_standard_layout_v); static_assert(std::is_trivially_copyable_v); +#if !(WITH_THREADS) + T old_value = *ptr; + *ptr = new_value; + return old_value; +#else + static_assert(sizeof(T) <= sizeof(void *)); // UPX convention: restrict to fundamental types + static_assert(alignof(T) == sizeof(T)); // UPX convention: require proper alignment #if __has_builtin(__atomic_exchange_n) && defined(__ATOMIC_SEQ_CST) return __atomic_exchange_n(ptr, new_value, __ATOMIC_SEQ_CST); #elif __has_builtin(__sync_swap) return __sync_swap(ptr, new_value); -#elif WITH_THREADS - return std::atomic_exchange(ptr_std_atomic_cast(ptr), new_value); #else - T old_value = *ptr; - *ptr = new_value; - return old_value; + return std::atomic_exchange(ptr_std_atomic_cast(ptr), new_value); +#endif #endif }