gh-133572: Disable error reporting for invalid memory access on unsupported WinAPI partitions #133573
Conversation
Modules/mmapmodule.c
Outdated
| @@ -292,7 +292,7 @@ filter_page_exception_method(mmap_object *self, EXCEPTION_POINTERS *ptrs, | |||
| } | |||
| #endif | |||
|
|
|||
| #if defined(MS_WINDOWS) && !defined(DONT_USE_SEH) | |||
| #if (defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_SYSTEM)) && !defined(DONT_USE_SEH) | |||
There was a problem hiding this comment.
Can we maybe rephrase as something like this at the top of the file? We have similar code over in ctypes/callproc.c, and I think it'll make it more likely that if/when we add more SEH handling, we'll get it right. (Update the comment as appropriate, I just made up something feasible, not necessarily correct)
#if !defined(DONT_USE_SEH) && !(defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_SYSTEM))
// Only the WINDOWS_DESKTOP and WINDOWS_SYSTEM API partitions support SEH handling
// the way we want to use it.
#define DONT_USE_SEH
#endif
There was a problem hiding this comment.
Ah I wasn't aware that SEH is a microsoft only thing in general and so didn't want to generally disable it. In theory I do now always disable it on Non Windows as well. so it would be enough to check #ifndef DONT_USE_SEH.
An alternative would be something like DONT_USE_LSA to make clear we just don't support the lsa handling, but do support structured exceptions.
Oh, it's only the error number conversion that's a problem? Let's just shim that bit out then and keep the SEH. Perhaps add this just before the macro and then call it? static void
_PyErr_SetFromNTSTATUS(NTSTATUS status) {
#if (macro here)
PyErr_SetFromWindowsErr(LsaNtStatusToWinError(status));
#else
if (status & 0x80000000) {
// HRESULT-shaped codes are supported by PyErr_SetFromWindowsErr
PyErr_SetFromWindowsErr((int)status);
} else {
// No mapping for NTSTATUS values, so just return it for diagnostic purposes
// If we provide it as winerror it could incorrectly change the type of the exception.
PyErr_Format(PyExc_OSError, "Operating system error NTSTATUS=0x%08lX", status & 0xFFFFFFFF);
}
#endif
}Or that function might even have a home with the other error setters, but I don't know that we've needed it anywhere else. If you want to put it in (IIRC) errors.c, don't make it static, but also don't use PyAPI_FUNC on it - no need for it to be exported from the DLL. |
|
And sorry about multiple changes - my fault for giving direction without having fully understood the problem! |
aside from the mimalloc fix that has to be downstreamed later on this is the last change required to get cpython to compile on the xbox again