Skip to content

Add MSan and integrate it with CI#13916

Merged
minchopaskal merged 9 commits intoredis:unstablefrom
minchopaskal:add-msan
May 9, 2025
Merged

Add MSan and integrate it with CI#13916
minchopaskal merged 9 commits intoredis:unstablefrom
minchopaskal:add-msan

Conversation

@minchopaskal
Copy link
Collaborator

@minchopaskal minchopaskal commented Apr 2, 2025

Description

Memory sanitizer (MSAN) is used to detect use-of-uninitialized memory issues. While Address Sanitizer catches a wide range of memory safety issues, it doesn't specifically detect uninitialized memory usage. Therefore, Memory Sanitizer complements Address Sanitizer. This PR adds MSAN run to the daily build, with the possibility of incorporating it into the ci.yml workflow in the future if needed.

Changes in source files fix false-positive issues and they should not introduce any runtime implications.

Note: Valgrind performs similar checks to both ASAN and MSAN but sanitizers run significantly faster.

Limitations

  • Memory sanitizer is only supported by Clang.
  • MSAN documentation states that all dependencies, including the standard library, must be compiled with MSAN. However, it also mentions there are interceptors for common libc functions, so compiling the standard library with the MSAN flag is not strictly necessary. Therefore, we are not compiling libc with MSAN.

@tezc tezc self-requested a review April 2, 2025 12:24
@tezc
Copy link
Collaborator

tezc commented Apr 2, 2025

Note: Msan is complementary to Asan. Asan has bunch of memory safety checks but it does not check use of uninitialized data. M san is doing that check.

minchopaskal and others added 3 commits April 3, 2025 10:09
fix no_sanitize macro definitions

add error msg for msan when not compiled with clang
@sundb sundb added this to Redis 8.2 May 7, 2025
@github-project-automation github-project-automation bot moved this to Todo in Redis 8.2 May 7, 2025
@sundb
Copy link
Collaborator

sundb commented May 7, 2025

@minchopaskal minchopaskal merged commit fdbf880 into redis:unstable May 9, 2025
18 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Redis 8.2 May 9, 2025
minchopaskal added a commit that referenced this pull request May 13, 2025
PR #13916 introduced a regression -
by overriding the `CFLAGS` and `LDFLAGS` variables for all of the
dependencies hiredis and fast_float lost some of their compiler/linker
flags.

This PR makes it so we can pass additional CFLAGS/LDFLAGS to hiredis,
without overriding them as it contains a bit more complex Makefile. As
for fast_float - passing CFLAGS/LDFLAGS from outside now doesn't break
the expected behavior.

The build step in the CI was changed so that the MacOS is now build with
TLS to catch such errors in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants