Skip to content

Commit d81431f

Browse files
committed
Issue python#23524: Replace _PyVerify_fd function with calling _set_thread_local_invalid_parameter_handler on every thread.
1 parent eef20de commit d81431f

9 files changed

Lines changed: 160 additions & 112 deletions

File tree

Include/fileobject.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,6 @@ PyAPI_DATA(int) Py_HasFileSystemDefaultEncoding;
3232
#ifndef Py_LIMITED_API
3333
PyAPI_FUNC(PyObject *) PyFile_NewStdPrinter(int);
3434
PyAPI_DATA(PyTypeObject) PyStdPrinter_Type;
35-
36-
#if defined _MSC_VER && _MSC_VER >= 1400
37-
/* A routine to check if a file descriptor is valid on Windows. Returns 0
38-
* and sets errno to EBADF if it isn't. This is to avoid Assertions
39-
* from various functions in the Windows CRT beginning with
40-
* Visual Studio 2005
41-
*/
42-
int _PyVerify_fd(int fd);
43-
#else
44-
#define _PyVerify_fd(A) (1) /* dummy */
45-
#endif
4635
#endif /* Py_LIMITED_API */
4736

4837
/* A routine to check if a file descriptor can be select()-ed. */

Include/fileutils.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,18 @@ PyAPI_FUNC(int) _Py_get_blocking(int fd);
108108
PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking);
109109
#endif /* !MS_WINDOWS */
110110

111+
#if defined _MSC_VER && _MSC_VER >= 1400
112+
/* A routine to check if a file descriptor is valid on Windows. Returns 0
113+
* and sets errno to EBADF if it isn't. This is to avoid Assertions
114+
* from various functions in the Windows CRT beginning with
115+
* Visual Studio 2005
116+
*/
117+
int _PyVerify_fd(int fd);
118+
119+
#else
120+
#define _PyVerify_fd(A) (1) /* dummy */
121+
#endif
122+
111123
#endif /* Py_LIMITED_API */
112124

113125
#ifdef __cplusplus

Modules/_io/fileio.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ check_fd(int fd)
182182
{
183183
#if defined(HAVE_FSTAT) || defined(MS_WINDOWS)
184184
struct _Py_stat_struct buf;
185-
if (!_PyVerify_fd(fd) || (_Py_fstat(fd, &buf) < 0 && errno == EBADF)) {
185+
if (_Py_fstat(fd, &buf) < 0 && errno == EBADF) {
186186
PyObject *exc;
187187
char *msg = strerror(EBADF);
188188
exc = PyObject_CallFunction(PyExc_OSError, "(is)",

Modules/posixmodule.c

Lines changed: 4 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,99 +1051,16 @@ PyLong_FromPy_off_t(Py_off_t offset)
10511051
}
10521052

10531053

1054-
#if defined _MSC_VER && _MSC_VER >= 1400
1055-
/* Microsoft CRT in VS2005 and higher will verify that a filehandle is
1056-
* valid and raise an assertion if it isn't.
1057-
* Normally, an invalid fd is likely to be a C program error and therefore
1058-
* an assertion can be useful, but it does contradict the POSIX standard
1059-
* which for write(2) states:
1060-
* "Otherwise, -1 shall be returned and errno set to indicate the error."
1061-
* "[EBADF] The fildes argument is not a valid file descriptor open for
1062-
* writing."
1063-
* Furthermore, python allows the user to enter any old integer
1064-
* as a fd and should merely raise a python exception on error.
1065-
* The Microsoft CRT doesn't provide an official way to check for the
1066-
* validity of a file descriptor, but we can emulate its internal behaviour
1067-
* by using the exported __pinfo data member and knowledge of the
1068-
* internal structures involved.
1069-
* The structures below must be updated for each version of visual studio
1070-
* according to the file internal.h in the CRT source, until MS comes
1071-
* up with a less hacky way to do this.
1072-
* (all of this is to avoid globally modifying the CRT behaviour using
1073-
* _set_invalid_parameter_handler() and _CrtSetReportMode())
1054+
#if defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900
1055+
/* Legacy implementation of _PyVerify_fd_dup2 while transitioning to
1056+
* MSVC 14.0. This should eventually be removed. (issue23524)
10741057
*/
1075-
/* The actual size of the structure is determined at runtime.
1076-
* Only the first items must be present.
1077-
*/
1078-
1079-
#if _MSC_VER >= 1900
1080-
1081-
typedef struct {
1082-
CRITICAL_SECTION lock;
1083-
intptr_t osfhnd;
1084-
__int64 startpos;
1085-
char osfile;
1086-
} my_ioinfo;
1087-
1088-
#define IOINFO_L2E 6
1089-
#define IOINFO_ARRAYS 128
1090-
1091-
#else
1092-
1093-
typedef struct {
1094-
intptr_t osfhnd;
1095-
char osfile;
1096-
} my_ioinfo;
1097-
10981058
#define IOINFO_L2E 5
10991059
#define IOINFO_ARRAYS 64
1100-
1101-
#endif
1102-
1103-
extern __declspec(dllimport) char * __pioinfo[];
11041060
#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
11051061
#define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS)
1106-
#define FOPEN 0x01
11071062
#define _NO_CONSOLE_FILENO (intptr_t)-2
11081063

1109-
/* This function emulates what the windows CRT does to validate file handles */
1110-
int
1111-
_PyVerify_fd(int fd)
1112-
{
1113-
const int i1 = fd >> IOINFO_L2E;
1114-
const int i2 = fd & ((1 << IOINFO_L2E) - 1);
1115-
1116-
static size_t sizeof_ioinfo = 0;
1117-
1118-
/* Determine the actual size of the ioinfo structure,
1119-
* as used by the CRT loaded in memory
1120-
*/
1121-
if (sizeof_ioinfo == 0 && __pioinfo[0] != NULL) {
1122-
sizeof_ioinfo = _msize(__pioinfo[0]) / IOINFO_ARRAY_ELTS;
1123-
}
1124-
if (sizeof_ioinfo == 0) {
1125-
/* This should not happen... */
1126-
goto fail;
1127-
}
1128-
1129-
/* See that it isn't a special CLEAR fileno */
1130-
if (fd != _NO_CONSOLE_FILENO) {
1131-
/* Microsoft CRT would check that 0<=fd<_nhandle but we can't do that. Instead
1132-
* we check pointer validity and other info
1133-
*/
1134-
if (0 <= i1 && i1 < IOINFO_ARRAYS && __pioinfo[i1] != NULL) {
1135-
/* finally, check that the file is open */
1136-
my_ioinfo* info = (my_ioinfo*)(__pioinfo[i1] + i2 * sizeof_ioinfo);
1137-
if (info->osfile & FOPEN) {
1138-
return 1;
1139-
}
1140-
}
1141-
}
1142-
fail:
1143-
errno = EBADF;
1144-
return 0;
1145-
}
1146-
11471064
/* the special case of checking dup2. The target fd must be in a sensible range */
11481065
static int
11491066
_PyVerify_fd_dup2(int fd1, int fd2)
@@ -1158,8 +1075,7 @@ _PyVerify_fd_dup2(int fd1, int fd2)
11581075
return 0;
11591076
}
11601077
#else
1161-
/* dummy version. _PyVerify_fd() is already defined in fileobject.h */
1162-
#define _PyVerify_fd_dup2(A, B) (1)
1078+
#define _PyVerify_fd_dup2(fd1, fd2) (_PyVerify_fd(fd1) && (fd2) >= 0)
11631079
#endif
11641080

11651081
#ifdef MS_WINDOWS

PC/invalid_parameter_handler.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#ifdef _MSC_VER
2+
3+
#include <stdlib.h>
4+
5+
#if _MSC_VER >= 1900
6+
/* pyconfig.h uses this function in the _Py_BEGIN/END_SUPPRESS_IPH
7+
* macros. It does not need to be defined when building using MSVC
8+
* earlier than 14.0 (_MSC_VER == 1900).
9+
*/
10+
11+
static void __cdecl _silent_invalid_parameter_handler(
12+
wchar_t const* expression,
13+
wchar_t const* function,
14+
wchar_t const* file,
15+
unsigned int line,
16+
uintptr_t pReserved) { }
17+
18+
void *_Py_silent_invalid_parameter_handler =
19+
(void*)_silent_invalid_parameter_handler;
20+
#endif
21+
22+
#endif

PCbuild/pythoncore.vcxproj

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@
333333
<ClCompile Include="..\Parser\parser.c" />
334334
<ClCompile Include="..\Parser\parsetok.c" />
335335
<ClCompile Include="..\Parser\tokenizer.c" />
336+
<ClCompile Include="..\PC\invalid_parameter_handler.c" />
336337
<ClCompile Include="..\PC\winreg.c" />
337338
<ClCompile Include="..\PC\config.c" />
338339
<ClCompile Include="..\PC\getpathp.c" />
@@ -394,25 +395,21 @@
394395
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
395396
<ImportGroup Label="ExtensionTargets">
396397
</ImportGroup>
397-
398398
<Target Name="_GetBuildInfo" BeforeTargets="PrepareForBuild">
399-
<Exec Command='hg id -b &gt; "$(IntDir)hgbranch.txt"' ContinueOnError="true" />
400-
<Exec Command='hg id -i &gt; "$(IntDir)hgversion.txt"' ContinueOnError="true" />
401-
<Exec Command='hg id -t &gt; "$(IntDir)hgtag.txt"' ContinueOnError="true" />
402-
399+
<Exec Command="hg id -b &gt; &quot;$(IntDir)hgbranch.txt&quot;" ContinueOnError="true" />
400+
<Exec Command="hg id -i &gt; &quot;$(IntDir)hgversion.txt&quot;" ContinueOnError="true" />
401+
<Exec Command="hg id -t &gt; &quot;$(IntDir)hgtag.txt&quot;" ContinueOnError="true" />
403402
<PropertyGroup>
404403
<HgBranch Condition="Exists('$(IntDir)hgbranch.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgbranch.txt').Trim())</HgBranch>
405404
<HgVersion Condition="Exists('$(IntDir)hgversion.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgversion.txt').Trim())</HgVersion>
406405
<HgTag Condition="Exists('$(IntDir)hgtag.txt')">$([System.IO.File]::ReadAllText('$(IntDir)hgtag.txt').Trim())</HgTag>
407406
</PropertyGroup>
408-
409407
<ItemGroup>
410408
<ClCompile Include="..\Modules\getbuildinfo.c">
411409
<PreprocessorDefinitions>HGVERSION="$(HgVersion)";HGTAG="$(HgTag)";HGBRANCH="$(HgBranch)";%(PreprocessorDefinitions)</PreprocessorDefinitions>
412410
</ClCompile>
413411
</ItemGroup>
414412
</Target>
415-
416413
<Target Name="_WarnAboutToolset" BeforeTargets="PrepareForBuild" Condition="$(PlatformToolset) != 'v140'">
417414
<Warning Text="Toolset $(PlatformToolset) is not used for official builds. Your build may have errors or incompatibilities." />
418415
</Target>

PCbuild/pythoncore.vcxproj.filters

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,9 @@
959959
<ClCompile Include="..\Modules\hashtable.c">
960960
<Filter>Modules</Filter>
961961
</ClCompile>
962+
<ClCompile Include="..\PC\invalid_parameter_handler.c">
963+
<Filter>PC</Filter>
964+
</ClCompile>
962965
</ItemGroup>
963966
<ItemGroup>
964967
<ResourceCompile Include="..\PC\python_nt.rc">

Python/fileutils.c

Lines changed: 103 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <locale.h>
44

55
#ifdef MS_WINDOWS
6+
# include <malloc.h>
67
# include <windows.h>
78
#endif
89

@@ -636,14 +637,10 @@ _Py_fstat(int fd, struct _Py_stat_struct *result)
636637
else
637638
h = (HANDLE)_get_osfhandle(fd);
638639

639-
/* Protocol violation: we explicitly clear errno, instead of
640-
setting it to a POSIX error. Callers should use GetLastError. */
641640
errno = 0;
642641

643642
if (h == INVALID_HANDLE_VALUE) {
644-
/* This is really a C library error (invalid file handle).
645-
We set the Win32 error to the closes one matching. */
646-
SetLastError(ERROR_INVALID_HANDLE);
643+
errno = EBADF;
647644
return -1;
648645
}
649646
memset(result, 0, sizeof(*result));
@@ -652,6 +649,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *result)
652649
if (type == FILE_TYPE_UNKNOWN) {
653650
DWORD error = GetLastError();
654651
if (error != 0) {
652+
errno = EINVAL;
655653
return -1;
656654
}
657655
/* else: valid but unknown file */
@@ -666,6 +664,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *result)
666664
}
667665

668666
if (!GetFileInformationByHandle(h, &info)) {
667+
errno = EINVAL;
669668
return -1;
670669
}
671670

@@ -1267,3 +1266,102 @@ _Py_set_blocking(int fd, int blocking)
12671266
}
12681267
#endif
12691268

1269+
#ifdef _MSC_VER
1270+
#if _MSC_VER >= 1900
1271+
1272+
/* This function lets the Windows CRT validate the file handle without
1273+
terminating the process if it's invalid. */
1274+
int
1275+
_PyVerify_fd(int fd)
1276+
{
1277+
intptr_t osh;
1278+
/* Fast check for the only condition we know */
1279+
if (fd < 0) {
1280+
_set_errno(EBADF);
1281+
return 0;
1282+
}
1283+
osh = _get_osfhandle(fd);
1284+
return osh != (intptr_t)-1;
1285+
}
1286+
1287+
#elif _MSC_VER >= 1400
1288+
/* Legacy implementation of _PyVerify_fd while transitioning to
1289+
* MSVC 14.0. This should eventually be removed. (issue23524)
1290+
*/
1291+
1292+
/* Microsoft CRT in VS2005 and higher will verify that a filehandle is
1293+
* valid and raise an assertion if it isn't.
1294+
* Normally, an invalid fd is likely to be a C program error and therefore
1295+
* an assertion can be useful, but it does contradict the POSIX standard
1296+
* which for write(2) states:
1297+
* "Otherwise, -1 shall be returned and errno set to indicate the error."
1298+
* "[EBADF] The fildes argument is not a valid file descriptor open for
1299+
* writing."
1300+
* Furthermore, python allows the user to enter any old integer
1301+
* as a fd and should merely raise a python exception on error.
1302+
* The Microsoft CRT doesn't provide an official way to check for the
1303+
* validity of a file descriptor, but we can emulate its internal behaviour
1304+
* by using the exported __pinfo data member and knowledge of the
1305+
* internal structures involved.
1306+
* The structures below must be updated for each version of visual studio
1307+
* according to the file internal.h in the CRT source, until MS comes
1308+
* up with a less hacky way to do this.
1309+
* (all of this is to avoid globally modifying the CRT behaviour using
1310+
* _set_invalid_parameter_handler() and _CrtSetReportMode())
1311+
*/
1312+
/* The actual size of the structure is determined at runtime.
1313+
* Only the first items must be present.
1314+
*/
1315+
typedef struct {
1316+
intptr_t osfhnd;
1317+
char osfile;
1318+
} my_ioinfo;
1319+
1320+
extern __declspec(dllimport) char * __pioinfo[];
1321+
#define IOINFO_L2E 5
1322+
#define IOINFO_ARRAYS 64
1323+
#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
1324+
#define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS)
1325+
#define FOPEN 0x01
1326+
#define _NO_CONSOLE_FILENO (intptr_t)-2
1327+
1328+
/* This function emulates what the windows CRT does to validate file handles */
1329+
int
1330+
_PyVerify_fd(int fd)
1331+
{
1332+
const int i1 = fd >> IOINFO_L2E;
1333+
const int i2 = fd & ((1 << IOINFO_L2E) - 1);
1334+
1335+
static size_t sizeof_ioinfo = 0;
1336+
1337+
/* Determine the actual size of the ioinfo structure,
1338+
* as used by the CRT loaded in memory
1339+
*/
1340+
if (sizeof_ioinfo == 0 && __pioinfo[0] != NULL) {
1341+
sizeof_ioinfo = _msize(__pioinfo[0]) / IOINFO_ARRAY_ELTS;
1342+
}
1343+
if (sizeof_ioinfo == 0) {
1344+
/* This should not happen... */
1345+
goto fail;
1346+
}
1347+
1348+
/* See that it isn't a special CLEAR fileno */
1349+
if (fd != _NO_CONSOLE_FILENO) {
1350+
/* Microsoft CRT would check that 0<=fd<_nhandle but we can't do that. Instead
1351+
* we check pointer validity and other info
1352+
*/
1353+
if (0 <= i1 && i1 < IOINFO_ARRAYS && __pioinfo[i1] != NULL) {
1354+
/* finally, check that the file is open */
1355+
my_ioinfo* info = (my_ioinfo*)(__pioinfo[i1] + i2 * sizeof_ioinfo);
1356+
if (info->osfile & FOPEN) {
1357+
return 1;
1358+
}
1359+
}
1360+
}
1361+
fail:
1362+
errno = EBADF;
1363+
return 0;
1364+
}
1365+
1366+
#endif /* _MSC_VER >= 1900 || _MSC_VER >= 1400 */
1367+
#endif /* defined _MSC_VER */

Python/pystate.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ to avoid the expense of doing their own locking).
2222
#endif
2323
#endif
2424

25+
#if defined _MSC_VER && _MSC_VER >= 1900
26+
/* Issue #23524: Temporary fix to disable termination due to invalid parameters */
27+
PyAPI_DATA(void*) _Py_silent_invalid_parameter_handler;
28+
#include <stdlib.h>
29+
#endif
30+
2531
#ifdef __cplusplus
2632
extern "C" {
2733
#endif
@@ -222,6 +228,11 @@ new_threadstate(PyInterpreterState *interp, int init)
222228
tstate->next->prev = tstate;
223229
interp->tstate_head = tstate;
224230
HEAD_UNLOCK();
231+
232+
#if defined _MSC_VER && _MSC_VER >= 1900
233+
/* Issue #23524: Temporary fix to disable termination due to invalid parameters */
234+
_set_thread_local_invalid_parameter_handler((_invalid_parameter_handler)_Py_silent_invalid_parameter_handler);
235+
#endif
225236
}
226237

227238
return tstate;

0 commit comments

Comments
 (0)