From d81431f587e9eab67db683908548b0ad46847b38 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 6 Mar 2015 14:47:02 -0800 Subject: [PATCH] Issue #23524: Replace _PyVerify_fd function with calling _set_thread_local_invalid_parameter_handler on every thread. --- Include/fileobject.h | 11 --- Include/fileutils.h | 12 ++++ Modules/_io/fileio.c | 2 +- Modules/posixmodule.c | 92 ++---------------------- PC/invalid_parameter_handler.c | 22 ++++++ PCbuild/pythoncore.vcxproj | 11 ++- PCbuild/pythoncore.vcxproj.filters | 3 + Python/fileutils.c | 108 +++++++++++++++++++++++++++-- Python/pystate.c | 11 +++ 9 files changed, 160 insertions(+), 112 deletions(-) create mode 100644 PC/invalid_parameter_handler.c diff --git a/Include/fileobject.h b/Include/fileobject.h index 0939744e6d3..03155d3da78 100644 --- a/Include/fileobject.h +++ b/Include/fileobject.h @@ -32,17 +32,6 @@ PyAPI_DATA(int) Py_HasFileSystemDefaultEncoding; #ifndef Py_LIMITED_API PyAPI_FUNC(PyObject *) PyFile_NewStdPrinter(int); PyAPI_DATA(PyTypeObject) PyStdPrinter_Type; - -#if defined _MSC_VER && _MSC_VER >= 1400 -/* A routine to check if a file descriptor is valid on Windows. Returns 0 - * and sets errno to EBADF if it isn't. This is to avoid Assertions - * from various functions in the Windows CRT beginning with - * Visual Studio 2005 - */ -int _PyVerify_fd(int fd); -#else -#define _PyVerify_fd(A) (1) /* dummy */ -#endif #endif /* Py_LIMITED_API */ /* A routine to check if a file descriptor can be select()-ed. */ diff --git a/Include/fileutils.h b/Include/fileutils.h index bb9048116d7..95632edb117 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -108,6 +108,18 @@ PyAPI_FUNC(int) _Py_get_blocking(int fd); PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking); #endif /* !MS_WINDOWS */ +#if defined _MSC_VER && _MSC_VER >= 1400 +/* A routine to check if a file descriptor is valid on Windows. Returns 0 + * and sets errno to EBADF if it isn't. This is to avoid Assertions + * from various functions in the Windows CRT beginning with + * Visual Studio 2005 + */ +int _PyVerify_fd(int fd); + +#else +#define _PyVerify_fd(A) (1) /* dummy */ +#endif + #endif /* Py_LIMITED_API */ #ifdef __cplusplus diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index c44423180c9..ab9eb8c061a 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -182,7 +182,7 @@ check_fd(int fd) { #if defined(HAVE_FSTAT) || defined(MS_WINDOWS) struct _Py_stat_struct buf; - if (!_PyVerify_fd(fd) || (_Py_fstat(fd, &buf) < 0 && errno == EBADF)) { + if (_Py_fstat(fd, &buf) < 0 && errno == EBADF) { PyObject *exc; char *msg = strerror(EBADF); exc = PyObject_CallFunction(PyExc_OSError, "(is)", diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e381bf1bbba..679fc8f3e89 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1051,99 +1051,16 @@ PyLong_FromPy_off_t(Py_off_t offset) } -#if defined _MSC_VER && _MSC_VER >= 1400 -/* Microsoft CRT in VS2005 and higher will verify that a filehandle is - * valid and raise an assertion if it isn't. - * Normally, an invalid fd is likely to be a C program error and therefore - * an assertion can be useful, but it does contradict the POSIX standard - * which for write(2) states: - * "Otherwise, -1 shall be returned and errno set to indicate the error." - * "[EBADF] The fildes argument is not a valid file descriptor open for - * writing." - * Furthermore, python allows the user to enter any old integer - * as a fd and should merely raise a python exception on error. - * The Microsoft CRT doesn't provide an official way to check for the - * validity of a file descriptor, but we can emulate its internal behaviour - * by using the exported __pinfo data member and knowledge of the - * internal structures involved. - * The structures below must be updated for each version of visual studio - * according to the file internal.h in the CRT source, until MS comes - * up with a less hacky way to do this. - * (all of this is to avoid globally modifying the CRT behaviour using - * _set_invalid_parameter_handler() and _CrtSetReportMode()) +#if defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900 +/* Legacy implementation of _PyVerify_fd_dup2 while transitioning to + * MSVC 14.0. This should eventually be removed. (issue23524) */ -/* The actual size of the structure is determined at runtime. - * Only the first items must be present. - */ - -#if _MSC_VER >= 1900 - -typedef struct { - CRITICAL_SECTION lock; - intptr_t osfhnd; - __int64 startpos; - char osfile; -} my_ioinfo; - -#define IOINFO_L2E 6 -#define IOINFO_ARRAYS 128 - -#else - -typedef struct { - intptr_t osfhnd; - char osfile; -} my_ioinfo; - #define IOINFO_L2E 5 #define IOINFO_ARRAYS 64 - -#endif - -extern __declspec(dllimport) char * __pioinfo[]; #define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E) #define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS) -#define FOPEN 0x01 #define _NO_CONSOLE_FILENO (intptr_t)-2 -/* This function emulates what the windows CRT does to validate file handles */ -int -_PyVerify_fd(int fd) -{ - const int i1 = fd >> IOINFO_L2E; - const int i2 = fd & ((1 << IOINFO_L2E) - 1); - - static size_t sizeof_ioinfo = 0; - - /* Determine the actual size of the ioinfo structure, - * as used by the CRT loaded in memory - */ - if (sizeof_ioinfo == 0 && __pioinfo[0] != NULL) { - sizeof_ioinfo = _msize(__pioinfo[0]) / IOINFO_ARRAY_ELTS; - } - if (sizeof_ioinfo == 0) { - /* This should not happen... */ - goto fail; - } - - /* See that it isn't a special CLEAR fileno */ - if (fd != _NO_CONSOLE_FILENO) { - /* Microsoft CRT would check that 0<=fd<_nhandle but we can't do that. Instead - * we check pointer validity and other info - */ - if (0 <= i1 && i1 < IOINFO_ARRAYS && __pioinfo[i1] != NULL) { - /* finally, check that the file is open */ - my_ioinfo* info = (my_ioinfo*)(__pioinfo[i1] + i2 * sizeof_ioinfo); - if (info->osfile & FOPEN) { - return 1; - } - } - } - fail: - errno = EBADF; - return 0; -} - /* the special case of checking dup2. The target fd must be in a sensible range */ static int _PyVerify_fd_dup2(int fd1, int fd2) @@ -1158,8 +1075,7 @@ _PyVerify_fd_dup2(int fd1, int fd2) return 0; } #else -/* dummy version. _PyVerify_fd() is already defined in fileobject.h */ -#define _PyVerify_fd_dup2(A, B) (1) +#define _PyVerify_fd_dup2(fd1, fd2) (_PyVerify_fd(fd1) && (fd2) >= 0) #endif #ifdef MS_WINDOWS diff --git a/PC/invalid_parameter_handler.c b/PC/invalid_parameter_handler.c new file mode 100644 index 00000000000..3bc0104f8dd --- /dev/null +++ b/PC/invalid_parameter_handler.c @@ -0,0 +1,22 @@ +#ifdef _MSC_VER + +#include + +#if _MSC_VER >= 1900 +/* pyconfig.h uses this function in the _Py_BEGIN/END_SUPPRESS_IPH + * macros. It does not need to be defined when building using MSVC + * earlier than 14.0 (_MSC_VER == 1900). + */ + +static void __cdecl _silent_invalid_parameter_handler( + wchar_t const* expression, + wchar_t const* function, + wchar_t const* file, + unsigned int line, + uintptr_t pReserved) { } + +void *_Py_silent_invalid_parameter_handler = + (void*)_silent_invalid_parameter_handler; +#endif + +#endif diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 479f68df554..a5690f6b06e 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -333,6 +333,7 @@ + @@ -394,25 +395,21 @@ - - - - - + + + $([System.IO.File]::ReadAllText('$(IntDir)hgbranch.txt').Trim()) $([System.IO.File]::ReadAllText('$(IntDir)hgversion.txt').Trim()) $([System.IO.File]::ReadAllText('$(IntDir)hgtag.txt').Trim()) - HGVERSION="$(HgVersion)";HGTAG="$(HgTag)";HGBRANCH="$(HgBranch)";%(PreprocessorDefinitions) - diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 174140a912d..b4154054068 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -959,6 +959,9 @@ Modules + + PC + diff --git a/Python/fileutils.c b/Python/fileutils.c index e7111c14311..c0dbc86a932 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -3,6 +3,7 @@ #include #ifdef MS_WINDOWS +# include # include #endif @@ -636,14 +637,10 @@ _Py_fstat(int fd, struct _Py_stat_struct *result) else h = (HANDLE)_get_osfhandle(fd); - /* Protocol violation: we explicitly clear errno, instead of - setting it to a POSIX error. Callers should use GetLastError. */ errno = 0; if (h == INVALID_HANDLE_VALUE) { - /* This is really a C library error (invalid file handle). - We set the Win32 error to the closes one matching. */ - SetLastError(ERROR_INVALID_HANDLE); + errno = EBADF; return -1; } memset(result, 0, sizeof(*result)); @@ -652,6 +649,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *result) if (type == FILE_TYPE_UNKNOWN) { DWORD error = GetLastError(); if (error != 0) { + errno = EINVAL; return -1; } /* else: valid but unknown file */ @@ -666,6 +664,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *result) } if (!GetFileInformationByHandle(h, &info)) { + errno = EINVAL; return -1; } @@ -1267,3 +1266,102 @@ error: } #endif +#ifdef _MSC_VER +#if _MSC_VER >= 1900 + +/* This function lets the Windows CRT validate the file handle without + terminating the process if it's invalid. */ +int +_PyVerify_fd(int fd) +{ + intptr_t osh; + /* Fast check for the only condition we know */ + if (fd < 0) { + _set_errno(EBADF); + return 0; + } + osh = _get_osfhandle(fd); + return osh != (intptr_t)-1; +} + +#elif _MSC_VER >= 1400 +/* Legacy implementation of _PyVerify_fd while transitioning to + * MSVC 14.0. This should eventually be removed. (issue23524) + */ + +/* Microsoft CRT in VS2005 and higher will verify that a filehandle is + * valid and raise an assertion if it isn't. + * Normally, an invalid fd is likely to be a C program error and therefore + * an assertion can be useful, but it does contradict the POSIX standard + * which for write(2) states: + * "Otherwise, -1 shall be returned and errno set to indicate the error." + * "[EBADF] The fildes argument is not a valid file descriptor open for + * writing." + * Furthermore, python allows the user to enter any old integer + * as a fd and should merely raise a python exception on error. + * The Microsoft CRT doesn't provide an official way to check for the + * validity of a file descriptor, but we can emulate its internal behaviour + * by using the exported __pinfo data member and knowledge of the + * internal structures involved. + * The structures below must be updated for each version of visual studio + * according to the file internal.h in the CRT source, until MS comes + * up with a less hacky way to do this. + * (all of this is to avoid globally modifying the CRT behaviour using + * _set_invalid_parameter_handler() and _CrtSetReportMode()) + */ +/* The actual size of the structure is determined at runtime. + * Only the first items must be present. + */ +typedef struct { + intptr_t osfhnd; + char osfile; +} my_ioinfo; + +extern __declspec(dllimport) char * __pioinfo[]; +#define IOINFO_L2E 5 +#define IOINFO_ARRAYS 64 +#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E) +#define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS) +#define FOPEN 0x01 +#define _NO_CONSOLE_FILENO (intptr_t)-2 + +/* This function emulates what the windows CRT does to validate file handles */ +int +_PyVerify_fd(int fd) +{ + const int i1 = fd >> IOINFO_L2E; + const int i2 = fd & ((1 << IOINFO_L2E) - 1); + + static size_t sizeof_ioinfo = 0; + + /* Determine the actual size of the ioinfo structure, + * as used by the CRT loaded in memory + */ + if (sizeof_ioinfo == 0 && __pioinfo[0] != NULL) { + sizeof_ioinfo = _msize(__pioinfo[0]) / IOINFO_ARRAY_ELTS; + } + if (sizeof_ioinfo == 0) { + /* This should not happen... */ + goto fail; + } + + /* See that it isn't a special CLEAR fileno */ + if (fd != _NO_CONSOLE_FILENO) { + /* Microsoft CRT would check that 0<=fd<_nhandle but we can't do that. Instead + * we check pointer validity and other info + */ + if (0 <= i1 && i1 < IOINFO_ARRAYS && __pioinfo[i1] != NULL) { + /* finally, check that the file is open */ + my_ioinfo* info = (my_ioinfo*)(__pioinfo[i1] + i2 * sizeof_ioinfo); + if (info->osfile & FOPEN) { + return 1; + } + } + } + fail: + errno = EBADF; + return 0; +} + +#endif /* _MSC_VER >= 1900 || _MSC_VER >= 1400 */ +#endif /* defined _MSC_VER */ diff --git a/Python/pystate.c b/Python/pystate.c index 32a635c789e..ee1e469635d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -22,6 +22,12 @@ to avoid the expense of doing their own locking). #endif #endif +#if defined _MSC_VER && _MSC_VER >= 1900 +/* Issue #23524: Temporary fix to disable termination due to invalid parameters */ +PyAPI_DATA(void*) _Py_silent_invalid_parameter_handler; +#include +#endif + #ifdef __cplusplus extern "C" { #endif @@ -222,6 +228,11 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->next->prev = tstate; interp->tstate_head = tstate; HEAD_UNLOCK(); + +#if defined _MSC_VER && _MSC_VER >= 1900 + /* Issue #23524: Temporary fix to disable termination due to invalid parameters */ + _set_thread_local_invalid_parameter_handler((_invalid_parameter_handler)_Py_silent_invalid_parameter_handler); +#endif } return tstate;