Skip to content

Run a job that the test suite with MSan to the CI - #158625

Open
StanFromIreland wants to merge 4 commits into
python:mainfrom
StanFromIreland:msan-ci
Open

StanFromIreland wants to merge 4 commits into
python:mainfrom
StanFromIreland:msan-ci

Conversation

@StanFromIreland

@StanFromIreland StanFromIreland commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

We have to disable extension modules that link against system libraries (which are not built with MSan) since memory those libraries initialise is reported as uninitialised. While this does significantly reduce coverage of some modules, it saves a great amount of CI time. I also had to unpoison a few buffers filled by libc that MSan does not intercept.

A little fix is included, allocate_instrumentation_data() now zeroes the tool_versions of a _PyCoMonitoringData, which update_instrumentation_data() previously read uninitialised. This doesn't have an affect in practice, as the garbage data only decides whether to clear some bits that are already zero, so the outcome is the same either way.

Inspired by #158584.

@StanFromIreland StanFromIreland added skip issue skip news infra CI, GitHub Actions, buildbots, Dependabot, etc. labels Oct 2, 2026
@StanFromIreland

Copy link
Copy Markdown
Member Author

Note, test_bytes will be failing till #158584 lands.

Comment thread Modules/socketmodule.c Outdated
Comment thread Modules/socketmodule.c Outdated

@vstinner vstinner left a comment

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.

Interesting change.

Comment thread Include/pyport.h Outdated
Comment thread Modules/socketmodule.c Outdated
Comment thread Python/instrumentation.c
}
monitoring->local_monitors = (_Py_LocalMonitors){ 0 };
monitoring->active_monitors = (_Py_LocalMonitors){ 0 };
memset(monitoring->tool_versions, 0, sizeof(monitoring->tool_versions));

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.

That's a surprising change. Does the current code rely on uninitialized memory? If it's a legit bug, it should be backported.

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.

Please see PR description:

A little fix is included, allocate_instrumentation_data() now zeroes the tool_versions of a _PyCoMonitoringData, which update_instrumentation_data() previously read uninitialised. This doesn't have an affect in practice, as the garbage data only decides whether to clear some bits that are already zero, so the outcome is the same either way.

@vstinner

vstinner commented Oct 3, 2026

Copy link
Copy Markdown
Member

test_faulthandler seems to log SEGV from faulthandler_raise_sigsegv and log FPE from faulthandler__sigfpe_impl().

TSan is run with TSAN_OPTIONS="handle_segv=0 (...)".

At least, skip_if_sanitizer_signal() of test_faulthandler can be updated to add memory=True:

    return support.skip_if_sanitizer(f"TSAN/UBSan itercepts {signame}",
                                     thread=True, ub=True, memory=True)

Co-authored-by: Victor Stinner <victor.stinner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review infra CI, GitHub Actions, buildbots, Dependabot, etc. skip issue skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants