Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-127076: Ignore memory mmap in FileIO testing #127088

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cmaloney
Copy link
Contributor

@cmaloney cmaloney commented Nov 21, 2024

mmap, munmap, and mprotect are used by CPython for memory management which may occur in the middle of the FileIO tests. The system calls can also be used with files, so strace includes them in its %file and %desc filters.

Filter out the mmap system calls related to memory allocation for the file tests. Currently FileIO doesn't do mmap at all, so didn't add code to track from mmap through munmap since it wouldn't be used. For now if an mmap on a fd happens, the call will be included (which may cause tests to fail), and at that time support for tracking the address through munmap could be added.

cc: @mgorny this should fix the glibc and musl Gentoo general failures outside sandbox
cc: @brandtbucher This re-enables for PYTHON_JIT, and filters out the mmap calls used for memory management

`mmap`, `munmap`, and `mprotect` are used by CPython for memory
management, which may occur in the middle of the FileIO tests. The
system calls can also be used with files, so `strace` includes them
in its `%file` and `%desc` filters.

Filter out the `mmap` system calls related to memory allocation for the
file tests. Currently FileIO doesn't do `mmap` at all, so didn't add
code to track from `mmap` through `munmap` since it wouldn't be used.
For now if an `mmap` on a fd happens, the call will be included (which
may cause test to fail), and at that time support for tracking the
address throug `munmap` could be added.
@cmaloney cmaloney changed the title gh-127086: Ignore memory mmap in FileIO testing gh-127076: Ignore memory mmap in FileIO testing Nov 21, 2024
def _filter(call):
# mmap can operate on a fd or `NULL` which gives a block of memory.
# Ignore the `NULL` ones.
if call.syscall == 'mmap' and call.args[0] == 'NULL':
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if call.syscall == 'mmap' and call.args[0] == 'NULL':
if call.syscall == 'mmap' and call.args[4] == '-1':

I think the key here is that they pass -1 as fd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately for glibc man pages at least -1 isn't required... Going to see if I can look for MAP_ANONYMOUS easily. (Fortunately this code is linux only, MacOS did MAP_ANON....)


       MAP_ANONYMOUS
              The mapping is not backed by any file; its contents are initialized to zero.  The fd argument is ignored; however, some implementations require fd to be -1 if MAP_ANONYMOUS (or MAP_ANON) is specified, and portable applications should ensure this.  The  off‐
              set argument should be zero.  Support for MAP_ANONYMOUS in conjunction with MAP_SHARED was added in Linux 2.4.

https://man7.org/linux/man-pages/man2/mmap.2.html

@cmaloney
Copy link
Contributor Author

Validated by modifying test_fileio test_syscalls_read to include a big string which results in an allocation:

        # "open, read, close" file using different common patterns.
        check_readall(
            "open builtin with default options",
            f"""
            f = open('{TESTFN}')
            a = "123" * 100000
            f.read()
            f.close()
            """
        )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants