From 7abf979a438f9dfc3517ab913f9075b950628835 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 28 May 2024 09:37:00 +0100 Subject: [PATCH 1/4] cmake: Disable `ctime_tests` if build with `-fsanitize=memory` Clang >= 16 has `-fsanitize-memory-param-retval` turned on by default, which is incompatible with --- CMakeLists.txt | 20 +++++++++++++++++++- cmake/CheckMemorySanitizer.cmake | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 cmake/CheckMemorySanitizer.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 88994d828a..ef079cd1f9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -263,6 +263,17 @@ endif() set(CMAKE_C_VISIBILITY_PRESET hidden) +set(print_msan_notice) +if(SECP256K1_BUILD_CTIME_TESTS) + include(CheckMemorySanitizer) + check_memory_sanitizer(msan_enabled) + if(msan_enabled) + try_append_c_flags(-fno-sanitize-memory-param-retval) + set(print_msan_notice YES) + endif() + unset(msan_enabled) +endif() + # Ask CTest to create a "check" target (e.g., make check) as alias for the "test" target. # CTEST_TEST_TARGET_ALIAS is not documented but supposed to be user-facing. # See: https://gitlab.kitware.com/cmake/cmake/-/commit/816c9d1aa1f2b42d40c81a991b68c96eb12b6d2 @@ -358,7 +369,14 @@ endif() if(SECP256K1_LATE_CFLAGS) message("SECP256K1_LATE_CFLAGS ................. ${SECP256K1_LATE_CFLAGS}") endif() -message("\n") +message("") +if(print_msan_notice) + message( + "Note:\n" + " MemorySanitizer detected, tried to add -fno-sanitize-memory-param-retval to compile options\n" + " to avoid false positives in ctime_tests. Pass -DSECP256K1_BUILD_CTIME_TESTS=OFF to avoid this.\n" + ) +endif() if(SECP256K1_EXPERIMENTAL) message( " ******\n" diff --git a/cmake/CheckMemorySanitizer.cmake b/cmake/CheckMemorySanitizer.cmake new file mode 100644 index 0000000000..d9ef681e65 --- /dev/null +++ b/cmake/CheckMemorySanitizer.cmake @@ -0,0 +1,18 @@ +include_guard(GLOBAL) +include(CheckCSourceCompiles) + +function(check_memory_sanitizer output) + set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) + check_c_source_compiles(" + #if defined(__has_feature) + # if __has_feature(memory_sanitizer) + /* MemorySanitizer is enabled. */ + # elif + # error \"MemorySanitizer is disabled.\" + # endif + #else + # error \"__has_feature is not defined.\" + #endif + " HAVE_MSAN) + set(${output} ${HAVE_MSAN} PARENT_SCOPE) +endfunction() From abde59f52dd901287fa774225d3dbd22933d90c2 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 26 May 2024 15:53:42 +0100 Subject: [PATCH 2/4] cmake: Report more compiler details in summary --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ef079cd1f9..a02cc8ce75 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -343,7 +343,7 @@ message("Valgrind .............................. ${SECP256K1_VALGRIND}") get_directory_property(definitions COMPILE_DEFINITIONS) string(REPLACE ";" " " definitions "${definitions}") message("Preprocessor defined macros ........... ${definitions}") -message("C compiler ............................ ${CMAKE_C_COMPILER}") +message("C compiler ............................ ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION}, ${CMAKE_C_COMPILER}") message("CFLAGS ................................ ${CMAKE_C_FLAGS}") get_directory_property(compile_options COMPILE_OPTIONS) string(REPLACE ";" " " compile_options "${compile_options}") From 396e885886b2665850c3b10e4cd029cffffce1b7 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 28 May 2024 09:44:47 +0100 Subject: [PATCH 3/4] autotools: Align MSan checking code with CMake's implementation --- build-aux/m4/bitcoin_secp.m4 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index fee2d7b4db..048267fa6e 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/build-aux/m4/bitcoin_secp.m4 @@ -50,10 +50,14 @@ AC_MSG_CHECKING(whether MemorySanitizer is enabled) AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ #if defined(__has_feature) # if __has_feature(memory_sanitizer) - # error "MemorySanitizer is enabled." + /* MemorySanitizer is enabled. */ + # elif + # error "MemorySanitizer is disabled." # endif + #else + # error "__has_feature is not defined." #endif - ]])], [msan_enabled=no], [msan_enabled=yes]) + ]])], [msan_enabled=yes], [msan_enabled=no]) AC_MSG_RESULT([$msan_enabled]) ]) From f55703ba49454fc46226f4846fe292d4a3dfa3ef Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 28 May 2024 09:47:00 +0100 Subject: [PATCH 4/4] autotools: Delete unneeded compiler test This change makes both Autotools and CMake build systems consistent. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9640a67399..b6d20cc9e1 100644 --- a/configure.ac +++ b/configure.ac @@ -248,7 +248,7 @@ if test x"$enable_ctime_tests" = x"auto"; then fi print_msan_notice=no -if test x"$enable_ctime_tests" = x"yes" && test x"$GCC" = x"yes"; then +if test x"$enable_ctime_tests" = x"yes"; then SECP_MSAN_CHECK # MSan on Clang >=16 reports unitialized memory in function parameters and return values, even if # the uninitalized variable is never actually "used". This is called "eager" checking, and it's