Same as #1517, but for the CMakle build system.
The second commit improves the configure summary (similar to https://github.com/hebasto/bitcoin/pull/189.
cmake: Disable eager MSan in ctime_tests #1532
pull hebasto wants to merge 4 commits into bitcoin-core:master from hebasto:240526-msan-cmake changing 4 files +45 −5-
hebasto commented at 2:56 PM on May 26, 2024: member
-
real-or-random commented at 12:25 PM on May 27, 2024: contributor
Concept ACK
-
in cmake/CheckMemorySanitizer.cmake:9 in 7e2afc4968 outdated
0 | @@ -0,0 +1,16 @@ 1 | +include_guard(GLOBAL) 2 | +include(CheckCSourceCompiles) 3 | + 4 | +function(check_memory_sanitizer output) 5 | + set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) 6 | + check_c_source_compiles(" 7 | + #if defined(__has_feature) 8 | + # if __has_feature(memory_sanitizer) 9 | + // MemorySanitizer is enabled.
real-or-random commented at 12:27 PM on May 27, 2024:nit: We should probably use the same test program on configure and CMake (and perhaps avoid
//comments which are invalid in C89, though that really is more of a philosophical concern.)
hebasto commented at 12:45 PM on May 27, 2024:nit: We should probably use the same test program on configure and CMake
I'd prefer to align Autotools'
SECP_MSAN_CHECKcode with CMake'scheck_memory_sanitizerto keep the latter straightforward without negated values.... and perhaps avoid
//comments which are invalid in C89Thanks. Fixed.
real-or-random commented at 8:48 PM on May 27, 2024:I'd prefer to align Autotools'
SECP_MSAN_CHECKcode with CMake'scheck_memory_sanitizerto keep the latter straightforward without negated values.Fine with that if you want to add a commit that makes them consistent, but then we'll also need to
#errorif!defined(__has_feature).
hebasto commented at 8:49 AM on May 28, 2024:I'd prefer to align Autotools'
SECP_MSAN_CHECKcode with CMake'scheck_memory_sanitizerto keep the latter straightforward without negated values.Fine with that if you want to add a commit that makes them consistent, but then we'll also need to
#errorif!defined(__has_feature).Thanks! Fixed.
in CMakeLists.txt:267 in 7e2afc4968 outdated
262 | @@ -263,6 +263,17 @@ endif() 263 | 264 | set(CMAKE_C_VISIBILITY_PRESET hidden) 265 | 266 | +set(print_msan_notice) 267 | +if(SECP256K1_BUILD_CTIME_TESTS AND CMAKE_C_COMPILER_ID MATCHES "^(Clang|AppleClang)$")
fanquake commented at 12:29 PM on May 27, 2024:Does Apple Clang makes sense here?
fanquake commented at 12:34 PM on May 27, 2024:Actually, why are you hard coding compiler IDs at all? This just makes things less flexible, and the check should be enough.
real-or-random commented at 12:38 PM on May 27, 2024:Yeah, I had the same thought. It saves a test compilation for GCC, but it's probably simpler to drop the check for clang. In #1517, I also added a similar check (but for "GCC or Clang", so it's much more useless). Let's just drop it.
real-or-random commented at 8:49 PM on May 27, 2024:Thanks, do you want to add a commit that also removes the
$GCCcheck in configure.ac, just to keep the system consistent?
real-or-random added the label assurance on May 27, 2024real-or-random added the label build on May 27, 2024real-or-random added the label side-channel on May 27, 2024hebasto force-pushed on May 27, 2024hebasto commented at 12:43 PM on May 27, 2024: memberAddressed comments:
avoid
//comments which are invalid in C89Actually, why are you hard coding compiler IDs at all? This just makes things less flexible, and the check should be enough.
7abf979a43cmake: Disable `ctime_tests` if build with `-fsanitize=memory`
Clang >= 16 has `-fsanitize-memory-param-retval` turned on by default, which is incompatible with
cmake: Report more compiler details in summary abde59f52dautotools: Align MSan checking code with CMake's implementation 396e885886f55703ba49autotools: Delete unneeded compiler test
This change makes both Autotools and CMake build systems consistent.
hebasto force-pushed on May 28, 2024hebasto commented at 8:49 AM on May 28, 2024: memberAddressed comments:
... to add a commit that makes them consistent, but then we'll also need to
#errorif!defined(__has_feature).... to add a commit that also removes the $GCC check in configure.ac, just to keep the system consistent
real-or-random approvedreal-or-random commented at 12:33 PM on June 5, 2024: contributorACK f55703ba49454fc46226f4846fe292d4a3dfa3ef
real-or-random merged this on Jun 10, 2024real-or-random closed this on Jun 10, 2024hebasto deleted the branch on Jun 10, 2024fanquake referenced this in commit b941c15808 on Jun 25, 2024hebasto referenced this in commit b20389c74a on Jun 25, 2024fanquake referenced this in commit 1408944d2e on Jun 25, 2024achow101 referenced this in commit 9ac4f69ec2 on Jun 26, 2024vmta referenced this in commit bafdd96c0a on Jul 3, 2024janus referenced this in commit 411aef6677 on Jul 26, 2024vmta referenced this in commit 871c80c433 on Sep 6, 2024vmta referenced this in commit 4d1f6d5635 on Oct 29, 2024div72 referenced this in commit af627d47c3 on Apr 12, 2025oskarszoon referenced this in commit 0d5c9260f4 on Jul 1, 2025github-actions[bot] referenced this in commit 758d4e90b4 on Mar 1, 2026github-actions[bot] referenced this in commit 68a2178f22 on Mar 1, 2026github-actions[bot] referenced this in commit a8bc1a0b2b on Mar 1, 20260x000000000019d6689c085ae165831e934ff76 referenced this in commit 3b9450150d on Mar 2, 2026csjones referenced this in commit a4d92824ae on Mar 2, 2026Contributors
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC
This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC