Skip to content

Commit b93fb19

Browse files
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 <victor.stinner@gmail.com> * Apply Victor's suggestions Co-authored-by: Victor Stinner <victor.stinner@gmail.com> --------- Co-authored-by: Victor Stinner <victor.stinner@gmail.com>
1 parent f839c06 commit b93fb19

15 files changed

Lines changed: 61 additions & 11 deletions

File tree

‎.github/workflows/build.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,9 @@ jobs:
547547
- check-name: Undefined behavior
548548
sanitizer: UBSan
549549
free-threading: false
550+
- check-name: Memory
551+
sanitizer: MSan
552+
free-threading: false
550553
uses: ./.github/workflows/reusable-san.yml
551554
with:
552555
sanitizer: ${{ matrix.sanitizer }}

‎.github/workflows/reusable-san.yml‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
|| ''
6161
}}
6262
- name: UBSan option setup
63-
if: inputs.sanitizer != 'TSan'
63+
if: inputs.sanitizer == 'UBSan'
6464
run: >-
6565
echo
6666
"UBSAN_OPTIONS=${SAN_LOG_OPTION}
@@ -69,6 +69,20 @@ jobs:
6969
>> "$GITHUB_ENV"
7070
env:
7171
SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log
72+
- name: MSan option setup
73+
if: inputs.sanitizer == 'MSan'
74+
run: |
75+
echo "MSAN_OPTIONS=${SAN_LOG_OPTION} allocator_may_return_null=1 handle_segv=0" >> "$GITHUB_ENV"
76+
# MSan reports false positives for memory initialized by libraries
77+
# that are not built with MSan, so disable modules that use them.
78+
# _remote_debugging links to libzstd directly, but we unpoision the memory.
79+
{
80+
echo '*disabled*'
81+
echo '_bz2 _ctypes _curses _curses_panel _dbm _decimal _gdbm _hashlib'
82+
echo '_lzma _sqlite3 _ssl _tkinter _uuid _zstd readline zlib'
83+
} > Modules/Setup.local
84+
env:
85+
SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log
7286
- name: Add ccache to PATH
7387
run: |
7488
echo "PATH=/usr/lib/ccache:$PATH" >> "$GITHUB_ENV"
@@ -93,6 +107,8 @@ jobs:
93107
# gh-157958: -O2 instead of the pydebug default -Og to avoid a clang 21
94108
# compile-time blowup on some interpreter files.
95109
# (https://github.com/llvm/llvm-project/issues/179695)
110+
# MSan uses --with-assertions instead of --with-pydebug because its
111+
# hooks on the Python memory allocators hide uninitialized reads.
96112
- name: Configure CPython
97113
run: >-
98114
./configure
@@ -101,9 +117,11 @@ jobs:
101117
${{
102118
inputs.sanitizer == 'TSan'
103119
&& '--with-thread-sanitizer'
120+
|| inputs.sanitizer == 'MSan'
121+
&& '--with-memory-sanitizer'
104122
|| '--with-undefined-behavior-sanitizer --with-strict-overflow'
105123
}}
106-
--with-pydebug
124+
${{ inputs.sanitizer == 'MSan' && '--with-assertions' || '--with-pydebug' }}
107125
${{ inputs.sanitizer == 'TSan' && '--with-openssl="$OPENSSL_DIR" --with-openssl-rpath=auto' || '' }}
108126
${{ inputs.free-threading && '--disable-gil' || '' }}
109127
- name: Build CPython

‎Doc/using/configure.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,10 @@ Debug options
10261026

10271027
Enable MemorySanitizer allocation error detector, ``msan`` (default is no).
10281028

1029+
MSan reports false positives for memory initialized by libraries that are
1030+
not built with MSan, so either build all dependencies with MSan or disable
1031+
the extension modules that use them in :file:`Modules/Setup.local`.
1032+
10291033
.. versionadded:: 3.6
10301034

10311035
.. option:: --with-undefined-behavior-sanitizer

