From 6e8ca500c689d084e3b04071e68764e2a6017fc3 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Mon, 5 Oct 2026 02:11:42 +0100 Subject: [PATCH] Run a job that the test suite with MSan to the CI (#158625) * Run the test suite with MSan in CI * Additional fixes * Add `_Py_MSAN_UNPOISON_STRING` * Apply Victor's suggestions Co-authored-by: Victor Stinner * Apply Victor's suggestions Co-authored-by: Victor Stinner --------- Co-authored-by: Victor Stinner (cherry picked from commit b93fb19a6e3118857d9a1dc4ffcc179b632a88c1) --- .github/workflows/build.yml | 3 +++ .github/workflows/reusable-san.yml | 22 ++++++++++++++++++-- Doc/using/configure.rst | 4 ++++ Include/pyport.h | 4 ++++ Lib/test/test_faulthandler.py | 4 ++-- Modules/_remote_debugging/binary_io_reader.c | 5 +++++ Modules/_remote_debugging/binary_io_writer.c | 6 ++++++ Modules/_testinternalcapi.c | 6 +++--- Modules/posixmodule.c | 1 + Modules/socketmodule.c | 9 ++++++-- Python/instrumentation.c | 1 + configure | 2 +- configure.ac | 2 +- 13 files changed, 58 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 90c5d0fbec686a..2f8f97b34d25b5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -605,6 +605,9 @@ jobs: - check-name: Undefined behavior sanitizer: UBSan free-threading: false + - check-name: Memory + sanitizer: MSan + free-threading: false uses: ./.github/workflows/reusable-san.yml with: sanitizer: ${{ matrix.sanitizer }} diff --git a/.github/workflows/reusable-san.yml b/.github/workflows/reusable-san.yml index ad3232743874d6..da6306a50cf7bc 100644 --- a/.github/workflows/reusable-san.yml +++ b/.github/workflows/reusable-san.yml @@ -60,7 +60,7 @@ jobs: || '' }} - name: UBSan option setup - if: inputs.sanitizer != 'TSan' + if: inputs.sanitizer == 'UBSan' run: >- echo "UBSAN_OPTIONS=${SAN_LOG_OPTION} @@ -69,6 +69,20 @@ jobs: >> "$GITHUB_ENV" env: SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log + - name: MSan option setup + if: inputs.sanitizer == 'MSan' + run: | + echo "MSAN_OPTIONS=${SAN_LOG_OPTION} allocator_may_return_null=1 handle_segv=0" >> "$GITHUB_ENV" + # MSan reports false positives for memory initialized by libraries + # that are not built with MSan, so disable modules that use them. + # _remote_debugging links to libzstd directly, but we unpoision the memory. + { + echo '*disabled*' + echo '_bz2 _ctypes _curses _curses_panel _dbm _decimal _gdbm _hashlib' + echo '_lzma _sqlite3 _ssl _tkinter _uuid _zstd readline zlib' + } > Modules/Setup.local + env: + SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log - name: Add ccache to PATH run: | echo "PATH=/usr/lib/ccache:$PATH" >> "$GITHUB_ENV" @@ -93,6 +107,8 @@ jobs: # gh-157958: -O2 instead of the pydebug default -Og to avoid a clang 21 # compile-time blowup on some interpreter files. # (https://github.com/llvm/llvm-project/issues/179695) + # MSan uses --with-assertions instead of --with-pydebug because its + # hooks on the Python memory allocators hide uninitialized reads. - name: Configure CPython run: >- ./configure @@ -101,9 +117,11 @@ jobs: ${{ inputs.sanitizer == 'TSan' && '--with-thread-sanitizer' + || inputs.sanitizer == 'MSan' + && '--with-memory-sanitizer' || '--with-undefined-behavior-sanitizer --with-strict-overflow' }} - --with-pydebug + ${{ inputs.sanitizer == 'MSan' && '--with-assertions' || '--with-pydebug' }} ${{ inputs.sanitizer == 'TSan' && '--with-openssl="$OPENSSL_DIR" --with-openssl-rpath=auto' || '' }} ${{ inputs.free-threading && '--disable-gil' || '' }} - name: Build CPython diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index 3745c21a567761..d29eb891523243 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -1015,6 +1015,10 @@ Debug options Enable MemorySanitizer allocation error detector, ``msan`` (default is no). + MSan reports false positives for memory initialized by libraries that are + not built with MSan, so either build all dependencies with MSan or disable + the extension modules that use them in :file:`Modules/Setup.local`. + .. versionadded:: 3.6 .. option:: --with-undefined-behavior-sanitizer diff --git a/Include/pyport.h b/Include/pyport.h index 73a3e6cdaf0920..92e65b4d6d1e2a 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -554,6 +554,7 @@ extern "C" { # define _Py_MEMORY_SANITIZER # define _Py_NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory)) # define _Py_MSAN_UNPOISON(PTR, SIZE) (__msan_unpoison(PTR, SIZE)) +# define _Py_MSAN_UNPOISON_STRING(STR) (__msan_unpoison_string(STR)) # endif # endif # if __has_feature(address_sanitizer) @@ -595,6 +596,9 @@ extern "C" { #ifndef _Py_MSAN_UNPOISON # define _Py_MSAN_UNPOISON(PTR, SIZE) #endif +#ifndef _Py_MSAN_UNPOISON_STRING +# define _Py_MSAN_UNPOISON_STRING(STR) +#endif /* AIX has __bool__ redefined in it's system header file. */ #if defined(_AIX) && defined(__bool__) diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py index 5a493a4fd95680..82b347c8f8c045 100644 --- a/Lib/test/test_faulthandler.py +++ b/Lib/test/test_faulthandler.py @@ -34,8 +34,8 @@ def skip_if_sanitizer_signal(signame): - return support.skip_if_sanitizer(f"TSAN/UBSan itercepts {signame}", - thread=True, ub=True) + return support.skip_if_sanitizer(f"TSan/UBSan/MSan intercepts {signame}", + thread=True, ub=True, memory=True) def expected_traceback(lineno1, lineno2, header, min_count=1): diff --git a/Modules/_remote_debugging/binary_io_reader.c b/Modules/_remote_debugging/binary_io_reader.c index 9625ee6f301f05..8af1d281cee6b6 100644 --- a/Modules/_remote_debugging/binary_io_reader.c +++ b/Modules/_remote_debugging/binary_io_reader.c @@ -19,6 +19,10 @@ #include #endif +#ifdef _Py_MEMORY_SANITIZER +# include +#endif + /* ============================================================================ * CONSTANTS FOR BINARY FORMAT SIZES * ============================================================================ */ @@ -315,6 +319,7 @@ reader_decompress_samples(BinaryReader *reader, const uint8_t *data) return -1; } + _Py_MSAN_UNPOISON(output.dst, output.pos); total_output += output.pos; } diff --git a/Modules/_remote_debugging/binary_io_writer.c b/Modules/_remote_debugging/binary_io_writer.c index 6af81515e7131d..9ea0caa3b82b2b 100644 --- a/Modules/_remote_debugging/binary_io_writer.c +++ b/Modules/_remote_debugging/binary_io_writer.c @@ -19,6 +19,10 @@ #include #endif +#ifdef _Py_MEMORY_SANITIZER +# include +#endif + /* ============================================================================ * CONSTANTS FOR BINARY FORMAT SIZES * ============================================================================ */ @@ -235,6 +239,7 @@ writer_flush_buffer(BinaryWriter *writer) return -1; } + _Py_MSAN_UNPOISON(writer->zstd.compressed_buffer, output.pos); if (output.pos > 0) { if (fwrite_checked_allow_threads(writer->zstd.compressed_buffer, output.pos, writer->fp) < 0) { return -1; @@ -1084,6 +1089,7 @@ binary_writer_finalize(BinaryWriter *writer) return -1; } + _Py_MSAN_UNPOISON(writer->zstd.compressed_buffer, output.pos); if (output.pos > 0) { if (fwrite_checked_allow_threads(writer->zstd.compressed_buffer, output.pos, writer->fp) < 0) { return -1; diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 124e06e5302f45..02c3d4433b79e9 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -438,7 +438,7 @@ next_frame_pointer_is_valid(uintptr_t *frame_pointer, uintptr_t *next_fp, #endif } -static PyObject * +static PyObject * _Py_NO_SANITIZE_MEMORY manual_unwind_from_fp(uintptr_t *frame_pointer) { uintptr_t stack_min = 0; @@ -2049,8 +2049,8 @@ check_pyobject_forbidden_bytes_is_freed(PyObject *self, static PyObject * check_pyobject_freed_is_freed(PyObject *self, PyObject *Py_UNUSED(args)) { - /* ASan or TSan would report an use-after-free error */ -#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER) + /* ASan, MSan or TSan would report an error. */ +#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER) || defined(_Py_MEMORY_SANITIZER) Py_RETURN_NONE; #else PyObject *op = PyObject_CallNoArgs((PyObject *)&PyBaseObject_Type); diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0b42b059541d35..b30ec5789643c8 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -10137,6 +10137,7 @@ os_getlogin_impl(PyObject *module) errno = old_errno; } else { + _Py_MSAN_UNPOISON(name, sizeof(name)); result = PyUnicode_DecodeFSDefault(name); } #else diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index fc870aaa5c1c2e..53d380eb4626c5 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -754,7 +754,9 @@ set_herror(socket_state *state, int h_error) PyObject *v; #ifdef HAVE_HSTRERROR - v = Py_BuildValue("(iN)", h_error, decode_error_message(hstrerror(h_error))); + const char *errmsg = hstrerror(h_error); + _Py_MSAN_UNPOISON_STRING(errmsg); + v = Py_BuildValue("(iN)", h_error, decode_error_message(errmsg)); #else v = Py_BuildValue("(is)", h_error, "host not found"); #endif @@ -781,7 +783,9 @@ set_gaierror(socket_state *state, int error) #endif #ifdef HAVE_GAI_STRERROR - v = Py_BuildValue("(iN)", error, decode_error_message(gai_strerror(error))); + const char *errmsg = gai_strerror(error); + _Py_MSAN_UNPOISON_STRING(errmsg); + v = Py_BuildValue("(iN)", error, decode_error_message(errmsg)); #else v = Py_BuildValue("(is)", error, "getaddrinfo failed"); #endif @@ -6420,6 +6424,7 @@ socket_getservbyport(PyObject *self, PyObject *args) PyErr_SetString(PyExc_OSError, "port/proto not found"); return NULL; } + _Py_MSAN_UNPOISON_STRING(sp->s_name); return PyUnicode_FromString(sp->s_name); } diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 646fc15c6872e5..88e23b59db1ce6 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1690,6 +1690,7 @@ allocate_instrumentation_data(PyCodeObject *code) } monitoring->local_monitors = (_Py_LocalMonitors){ 0 }; monitoring->active_monitors = (_Py_LocalMonitors){ 0 }; + memset(monitoring->tool_versions, 0, sizeof(monitoring->tool_versions)); monitoring->tools = NULL; monitoring->lines = NULL; monitoring->line_tools = NULL; diff --git a/configure b/configure index a68a98c5a4de4a..3e08f8af8650b5 100755 --- a/configure +++ b/configure @@ -16392,7 +16392,7 @@ int main(void) { return 2; } - ffi_arg rc; + ffi_arg rc = 0; ffi_call(&cif, FFI_FN(z_is_expected), &rc, values); return !rc; } diff --git a/configure.ac b/configure.ac index ed8b53dd3a4d28..f5365b0ae194d0 100644 --- a/configure.ac +++ b/configure.ac @@ -4360,7 +4360,7 @@ int main(void) { return 2; } - ffi_arg rc; + ffi_arg rc = 0; ffi_call(&cif, FFI_FN(z_is_expected), &rc, values); return !rc; }