Skip to content

gh-118209: Add structured exception handling to mmap module #118213

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.

Sign up for GitHub

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

Merged
zooba merged 35 commits into from
May 10, 2024
Merged

gh-118209: Add structured exception handling to mmap module #118213

Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
4013627
add structured exception handling to mmap_read_method
Dobatymo Apr 24, 2024
6484134
📜🤖 Added by blurb_it.
blurb-it[bot] Apr 24, 2024
71218d2
adjust indentation
Dobatymo Apr 24, 2024
cd4b1fc
restructure to use safe_memcpy to avoid memory leak
Dobatymo Apr 25, 2024
cb1ef19
Apply suggestions from code review
Dobatymo Apr 25, 2024
448fa90
fix passing dest as pointer in mmap_read_byte_method
Dobatymo Apr 26, 2024
ba96163
use PyBytes_AS_STRING instead of raw access
Dobatymo Apr 26, 2024
f97d213
handle mmap_write_method
Dobatymo Apr 26, 2024
7983869
handle mmap_write_byte_method
Dobatymo Apr 26, 2024
c0bd57b
add safe_byte_copy, safe_memmove, safe_memchr, safe_copy_from_slice a…
Dobatymo Apr 30, 2024
92f4bf0
improve macro
Dobatymo May 1, 2024
364d846
Update Modules/mmapmodule.c
Dobatymo May 1, 2024
04dc2d9
handle EXCEPTION_ACCESS_VIOLATION
Dobatymo May 2, 2024
0654f9b
fix formatting
Dobatymo May 2, 2024
d3661ff
use PyBytes_FromStringAndSize instead of extracting the optization
Dobatymo May 2, 2024
30aa64b
update news entry
Dobatymo May 3, 2024
b809ce9
Update Misc/NEWS.d/next/Windows/2024-04-24-05-16-32.gh-issue-118209.R…
Dobatymo May 6, 2024
e907e6d
updated whatsnew entry
Dobatymo May 6, 2024
d1ad0ab
fix typo
Dobatymo May 6, 2024
2ca440d
fix handling some cases in mmap_subscript
Dobatymo May 6, 2024
f7c356c
add test using private function
Dobatymo May 6, 2024
0d70108
Update Lib/test/test_mmap.py
Dobatymo May 6, 2024
6f1f726
run test in subprocess
Dobatymo May 6, 2024
05e8ca0
add expected failure for find method
Dobatymo May 6, 2024
29b12bf
disable faulthandler
Dobatymo May 6, 2024
59e8c4e
typo
Dobatymo May 6, 2024
4f610a5
trailing blanks
Dobatymo May 6, 2024
aa2b41d
Update Lib/test/test_mmap.py
Dobatymo May 6, 2024
11fa04a
add asserts to tests
Dobatymo May 6, 2024
f02c83a
Update Lib/test/test_mmap.py
Dobatymo May 7, 2024
8533af5
support mmap_gfind
Dobatymo May 7, 2024
6aeaa32
aesthetics
Dobatymo May 7, 2024
9737f22
Apply suggestions from code review
Dobatymo May 7, 2024
12fb561
remove restrict for compliance with the python C dialect
Dobatymo May 8, 2024
880a9c0
Update Modules/mmapmodule.c
Dobatymo May 8, 2024
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
Prev Previous commit
Next Next commit
Apply suggestions from code review 
Co-authored-by: Eryk Sun <eryksun@gmail.com>
  • Loading branch information
@Dobatymo @eryksun
Dobatymo and eryksun authored May 7, 2024
commit 9737f222b33f7d253c38acd97b5b5a76adad6331
88 changes: 56 additions & 32 deletions Modules/mmapmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,28 +256,31 @@ do { \
} while (0)
#endif /* UNIX */

