Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,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 }}
Expand Down
22 changes: 20 additions & 2 deletions .github/workflows/reusable-san.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
|| ''
}}
- name: UBSan option setup
if: inputs.sanitizer != 'TSan'
if: inputs.sanitizer == 'UBSan'
run: >-
echo
"UBSAN_OPTIONS=${SAN_LOG_OPTION}
Expand All @@ -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"
Expand All @@ -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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Objects/obmalloc.c can be modified to not use debug hooks on memory allocations when Python is built with --with-memory-sanitizer. When I did tests on Valgrind, I set PYTHONMALLOC=malloc environment variable to disable debug hooks and disable pymalloc, to use the generic malloc()/free().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do it as a follow-up to this/#157934. I thought about building with debug here and using PYTHONMALLOC=malloc here, but it does not propagate to subprocesses so I worry we would lose some coverage.

- name: Configure CPython
run: >-
./configure
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions Doc/using/configure.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,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
Expand Down
4 changes: 4 additions & 0 deletions Include/pyport.h
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,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)
Expand Down Expand Up @@ -599,6 +600,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__)
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/_test_multiprocessing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3159,6 +3159,7 @@ def test_imap_and_imap_unordered_with_buffersize_type_validation(
with self.assertRaisesRegex(expected_exception, expected_regex):
method(str, range(4), buffersize=buffersize)

@unittest.skipUnless(HAS_SHAREDCTYPES, 'needs sharedctypes')
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
@support.subTests('method_name', ("imap", "imap_unordered"))
def test_imap_and_imap_unordered_when_buffer_is_full(self, method_name):
Expand Down Expand Up @@ -3194,6 +3195,7 @@ def produce_args():
p.terminate()
p.join()

@unittest.skipUnless(HAS_SHAREDCTYPES, 'needs sharedctypes')
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
@support.subTests('method_name', ("imap", "imap_unordered"))
def test_imap_and_imap_unordered_with_buffersize_when_buffer_is_full(
Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_cext/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def test_build(self):
self.check_build('_test_cppext_internal')


@support.requires_venv_with_pip()
def setUpModule():
global VENV_CONTEXT, PYTHON_EXE
VENV_CONTEXT = support.setup_venv_with_pip_setuptools('env')
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_faulthandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
5 changes: 5 additions & 0 deletions Modules/_remote_debugging/binary_io_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#include <zstd.h>
#endif

#ifdef _Py_MEMORY_SANITIZER
# include <sanitizer/msan_interface.h>
#endif

/* ============================================================================
* CONSTANTS FOR BINARY FORMAT SIZES
* ============================================================================ */
Expand Down Expand Up @@ -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;
}

Expand Down
6 changes: 6 additions & 0 deletions Modules/_remote_debugging/binary_io_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#include <zstd.h>
#endif

#ifdef _Py_MEMORY_SANITIZER
# include <sanitizer/msan_interface.h>
#endif

/* ============================================================================
* CONSTANTS FOR BINARY FORMAT SIZES
* ============================================================================ */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,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;
Expand Down Expand Up @@ -2083,8 +2083,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);
Expand Down
1 change: 1 addition & 0 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -10138,6 +10138,7 @@ os_getlogin_impl(PyObject *module)
errno = old_errno;
}
else {
_Py_MSAN_UNPOISON(name, sizeof(name));
result = PyUnicode_DecodeFSDefault(name);
}
#else
Expand Down
9 changes: 7 additions & 2 deletions Modules/socketmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,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
Expand All @@ -801,7 +803,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
Expand Down Expand Up @@ -6522,6 +6526,7 @@ _socket_getservbyport_impl(PyObject *module, int port, const char *proto)
PyErr_SetString(PyExc_OSError, "port/proto not found");
return NULL;
}
_Py_MSAN_UNPOISON_STRING(sp->s_name);
return PyUnicode_FromString(sp->s_name);
}

Expand Down
1 change: 1 addition & 0 deletions Python/instrumentation.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Comment thread
StanFromIreland marked this conversation as resolved.
monitoring->tools = NULL;
monitoring->lines = NULL;
monitoring->line_tools = NULL;
Expand Down
2 changes: 1 addition & 1 deletion configure

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -4458,7 +4458,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;
}
Expand Down
Loading