Skip to content

Commit 1bc78de

Browse files
pablogsalStanFromIrelandvstinner
authored
[3.15] Add MSan to CI (GH-158625) (#158832)
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 * Apply Victor's suggestions --------- (cherry picked from commit b93fb19) Co-authored-by: Stan Ulbrych <stan@python.org> Co-authored-by: Victor Stinner <victor.stinner@gmail.com>
1 parent c27f494 commit 1bc78de

13 files changed

Lines changed: 58 additions & 11 deletions

File tree

‎.github/workflows/build.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,9 @@ jobs:
605605
- check-name: Undefined behavior
606606
sanitizer: UBSan
607607
free-threading: false
608+
- check-name: Memory
609+
sanitizer: MSan
610+
free-threading: false
608611
uses: ./.github/workflows/reusable-san.yml
609612
with:
610613
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
@@ -1015,6 +1015,10 @@ Debug options
10151015

10161016
Enable MemorySanitizer allocation error detector, ``msan`` (default is no).
10171017

1018+
MSan reports false positives for memory initialized by libraries that are
1019+
not built with MSan, so either build all dependencies with MSan or disable
1020+
the extension modules that use them in :file:`Modules/Setup.local`.
1021+
10181022
.. versionadded:: 3.6
10191023

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

‎Include/pyport.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,7 @@ extern "C" {
554554
# define _Py_MEMORY_SANITIZER
555555
# define _Py_NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory))
556556
# define _Py_MSAN_UNPOISON(PTR, SIZE) (__msan_unpoison(PTR, SIZE))
557+
# define _Py_MSAN_UNPOISON_STRING(STR) (__msan_unpoison_string(STR))
557558
# endif
558559
# endif
559560
# if __has_feature(address_sanitizer)
@@ -595,6 +596,9 @@ extern "C" {
595596
#ifndef _Py_MSAN_UNPOISON
596597
# define _Py_MSAN_UNPOISON(PTR, SIZE)
597598
#endif
599+
#ifndef _Py_MSAN_UNPOISON_STRING
600+
# define _Py_MSAN_UNPOISON_STRING(STR)
601+
#endif
598602

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

‎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
@@ -438,7 +438,7 @@ next_frame_pointer_is_valid(uintptr_t *frame_pointer, uintptr_t *next_fp,
438438
#endif
439439
}
440440

441-
static PyObject *
441+
static PyObject * _Py_NO_SANITIZE_MEMORY
442442
manual_unwind_from_fp(uintptr_t *frame_pointer)
443443
{
444444
uintptr_t stack_min = 0;
@@ -2049,8 +2049,8 @@ check_pyobject_forbidden_bytes_is_freed(PyObject *self,
20492049
static PyObject *
20502050
check_pyobject_freed_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
20512051
{
2052-
/* ASan or TSan would report an use-after-free error */
2053-
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER)
2052+
/* ASan, MSan or TSan would report an error. */
2053+
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER) || defined(_Py_MEMORY_SANITIZER)
20542054
Py_RETURN_NONE;
20552055
#else
20562056
PyObject *op = PyObject_CallNoArgs((PyObject *)&PyBaseObject_Type);

‎Modules/posixmodule.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10137,6 +10137,7 @@ os_getlogin_impl(PyObject *module)
1013710137
errno = old_errno;
1013810138
}
1013910139
else {
10140+
_Py_MSAN_UNPOISON(name, sizeof(name));
1014010141
result = PyUnicode_DecodeFSDefault(name);
1014110142
}
1014210143
#else

‎Modules/socketmodule.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,9 @@ set_herror(socket_state *state, int h_error)
754754
PyObject *v;
755755

756756
#ifdef HAVE_HSTRERROR
757-
v = Py_BuildValue("(iN)", h_error, decode_error_message(hstrerror(h_error)));
757+
const char *errmsg = hstrerror(h_error);
758+
_Py_MSAN_UNPOISON_STRING(errmsg);
759+
v = Py_BuildValue("(iN)", h_error, decode_error_message(errmsg));
758760
#else
759761
v = Py_BuildValue("(is)", h_error, "host not found");
760762
#endif
@@ -781,7 +783,9 @@ set_gaierror(socket_state *state, int error)
781783
#endif
782784

783785
#ifdef HAVE_GAI_STRERROR
784-
v = Py_BuildValue("(iN)", error, decode_error_message(gai_strerror(error)));
786+
const char *errmsg = gai_strerror(error);
787+
_Py_MSAN_UNPOISON_STRING(errmsg);
788+
v = Py_BuildValue("(iN)", error, decode_error_message(errmsg));
785789
#else
786790
v = Py_BuildValue("(is)", error, "getaddrinfo failed");
787791
#endif
@@ -6420,6 +6424,7 @@ socket_getservbyport(PyObject *self, PyObject *args)
64206424
PyErr_SetString(PyExc_OSError, "port/proto not found");
64216425
return NULL;
64226426
}
6427+
_Py_MSAN_UNPOISON_STRING(sp->s_name);
64236428
return PyUnicode_FromString(sp->s_name);
64246429
}
64256430

0 commit comments

Comments
 (0)