Issue #23524: Replace _PyVerify_fd function with calling _set_thread_local_invalid_parameter_handler on every thread.

9 files changed, 160 insertions, 112 deletions

diff --git a/Include/fileobject.h b/Include/fileobject.h
index 0939744e6d..03155d3da7 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 bb9048116d..95632edb11 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 c44423180c..ab9eb8c061 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 e381bf1bbb..679fc8f3e8 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 0000000000..3bc0104f8d
--- /dev/null
+++ b/PC/invalid_parameter_handler.c
@@ -0,0 +1,22 @@
+#ifdef _MSC_VER
+
+#include <stdlib.h>
+
+#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 479f68df55..a5690f6b06 100644
--- a/PCbuild/pythoncore.vcxproj
+++ b/PCbuild/pythoncore.vcxproj
@@ -333,6 +333,7 @@
     <ClCompile Include="..\Parser\parser.c" />
     <ClCompile Include="..\Parser\parsetok.c" />
     <ClCompile Include="..\Parser\tokenizer.c" />
+    <ClCompile Include="..\PC\invalid_parameter_handler.c" />
     <ClCompile Include="..\PC\winreg.c" />
     <ClCompile Include="..\PC\config.c" />
     <ClCompile Include="..\PC\getpathp.c" />
@@ -394,25 +395,21 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-  
   <Target Name="_GetBuildInfo" BeforeTargets="PrepareForBuild">
-    <Exec Command='hg id -b &gt; "$(IntDir)hgbranch.txt"' ContinueOnError="true" />
-    <Exec Command='hg id -i &gt; "$(IntDir)hgversion.txt"' ContinueOnError="true" />
-    <Exec Command='hg id -t &gt; "$(IntDir)hgtag.txt"' ContinueOnError="true" />
-    
+    <Exec Command="hg id -b &gt; &quot;$(IntDir)hgbranch.txt&quot;" ContinueOnError="true" />
+    <Exec Command="hg id -i &gt; &quot;$(IntDir)hgversion.txt&quot;" ContinueOnError="true" />
+    <Exec Command="hg id -t &gt; &quot;$(IntDir)hgtag.txt&quot;" ContinueOnError="true" />
     <PropertyGroup>
       <HgBranch Condition="Exists('$(IntDir)hgbranch.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgbranch.txt').Trim())</HgBranch>
       <HgVersion Condition="Exists('$(IntDir)hgversion.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgversion.txt').Trim())</HgVersion>
       <HgTag Condition="Exists('$(IntDir)hgtag.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgtag.txt').Trim())</HgTag>
     </PropertyGroup>
-    
     <ItemGroup>
       <ClCompile Include="..\Modules\getbuildinfo.c">
         <PreprocessorDefinitions>HGVERSION="$(HgVersion)";HGTAG="$(HgTag)";HGBRANCH="$(HgBranch)";%(PreprocessorDefinitions)</PreprocessorDefinitions>
       </ClCompile>
     </ItemGroup>
   </Target>
-
   <Target Name="_WarnAboutToolset" BeforeTargets="PrepareForBuild" Condition="$(PlatformToolset) != 'v140'">
     <Warning Text="Toolset $(PlatformToolset) is not used for official builds. Your build may have errors or incompatibilities." />
   </Target>
diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters
index 174140a912..b415405406 100644
--- a/PCbuild/pythoncore.vcxproj.filters
+++ b/PCbuild/pythoncore.vcxproj.filters
@@ -959,6 +959,9 @@
     <ClCompile Include="..\Modules\hashtable.c">
       <Filter>Modules</Filter>
     </ClCompile>
+    <ClCompile Include="..\PC\invalid_parameter_handler.c">
+      <Filter>PC</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="..\PC\python_nt.rc">
diff --git a/Python/fileutils.c b/Python/fileutils.c
index e7111c1431..c0dbc86a93 100644
--- a/Python/fileutils.c
+++ b/Python/fileutils.c
@@ -3,6 +3,7 @@
 #include <locale.h>
 
 #ifdef MS_WINDOWS
+#  include <malloc.h>
 #  include <windows.h>
 #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 32a635c789..ee1e469635 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 <stdlib.h>
+#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;