#if defined(MS_WIN32) && !defined(DONT_USE_SEH)
#if defined(MS_WINDOWS) && !defined(DONT_USE_SEH)
static DWORD
filter_page_exception(EXCEPTION_POINTERS *ptrs, EXCEPTION_RECORD *record)
{
*record = *ptrs->ExceptionRecord;
if (record->ExceptionCode == EXCEPTION_IN_PAGE_ERROR
|| record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) {
if (record->ExceptionCode == EXCEPTION_IN_PAGE_ERROR ||
record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
{
return EXCEPTION_EXECUTE_HANDLER;
}
return EXCEPTION_CONTINUE_SEARCH;
}

static DWORD
filter_page_exception_method(mmap_object *self, EXCEPTION_POINTERS *ptrs, EXCEPTION_RECORD *record)
filter_page_exception_method(mmap_object *self, EXCEPTION_POINTERS *ptrs,
EXCEPTION_RECORD *record)
{
*record = *ptrs->ExceptionRecord;
if (record->ExceptionCode == EXCEPTION_IN_PAGE_ERROR
|| record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) {
if (record->ExceptionCode == EXCEPTION_IN_PAGE_ERROR ||
record->ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
{

ULONG_PTR address = record->ExceptionInformation[1];
if (address >= (ULONG_PTR) self->data
&& address < (ULONG_PTR) self->data + (ULONG_PTR) self->size)
if (address >= (ULONG_PTR) self->data &&
address < (ULONG_PTR) self->data + (ULONG_PTR) self->size)
{
return EXCEPTION_EXECUTE_HANDLER;
}
Expand All @@ -286,16 +289,16 @@ filter_page_exception_method(mmap_object *self, EXCEPTION_POINTERS *ptrs, EXCEPT
}
#endif

#if defined(MS_WIN32) && !defined(DONT_USE_SEH)
#if defined(MS_WINDOWS) && !defined(DONT_USE_SEH)
#define HANDLE_INVALID_MEM(sourcecode) \
do { \
EXCEPTION_RECORD record; \
__try { \
sourcecode \
} \
__except (filter_page_exception(GetExceptionInformation(), &record)) { \
assert(record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR \
|| record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION); \
assert(record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR || \
record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION); \
if (record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR) { \
NTSTATUS status = (NTSTATUS) record.ExceptionInformation[2]; \
ULONG code = LsaNtStatusToWinError(status); \
Expand All @@ -314,16 +317,16 @@ do { \
} while (0)
#endif

#if defined(MS_WIN32) && !defined(DONT_USE_SEH)
#if defined(MS_WINDOWS) && !defined(DONT_USE_SEH)
#define HANDLE_INVALID_MEM_METHOD(self, sourcecode) \
do { \
EXCEPTION_RECORD record; \
__try { \
sourcecode \
} \
__except (filter_page_exception_method(self, GetExceptionInformation(), &record)) { \
assert(record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR \
|| record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION); \
assert(record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR || \
record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION); \
if (record.ExceptionCode == EXCEPTION_IN_PAGE_ERROR) { \
NTSTATUS status = (NTSTATUS) record.ExceptionInformation[2]; \
ULONG code = LsaNtStatusToWinError(status); \
Expand All @@ -343,39 +346,45 @@ do {
#endif

int
safe_memcpy(void *restrict dest, const void *restrict src, size_t count) {
safe_memcpy(void *restrict dest, const void *restrict src, size_t count)
{
HANDLE_INVALID_MEM(
memcpy(dest, src, count);
);
return 0;
}

int
safe_byte_copy(char *dest, const char *src) {
safe_byte_copy(char *dest, const char *src)
{
HANDLE_INVALID_MEM(
*dest = *src;
);
return 0;
}

int
safe_memchr(char **out, const void *ptr, int ch, size_t count) {
safe_memchr(char **out, const void *ptr, int ch, size_t count)
{
HANDLE_INVALID_MEM(
*out = (char *) memchr(ptr, ch, count);
);
return 0;
}

int
safe_memmove(void *dest, const void *src, size_t count) {
safe_memmove(void *dest, const void *src, size_t count)
{
HANDLE_INVALID_MEM(
memmove(dest, src, count);
);
return 0;
}

int
safe_copy_from_slice(char *dest, const char *src, Py_ssize_t start, Py_ssize_t step, Py_ssize_t slicelen) {
safe_copy_from_slice(char *dest, const char *src, Py_ssize_t start,
Py_ssize_t step, Py_ssize_t slicelen)
{
HANDLE_INVALID_MEM(
size_t cur;
Py_ssize_t i;
Expand All @@ -387,7 +396,9 @@ safe_copy_from_slice(char *dest, const char *src, Py_ssize_t start, Py_ssize_t s
}

int
safe_copy_to_slice(char *dest, const char *src, Py_ssize_t start, Py_ssize_t step, Py_ssize_t slicelen) {
safe_copy_to_slice(char *dest, const char *src, Py_ssize_t start,
Py_ssize_t step, Py_ssize_t slicelen)
{
HANDLE_INVALID_MEM(
size_t cur;
Py_ssize_t i;
Expand All @@ -401,20 +412,24 @@ safe_copy_to_slice(char *dest, const char *src, Py_ssize_t start, Py_ssize_t ste

int
_safe_PyBytes_Find(Py_ssize_t *out, mmap_object *self, const char *haystack,
Py_ssize_t len_haystack, const char *needle, Py_ssize_t len_needle,
Py_ssize_t offset) {
Py_ssize_t len_haystack, const char *needle,
Py_ssize_t len_needle, Py_ssize_t offset)
{
HANDLE_INVALID_MEM_METHOD(self,
*out = _PyBytes_Find(haystack, len_haystack, needle, len_needle, offset);
);
return 0;
}

int
_safe_PyBytes_ReverseFind(Py_ssize_t *out, mmap_object *self, const char *haystack,
Py_ssize_t len_haystack, const char *needle, Py_ssize_t len_needle,
Py_ssize_t offset) {
_safe_PyBytes_ReverseFind(Py_ssize_t *out, mmap_object *self,
const char *haystack, Py_ssize_t len_haystack,
const char *needle, Py_ssize_t len_needle,
Py_ssize_t offset)
{
HANDLE_INVALID_MEM_METHOD(self,
*out = _PyBytes_ReverseFind(haystack, len_haystack, needle, len_needle, offset);
*out = _PyBytes_ReverseFind(haystack, len_haystack, needle, len_needle,
offset);
);
return 0;
}
Expand Down Expand Up @@ -505,7 +520,8 @@ mmap_read_method(mmap_object *self,
if (num_bytes < 0 || num_bytes > remaining)
num_bytes = remaining;

PyObject *result = _safe_PyBytes_FromStringAndSize(self->data + self->pos, num_bytes);
PyObject *result = _safe_PyBytes_FromStringAndSize(self->data + self->pos,
num_bytes);
if (result != NULL) {
self->pos += num_bytes;
}
Expand Down Expand Up @@ -551,7 +567,8 @@ mmap_gfind(mmap_object *self,
assert(0 <= start && start <= end && end <= self->size);
if (_safe_PyBytes_ReverseFind(&index, self,
self->data + start, end - start,
view.buf, view.len, start) < 0) {
view.buf, view.len, start) < 0)
{
result = NULL;
}
else {
Expand All @@ -562,7 +579,8 @@ mmap_gfind(mmap_object *self,
assert(0 <= start && start <= end && end <= self->size);
if (_safe_PyBytes_Find(&index, self,
self->data + start, end - start,
view.buf, view.len, start) < 0) {
view.buf, view.len, start) < 0)
{
result = NULL;
}
else {
Expand Down Expand Up @@ -1087,7 +1105,9 @@ mmap_protect_method(mmap_object *self, PyObject *args) {
return NULL;
}

if (VirtualProtect((void *) (self->data + start), length, flNewProtect, &flOldProtect) == 0) {
if (!VirtualProtect((void *) (self->data + start), length, flNewProtect,
&flOldProtect))
{
PyErr_SetFromWindowsErr(GetLastError());
return NULL;
}
Expand Down Expand Up @@ -1263,7 +1283,9 @@ mmap_subscript(mmap_object *self, PyObject *item)
if (result_buf == NULL)
return PyErr_NoMemory();

if (safe_copy_to_slice(result_buf, self->data, start, step, slicelen) < 0) {
if (safe_copy_to_slice(result_buf, self->data, start, step,
slicelen) < 0)
{
result = NULL;
}
else {
Expand Down Expand Up @@ -1390,7 +1412,9 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
}
}
else {
if (safe_copy_from_slice(self->data, (char *)vbuf.buf, start, step, slicelen) < 0) {
if (safe_copy_from_slice(self->data, (char *)vbuf.buf, start, step,
slicelen) < 0)
{
result = -1;
}
}
Expand Down