‎Include/pyport.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ extern "C" {
558558
# define _Py_MEMORY_SANITIZER
559559
# define _Py_NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory))
560560
# define _Py_MSAN_UNPOISON(PTR, SIZE) (__msan_unpoison(PTR, SIZE))
561+
# define _Py_MSAN_UNPOISON_STRING(STR) (__msan_unpoison_string(STR))
561562
# endif
562563
# endif
563564
# if __has_feature(address_sanitizer)
@@ -599,6 +600,9 @@ extern "C" {
599600
#ifndef _Py_MSAN_UNPOISON
600601
# define _Py_MSAN_UNPOISON(PTR, SIZE)
601602
#endif
603+
#ifndef _Py_MSAN_UNPOISON_STRING
604+
# define _Py_MSAN_UNPOISON_STRING(STR)
605+
#endif
602606

603607
/* AIX has __bool__ redefined in it's system header file. */
604608
#if defined(_AIX) && defined(__bool__)

‎Lib/test/_test_multiprocessing.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3159,6 +3159,7 @@ def test_imap_and_imap_unordered_with_buffersize_type_validation(
31593159
with self.assertRaisesRegex(expected_exception, expected_regex):
31603160
method(str, range(4), buffersize=buffersize)
31613161

3162+
@unittest.skipUnless(HAS_SHAREDCTYPES, 'needs sharedctypes')
31623163
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
31633164
@support.subTests('method_name', ("imap", "imap_unordered"))
31643165
def test_imap_and_imap_unordered_when_buffer_is_full(self, method_name):
@@ -3194,6 +3195,7 @@ def produce_args():
31943195
p.terminate()
31953196
p.join()
31963197

3198+
@unittest.skipUnless(HAS_SHAREDCTYPES, 'needs sharedctypes')
31973199
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
31983200
@support.subTests('method_name', ("imap", "imap_unordered"))
31993201
def test_imap_and_imap_unordered_with_buffersize_when_buffer_is_full(

‎Lib/test/test_cext/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ def test_build(self):
192192
self.check_build('_test_cppext_internal')
193193

194194

195+
@support.requires_venv_with_pip()
195196
def setUpModule():
196197
global VENV_CONTEXT, PYTHON_EXE
197198
VENV_CONTEXT = support.setup_venv_with_pip_setuptools('env')

‎Lib/test/test_faulthandler.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@
3434

3535

3636
def skip_if_sanitizer_signal(signame):
37-
return support.skip_if_sanitizer(f"TSAN/UBSan itercepts {signame}",
38-
thread=True, ub=True)
37+
return support.skip_if_sanitizer(f"TSan/UBSan/MSan intercepts {signame}",
38+
thread=True, ub=True, memory=True)
3939

4040

4141
def expected_traceback(lineno1, lineno2, header, min_count=1):

‎Modules/_remote_debugging/binary_io_reader.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
#include <zstd.h>
2020
#endif
2121

22+
#ifdef _Py_MEMORY_SANITIZER
23+
# include <sanitizer/msan_interface.h>
24+
#endif
25+
2226
/* ============================================================================
2327
* CONSTANTS FOR BINARY FORMAT SIZES
2428
* ============================================================================ */
@@ -315,6 +319,7 @@ reader_decompress_samples(BinaryReader *reader, const uint8_t *data)
315319
return -1;
316320
}
317321

322+
_Py_MSAN_UNPOISON(output.dst, output.pos);
318323
total_output += output.pos;
319324
}
320325

‎Modules/_remote_debugging/binary_io_writer.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
#include <zstd.h>
2020
#endif
2121

22+
#ifdef _Py_MEMORY_SANITIZER
23+
# include <sanitizer/msan_interface.h>
24+
#endif
25+
2226
/* ============================================================================
2327
* CONSTANTS FOR BINARY FORMAT SIZES
2428
* ============================================================================ */
@@ -235,6 +239,7 @@ writer_flush_buffer(BinaryWriter *writer)
235239
return -1;
236240
}
237241

242+
_Py_MSAN_UNPOISON(writer->zstd.compressed_buffer, output.pos);
238243
if (output.pos > 0) {
239244
if (fwrite_checked_allow_threads(writer->zstd.compressed_buffer, output.pos, writer->fp) < 0) {
240245
return -1;
@@ -1104,6 +1109,7 @@ binary_writer_finalize(BinaryWriter *writer)
11041109
return -1;
11051110
}
11061111

1112+
_Py_MSAN_UNPOISON(writer->zstd.compressed_buffer, output.pos);
11071113
if (output.pos > 0) {
11081114
if (fwrite_checked_allow_threads(writer->zstd.compressed_buffer, output.pos, writer->fp) < 0) {
11091115
return -1;

‎Modules/_testinternalcapi.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ next_frame_pointer_is_valid(uintptr_t *frame_pointer, uintptr_t *next_fp,
469469
#endif
470470
}
471471

472-
static PyObject *
472+
static PyObject * _Py_NO_SANITIZE_MEMORY
473473
manual_unwind_from_fp(uintptr_t *frame_pointer)
474474
{
475475
uintptr_t stack_min = 0;
@@ -2110,8 +2110,8 @@ check_pyobject_forbidden_bytes_is_freed(PyObject *self,
21102110
static PyObject *
21112111
check_pyobject_freed_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
21122112
{
2113-
/* ASan or TSan would report an use-after-free error */
2114-
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER)
2113+
/* ASan, MSan or TSan would report an error. */
2114+
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER) || defined(_Py_MEMORY_SANITIZER)
21152115
Py_RETURN_NONE;
21162116
#else
21172117
PyObject *op = PyObject_CallNoArgs((PyObject *)&PyBaseObject_Type);

0 commit comments

Comments
 (0)