p5pRT
commented
11 years ago From @bulk88
Created by @bulk88
Need an RT number.
Closed p5pRT closed 11 years ago
p5pRT
commented
11 years ago Need an RT number.
p5pRT
commented
11 years ago Patch attached. It needs a smoke-me test since it needs to be tested on VC 2005\, and Windows SDKs (specifically early 64 bit ones that used msvcrt.dll). It also needs Mingw testing. I assume all smoke-mes do DEBUGGING perl runs right? The patch will probably need to be remade before going into blead for some reason or another. This should also be linked to #118127 in RT as "Refers to" or "Parent"\, IDK stylistically.
runtime determination alternatives: - _mize()\, see http://bugs.python.org/file13537/verify_fd.patch - magic handle with _open_osfhandle and brute force search the allocation of ioinfo structs for delta between magic handle values - brute force search for the handle of _get_osfhandle(1) in starting at address _pioinfo[0] ("_pioinfo[0]" is a struct ioinfo *\, not struct ioinfo\, _pioinfo is a struct ioinfo **\, there are 2 arrays\, first is an array of ioinfo *s the 2nd is an array of ioinfos)
compile time alternatives: - don't swap handles in miniperl\, parse PE header of miniperl.exe with pure Perl (sort of done in exetype.pl already) for a CRT DLL\, then match CRT DLL's name to -size table of ioinfo structs -a def that represents CRT version to pick correct ioinfo struct definition
all ioinfo structs from V6 and onward\, their beginings are common\, MS only adds new members to the END of the ioinfo struct over the years\, it doesn't reorder the members
msvcr71 and msvcrt are identical\, 32 bit ioinfo is (1+1*8)*4=36 bytes long
mov ecx\, eax and eax\, 1Fh sar ecx\, 5 mov ecx\, ___pioinfo[ecx*4] lea eax\, [eax+eax*8] lea eax\, [ecx+eax*4] test byte ptr [eax+4]\, 1
msvcr80 msvcr100 and msvcr90 are identical but 80 and 90 come in SxS dll hell i only checked one 80 and 90\, not all of them\, 32 bit ioinfo is 1 \<\< 6=64 bytes long
mov ecx\, eax and eax\, 1Fh sar ecx\, 5 mov ecx\, ___pioinfo[ecx*4] shl eax\, 6 add eax\, ecx test byte ptr [eax+4]\, 1
p5pRT
commented
11 years ago p5pRT
commented
11 years ago Another problem I didn't thoroughly think through. There might be a side effect to setting the OS handle to INVALID_HANDLE_VALUE before close. MS's close()/close_lk calls _free_osfhnd\, which is NOT exported. MS's _free_osfhnd is a bit more complicated ("if ( __app_type == _CONSOLE_APP ) {" is missing) than this
http://code.ohloh.net/file?fid=wcf_fJvvZwVJPLQrPoTfj7w5Cds&cid=6h85rPuTtus&s=&browser=Default&fp=302894&mpundefined&projSelected=true#L687
but\, note the Win32 console handle setting calls. if you read the code\, if the OS handle in the fd is INVALID_HANDLE_VALUE\, the Win32 console handles wont be reset to their defaults. I did do a trimmed harness run with the assert in the patch not being in assert()\, and no tests failed. I dont understand enough about stdio to know whether the console handles need to be reset\, or can a socket ever wind up as stdin/out console handle for the process.
p5pRT
commented
11 years ago On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On 14 October 2013 21:23\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
I'm not confident that I know enough to judge whether the patch is good so I would appreciate more eyes on this\, but for the record it builds and tests cleanly with the following selection of compilers: VC6\, VC8\, VC11\, MinGW/gcc-4.7.0 (all x86) and Platform SDK for Windows Server 2003 R2 (*) and MinGW-w64/gcc-4.7.1 (both x64).
(*) SDK2003R2 actually fails in t/op/bop.t\, crashing perl.exe\, but does the same without this patch.
p5pRT
commented
11 years ago The RT System itself - Status changed from 'new' to 'open'
p5pRT
commented
11 years ago This patch also made #118059 unreproducible for me\, the connect()/close()/accept() psuedo fork WSAENOTSOCK bug. This patch was written to quiet down the C debugger when I was debugging #118059 from irrelevant noisy C/Win32 exceptions.
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On Mon Oct 14 15:57:32 2013\, shay wrote:
On 14 October 2013 21:23\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
I'm not confident that I know enough to judge whether the patch is good so I would appreciate more eyes on this\, but for the record it builds and tests cleanly with the following selection of compilers: VC6\, VC8\, VC11\, MinGW/gcc-4.7.0 (all x86) and Platform SDK for Windows Server 2003 R2 (*) and MinGW-w64/gcc-4.7.1 (both x64).
(*) SDK2003R2 actually fails in t/op/bop.t\, crashing perl.exe\, but does the same without this patch.
Bump.
On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues except worrying about different versions of the struct in different CRTs. jdb also said he assumes TonyC will manage the ticket/smoke/apply the commit\, not himself. I'm not pasting the exact words since i'm not jdb.
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On Tue Oct 29 06:32:34 2013\, bulk88 wrote:
On Mon Oct 14 15:57:32 2013\, shay wrote:
On 14 October 2013 21:23\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
I'm not confident that I know enough to judge whether the patch is good so I would appreciate more eyes on this\, but for the record it builds and tests cleanly with the following selection of compilers: VC6\, VC8\, VC11\, MinGW/gcc-4.7.0 (all x86) and Platform SDK for Windows Server 2003 R2 (*) and MinGW-w64/gcc-4.7.1 (both x64).
(*) SDK2003R2 actually fails in t/op/bop.t\, crashing perl.exe\, but does the same without this patch.
Bump.
On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues except worrying about different versions of the struct in different CRTs. jdb also said he assumes TonyC will manage the ticket/smoke/apply the commit\, not himself. I'm not pasting the exact words since i'm not jdb.
For my part I'm happy to apply this in the absence of any objections from other quarters\, unless Tony has any more comments\, in particular regarding its overlap with #118059? It seems to stop the occasional failure in dist/IO/t/cachepropagate-tcp.t for me\, which is what #118059 was about.
I've already tested with numerous compilers. I don't think the few Win32 smoke testers would add much to what I've already done and it doesn't affect any non-Win32 code.
One query\, though: what is the comment "shay's VS 2005 definition conflicts with other public code" about in the patch to win32.h?!
And as I've just noted on #118127\, it doesn't make that one unreproducible for me. I get the same crash with or without this patch.
p5pRT
commented
11 years ago On Wed Oct 30 02:15:45 2013\, shay wrote:
On Tue Oct 29 06:32:34 2013\, bulk88 wrote:
On Mon Oct 14 15:57:32 2013\, shay wrote:
On 14 October 2013 21:23\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
I'm not confident that I know enough to judge whether the patch is good so I would appreciate more eyes on this\, but for the record it builds and tests cleanly with the following selection of compilers: VC6\, VC8\, VC11\, MinGW/gcc-4.7.0 (all x86) and Platform SDK for Windows Server 2003 R2 (*) and MinGW-w64/gcc-4.7.1 (both x64).
(*) SDK2003R2 actually fails in t/op/bop.t\, crashing perl.exe\, but does the same without this patch.
Bump.
On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues except worrying about different versions of the struct in different CRTs. jdb also said he assumes TonyC will manage the ticket/smoke/apply the commit\, not himself. I'm not pasting the exact words since i'm not jdb.
For my part I'm happy to apply this in the absence of any objections from other quarters\, unless Tony has any more comments\, in particular regarding its overlap with #118059? It seems to stop the occasional failure in dist/IO/t/cachepropagate-tcp.t for me\, which is what #118059 was about.
I've already tested with numerous compilers. I don't think the few Win32 smoke testers would add much to what I've already done and it doesn't affect any non-Win32 code.
One query\, though: what is the comment "shay's VS 2005 definition conflicts with other public code" about in the patch to win32.h?!
And as I've just noted on #118127\, it doesn't make that one unreproducible for me. I get the same crash with or without this patch.
I think I'll remove mention of #118127 appverifier from the commit message. It not relevant enough to be in git\, the RT posts exist anyway for research. Unless you think it should stay in the commit message. I'll also add that it fixes #118059 cachepropagate which previously was never mentioned in the commit message.
"shay's VS 2005 definition conflicts with other public code" I'll get an explanation for that later today. Need to do some research. Basically I saw some definitions of that struct in other FOSS code\, where __int64 startpos BOOL utf8translations char dbcsBuffer and BOOL dbcsBufferUsed were defined for VC CRT 2005. IDK where you got that struct definition. My REing of assembly says VC 05 shares the same struct as VC 08 and newer\, but your struct said VC 05 was shorter VC 08 and newer. I'll recheck the asm later today.
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On 30 October 2013 17:56\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Wed Oct 30 02:15:45 2013\, shay wrote:
On Tue Oct 29 06:32:34 2013\, bulk88 wrote:
On Mon Oct 14 15:57:32 2013\, shay wrote:
On 14 October 2013 21:23\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 03 10:08:57 2013\, bulk88 wrote:
Patch attached.
Bump\, can someone look at this patch?
I'm not confident that I know enough to judge whether the patch is good so I would appreciate more eyes on this\, but for the record it builds and tests cleanly with the following selection of compilers: VC6\, VC8\, VC11\, MinGW/gcc-4.7.0 (all x86) and Platform SDK for Windows Server 2003 R2 (*) and MinGW-w64/gcc-4.7.1 (both x64).
(*) SDK2003R2 actually fails in t/op/bop.t\, crashing perl.exe\, but does the same without this patch.
Bump.
On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues except worrying about different versions of the struct in different CRTs. jdb also said he assumes TonyC will manage the ticket/smoke/apply the commit\, not himself. I'm not pasting the exact words since i'm not jdb.
For my part I'm happy to apply this in the absence of any objections from other quarters\, unless Tony has any more comments\, in particular regarding its overlap with #118059? It seems to stop the occasional failure in dist/IO/t/cachepropagate-tcp.t for me\, which is what #118059 was about.
I've already tested with numerous compilers. I don't think the few Win32 smoke testers would add much to what I've already done and it doesn't affect any non-Win32 code.
One query\, though: what is the comment "shay's VS 2005 definition conflicts with other public code" about in the patch to win32.h?!
And as I've just noted on #118127\, it doesn't make that one unreproducible for me. I get the same crash with or without this patch.
I think I'll remove mention of #118127 appverifier from the commit message. It not relevant enough to be in git\, the RT posts exist anyway for research. Unless you think it should stay in the commit message. I'll also add that it fixes #118059 cachepropagate which previously was never mentioned in the commit message.
Yes\, I think mentioning #118059 and not #118127 makes sense.
"shay's VS 2005 definition conflicts with other public code" I'll get an explanation for that later today. Need to do some research. Basically I saw some definitions of that struct in other FOSS code\, where __int64 startpos BOOL utf8translations char dbcsBuffer and BOOL dbcsBufferUsed were defined for VC CRT 2005. IDK where you got that struct definition. My REing of assembly says VC 05 shares the same struct as VC 08 and newer\, but your struct said VC 05 was shorter VC 08 and newer. I'll recheck the asm later today.
I assume you're talking about commit 0448a0bdbf which added a bit to the ioinfo struct? I must have got it from the VS 2005 installation that I was adding support for. The version now on my system is updated to Service Pack 1 + Service Pack 1 for Windows Vista\, and the struct is different. I can only assume that the updates changed it. For the record\, here are all the structs from VC6 through VS2013 that I now have:
VC++ 6.0 SP6:
typedef struct { long osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ #ifdef _MT int lockinitflag; CRITICAL_SECTION lock; #endif /* _MT */ } ioinfo;
VS .NET 2003 SP1:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ #ifdef _MT int lockinitflag; CRITICAL_SECTION lock; #endif /* _MT */ } ioinfo;
VS 2005 SP1 + SP1 for Vista:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ int lockinitflag; CRITICAL_SECTION lock; #ifndef _SAFECRT_IMPL /* Not used in the safecrt downlevel. We do not define them\, so we cannot use them accidentally */ char textmode : 7; /* __IOINFO_TM_ANSI or __IOINFO_TM_UTF8 or __IOINFO_TM_UTF16LE */ char unicode : 1; /* Was the file opened as unicode? */ char pipech2[2]; /* 2 more peak ahead chars for UNICODE mode */ __int64 startpos; /* File position that matches buffer start */ BOOL utf8translations; /* Buffer contains translations other than CRLF*/ #endif /* _SAFECRT_IMPL */ } ioinfo;
VS 2008 SP1 / VS2010 SP1 / VS2012 Update 3 / VS2013:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ int lockinitflag; CRITICAL_SECTION lock; #ifndef _SAFECRT_IMPL /* Not used in the safecrt downlevel. We do not define them\, so we cannot use them accidentally */ char textmode : 7; /* __IOINFO_TM_ANSI or __IOINFO_TM_UTF8 or __IOINFO_TM_UTF16LE */ char unicode : 1; /* Was the file opened as unicode? */ char pipech2[2]; /* 2 more peak ahead chars for UNICODE mode */ __int64 startpos; /* File position that matches buffer start */ BOOL utf8translations; /* Buffer contains translations other than CRLF*/ char dbcsBuffer; /* Buffer for the lead byte of dbcs when converting from dbcs to unicode */ BOOL dbcsBufferUsed; /* Bool for the lead byte buffer is used or not */ #endif /* _SAFECRT_IMPL */ } ioinfo;
p5pRT
commented
11 years ago On Wed Oct 30 10:56:49 2013\, bulk88 wrote:
"shay's VS 2005 definition conflicts with other public code" I'll get an explanation for that later today. Need to do some research. Basically I saw some definitions of that struct in other FOSS code\, where __int64 startpos BOOL utf8translations char dbcsBuffer and BOOL dbcsBufferUsed were defined for VC CRT 2005. IDK where you got that struct definition. My REing of assembly says VC 05 shares the same struct as VC 08 and newer\, but your struct said VC 05 was shorter VC 08 and newer. I'll recheck the asm later today.
Oh no. msvcr80.dll 8.0.50727.3053 uses a 64 byte struct. msvcr80.dll 8.0.50727.42 uses a 40 byte struct.
8.0.50727.42
mov ecx\, eax and eax\, 1Fh imul eax\, 28h sar ecx\, 5 mov ecx\, ___pioinfo[ecx*4] add eax\, ecx test byte ptr [eax+4]\, 1
0x28==40
8.0.50727.3053
mov eax\, ecx sar eax\, 5 mov esi\, ecx lea edi\, ___pioinfo[eax*4] mov eax\, [edi] and esi\, 1Fh shl esi\, 6 add eax\, esi test byte ptr [eax+4]\, 1
2\<\<6==64
Both of these were in my WinSxS dir. So shay\, what is the CRT that your VC 2005 uses?
This patch/bug just got alot more complicated.
Is the struct size supposed to be runtime discovered by comparing wrapping a fake magic value handle into a CRT fd\, then looping over the ioinfo struct and comparing 4/8 byte void *s until they match the magic value handle\, then storing the size of the struct in a C static? The static ioinfo struct size is set from PERL_SYS_INIT (the portable version of DllMain in perl)?
Or do the above loop search in the miniperl and write it to a .h during the build process\, swapping handle to -1 in ioinfo struct happens only in full perl\, not miniperl then.
Or take a function pointer to a function inside CRT\, get DLL handle\, then look in DLL's resource section for VS_FIXEDFILEINFO struct and get the file version out of that? Then map the build number of the particular CRT to a hand coded struct size\, fatally error on unknown build number?
the _msize way?
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On Wed\, Oct 30\, 2013 at 5:56 PM\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
Oh no. msvcr80.dll 8.0.50727.3053 uses a 64 byte struct. msvcr80.dll 8.0.50727.42 uses a 40 byte struct. [...] Both of these were in my WinSxS dir. So shay\, what is the CRT that your VC 2005 uses?
This patch/bug just got alot more complicated.
Yeah\, this is the concern you quoted earlier as:
| On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues | except worrying about different versions of the struct in different CRTs.
Is the struct size supposed to be runtime discovered by comparing wrapping a fake magic value handle into a CRT fd\, then looping over the ioinfo struct and comparing 4/8 byte void *s until they match the magic value handle\, then storing the size of the struct in a C static? The static ioinfo struct size is set from PERL_SYS_INIT (the portable version of DllMain in perl)?
Or do the above loop search in the miniperl and write it to a .h during the build process\, swapping handle to -1 in ioinfo struct happens only in full perl\, not miniperl then.
Or take a function pointer to a function inside CRT\, get DLL handle\, then look in DLL's resource section for VS_FIXEDFILEINFO struct and get the file version out of that? Then map the build number of the particular CRT to a hand coded struct size\, fatally error on unknown build number?
the _msize way?
To be honest\, I don't find any of these approaches reasonable; but the first one at least seems to be the least brittle (determine the size of the struct at runtime). Isn't this what Python is doing as well?
Both the 2nd and 3rd approaches require maintenance of a table of runtime library internals\, which grows over time and may be difficult to update in the future. There is always the chance of new versions coming out via service packs that we will miss. I also don't know what kind of binding manifest the compiler generates; e.g. if both of the msvcr80.dll versions would be considered compatible or not.
Cheers\, -Jan
p5pRT
commented
11 years ago On Wed Oct 30 16:39:55 2013\, shay wrote:
On 30 October 2013 17:56\, bulk88 via RT \perlbug\-followup@​perl\.org
I think I'll remove mention of #118127 appverifier from the commit message. It not relevant enough to be in git\, the RT posts exist anyway for research. Unless you think it should stay in the commit message. I'll also add that it fixes #118059 cachepropagate which previously was never mentioned in the commit message.
Yes\, I think mentioning #118059 and not #118127 makes sense.
Ok.
"shay's VS 2005 definition conflicts with other public code" I'll get an explanation for that later today. Need to do some research. Basically I saw some definitions of that struct in other FOSS code\, where __int64 startpos BOOL utf8translations char dbcsBuffer and BOOL dbcsBufferUsed were defined for VC CRT 2005. IDK where you got that struct definition. My REing of assembly says VC 05 shares the same struct as VC 08 and newer\, but your struct said VC 05 was shorter VC 08 and newer. I'll recheck the asm later today.
I assume you're talking about commit 0448a0bdbf which added a bit to the ioinfo struct? I must have got it from the VS 2005 installation that I was adding support for. The version now on my system is updated to Service Pack 1 + Service Pack 1 for Windows Vista\, and the struct is different. I can only assume that the updates changed it. For the record\, here are all the structs from VC6 through VS2013 that I now have:
VC++ 6.0 SP6:
typedef struct { long osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ #ifdef _MT int lockinitflag; CRITICAL_SECTION lock; #endif /* _MT */ } ioinfo;
VS .NET 2003 SP1:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ #ifdef _MT int lockinitflag; CRITICAL_SECTION lock; #endif /* _MT */ } ioinfo;
VS 2005 SP1 + SP1 for Vista:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ int lockinitflag; CRITICAL_SECTION lock; #ifndef _SAFECRT_IMPL /* Not used in the safecrt downlevel. We do not define them\, so we cannot use them accidentally */ char textmode : 7; /* __IOINFO_TM_ANSI or __IOINFO_TM_UTF8 or __IOINFO_TM_UTF16LE */ char unicode : 1; /* Was the file opened as unicode? */ char pipech2[2]; /* 2 more peak ahead chars for UNICODE mode */ __int64 startpos; /* File position that matches buffer start */ BOOL utf8translations; /* Buffer contains translations other than CRLF*/ #endif /* _SAFECRT_IMPL */ } ioinfo;
VS 2008 SP1 / VS2010 SP1 / VS2012 Update 3 / VS2013:
typedef struct { intptr_t osfhnd; /* underlying OS file HANDLE */ char osfile; /* attributes of file (e.g.\, open in text mode?) */ char pipech; /* one char buffer for handles opened on pipes */ int lockinitflag; CRITICAL_SECTION lock; #ifndef _SAFECRT_IMPL /* Not used in the safecrt downlevel. We do not define them\, so we cannot use them accidentally */ char textmode : 7; /* __IOINFO_TM_ANSI or __IOINFO_TM_UTF8 or __IOINFO_TM_UTF16LE */ char unicode : 1; /* Was the file opened as unicode? */ char pipech2[2]; /* 2 more peak ahead chars for UNICODE mode */ __int64 startpos; /* File position that matches buffer start */ BOOL utf8translations; /* Buffer contains translations other than CRLF*/ char dbcsBuffer; /* Buffer for the lead byte of dbcs when converting from dbcs to unicode */ BOOL dbcsBufferUsed; /* Bool for the lead byte buffer is used or not */ #endif /* _SAFECRT_IMPL */ } ioinfo;
I wasn't aware you posted https://rt.perl.org/Ticket/Display.html?id=120091#txn-1266017 before I posted https://rt-archive.perl.org/perl5/Ticket/Display.html?id=120091#txn-1266035 \, so my last post was written without seeing your last post (my last post was written while I was offline\, and sent it when I got back online).
Another VC 2005 CRT I found on my system.
8.0.50727.762
mov ecx\, eax and eax\, 1Fh imul eax\, 38h sar ecx\, 5 mov ecx\, ___pioinfo[ecx*4] add eax\, ecx test byte ptr [eax+4]\, 1
0x38==56 bytes
VC service pack numbers can be detected with macro _MSC_FULL_VER which gives a 4 part Windows style version number\, so the struct definition could use _MSC_FULL_VER to detect all the different VC 2005 distros.
Struct definition after adding VC2005 build 762
typedef struct { intptr_t osfhnd; char osfile; char pipech; // 2 bytes padding int lockinitflag; //12 bytes so far CRITICAL_SECTION lock; //CS is 24 bytes\, 36 bytes so far /* end of VC 2003 and VC 6 */ char textmode : 7; char unicode : 1; char pipech2[2]; //1 byte padding I think\, 40 bytes so far\, /* end of VC 2005 build 42 */ __int64 startpos; //48 bytes so far BOOL utf8translations; //52 bytes so far char dbcsBuffer; // 56 bytes so far\, 3 bytes padding /* end of VC 2005 build 762 */ BOOL dbcsBufferUsed; //60 bytes so far } ioinfo; //pad to 64 because 60 % 8 == 4\, which means unaligned __int64 in next array slice if not padded*/ /* end of VC 2005 build 3053 and VC 2008 and newer */
This link describes different VC 2005 CRT builds\, http://niemiro.co.uk/Blog/windows-update-troubleshooting/visual-c-file-versions/ but doesn't explain ABI compatibility\, or which builds are SxS redirection for security fixes of different ABIs.
Looking through dir Windows/WinSxS/Policies and .policy files
\
\
\
Below is VC 2005 CRT dll I didn't disassemble\, build 6195.
\
So in MS land\, all the VC 2005 CRTs are ABI compatible according to SxS regardless of the VC 2005 linking libs or manifests. I guess the __pioinfo export isn't public API ;)
I guess because of SxS\, the build number in the manifest is irrelevant to what dll is loaded at runtime. A perl can be built with CRT build 42 from VC 2005 SP0. Executes with build 42 on the builder's PC (the builder never uses Windows Updates). The builder then copies the perl build to another PC\, the same perl binary now executes with build 3053 which has a different struct. MS isn't using SxS to have multiple parallel branches of the 2005 CRT\, with all the branches simultaneously getting a wave of updates (with each branch getting its own newer higher build number for its particular security update of that branch). Below is made up example of what is NOT HAPPENING with the 2005 CRTs.
SP0 KB1 SP1 KB2 KB3 SP2 KB4 KB5 68 -> 470 -> 1015 -> 1429 -> 3898 -> 7208 782 -> 990 -> 1788 -> 3537 -> 5927 2371 -> 4145 -> 6093
On Wed Oct 30 18:26:53 2013\, jdb wrote:
On Wed\, Oct 30\, 2013 at 5:56 PM\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
Oh no. msvcr80.dll 8.0.50727.3053 uses a 64 byte struct. msvcr80.dll 8.0.50727.42 uses a 40 byte struct. [...] Both of these were in my WinSxS dir. So shay\, what is the CRT that your VC 2005 uses?
This patch/bug just got alot more complicated.
Yeah\, this is the concern you quoted earlier as:
| On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues | except worrying about different versions of the struct in different CRTs.
Is the struct size supposed to be runtime discovered by comparing wrapping a fake magic value handle into a CRT fd\, then looping over the ioinfo struct and comparing 4/8 byte void *s until they match the magic value handle\, then storing the size of the struct in a C static? The static ioinfo struct size is set from PERL_SYS_INIT (the portable version of DllMain in perl)?
Or do the above loop search in the miniperl and write it to a .h during the build process\, swapping handle to -1 in ioinfo struct happens only in full perl\, not miniperl then.
Or take a function pointer to a function inside CRT\, get DLL handle\, then look in DLL's resource section for VS_FIXEDFILEINFO struct and get the file version out of that? Then map the build number of the particular CRT to a hand coded struct size\, fatally error on unknown build number?
the _msize way?
To be honest\, I don't find any of these approaches reasonable; but the first one at least seems to be the least brittle (determine the size of the struct at runtime). Isn't this what Python is doing as well?
Python uses _msize on the array of ioinfo structs. _pioinfo is a pointer to an array of ioinfo */[]s. Each slice of the 1st array is a pointer to an ioinfo array of 32 ioinfo structs in sequence in memory. The layout is a tiny bit similar to a 2D design of a Perl HV. The _msize is done on the array of 32 ioinfo structs. Since there is always 32 of them\, the size in bytes/32 gives the size of 1 ioinfo struct.
The other algorithm I am thinking of is\, more refined than my earlier descriptions\, do _open_osfhandle on some handle to something temporarily (maybe a pipe)\, find the 32 slice array that the fd falls into (mask operation)\, search the whole ioinfo array (cast to a HANDLE []) for that handle and use IsBadReadPtr to make sure the search loop doesn't overflow past the end of the ioinfo array\, once a 4/8 byte pattern that matches the temp handle is fouind\, replace the HANDLE * that has the temp handle with magic value (_open_osfhandle won't return a new fd for a magic value/invalid handle)\, then do a _get_osfhandle\, compare the returned value to the magic value. If _get_osfhandle didn't return the magic value\, assume the brute force loop found that some member of ioinfo that just happened to have the same bit pattern as the temp handle. Replace the magic value with the temp handle\, keep searching the array for the next instance of temp handle. If access vio mem is found or more than 64(on 32 bit OS)*32 bytes are scanned\, croak() or something else (there might be zero interps in the process when PERL_SYS_INIT3 is called\, PL_curinterp is NULL).
Both options above are forwards compatible if MS adds more members to ioinfo.
Both the 2nd and 3rd approaches require maintenance of a table of runtime library internals\, which grows over time and may be difficult to update in the future. There is always the chance of new versions coming out via service packs that we will miss. I also don't know what kind of binding manifest the compiler generates; e.g. if both of the msvcr80.dll versions would be considered compatible or not.
Manifest redirection quotes are above. If my SxS interpretation is correct (and I hate SxS with a passion)\, all msvcr80.dlls are ABI successors to lower build number.
Yes\, a table will be required but only for VC 2005. The 2005 changes seem to be the biggest problem. The struct never changed in 2008\, 2010\, or 2012\, so its gone stable again. A preprocessor define and a traditional C struct definition of ioinfo struct can deal with everything but VC 2005 builds.
The other question if _msize or brute force or VS_VERSIONINFO and a table are used\, can \<2005 and >2005 be preprocessor hard coded to a ioinfo struct so no startup penalty and 1 C static var is saved? Or does >2005 need to be EOLed by MS first? Or will msvcrt.dll get more members in Win7/8/9? (I dont have any NT 6 machines to get a msvcrt.dll off of them to figure out the size of ioinfo in them) so only VC 2002/2003 can be hard coded safely at this point?
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago On Wed\, Oct 30\, 2013 at 9:11 PM\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
Yes\, a table will be required but only for VC 2005. The 2005 changes seem to be the biggest problem. The struct never changed in 2008\, 2010\, or 2012\, so its gone stable again. A preprocessor define and a traditional C struct definition of ioinfo struct can deal with everything but VC 2005 builds.
Personally I would see no problem with requiring either VC6\, or VS2008 or later for compiling Perl 5.20. VC6 is special because it links against MSVCRT.dll\, but there is nothing special about VS2005 AFAIK.
Just for reference\, Python requires VS2010 for building Python 3.3 and VS2008 for anything before that.
Cheers\, -Jan
p5pRT
commented
11 years ago On Wed Oct 30 18:26:53 2013\, jdb wrote:
On Wed\, Oct 30\, 2013 at 5:56 PM\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
Oh no. msvcr80.dll 8.0.50727.3053 uses a 64 byte struct. msvcr80.dll 8.0.50727.42 uses a 40 byte struct. [...] Both of these were in my WinSxS dir. So shay\, what is the CRT that your VC 2005 uses?
This patch/bug just got alot more complicated.
Yeah\, this is the concern you quoted earlier as:
| On IRC jdb a couple weeks ago in #p5p he said didn't have any major issues | except worrying about different versions of the struct in different CRTs.
Is the struct size supposed to be runtime discovered by comparing wrapping a fake magic value handle into a CRT fd\, then looping over the ioinfo struct and comparing 4/8 byte void *s until they match the magic value handle\, then storing the size of the struct in a C static? The static ioinfo struct size is set from PERL_SYS_INIT (the portable version of DllMain in perl)?
Or do the above loop search in the miniperl and write it to a .h during the build process\, swapping handle to -1 in ioinfo struct happens only in full perl\, not miniperl then.
Or take a function pointer to a function inside CRT\, get DLL handle\, then look in DLL's resource section for VS_FIXEDFILEINFO struct and get the file version out of that? Then map the build number of the particular CRT to a hand coded struct size\, fatally error on unknown build number?
the _msize way?
To be honest\, I don't find any of these approaches reasonable; but the first one at least seems to be the least brittle (determine the size of the struct at runtime). Isn't this what Python is doing as well?
Python uses _msize on the array of ioinfo structs. _pioinfo is a pointer to an array of ioinfo */[]s. Each slice of the 1st array is a pointer to an ioinfo array of 32 ioinfo structs in sequence in memory. The layout is a tiny bit similar to a 2D design of a Perl HV. The _msize is done on the array of 32 ioinfo structs. Since there is always 32 of them\, the size in bytes/32 gives the size of 1 ioinfo struct.
This sounds like the simplest/sanest option to me.
The other question if _msize or brute force or VS_VERSIONINFO and a table are used\, can \<2005 and >2005 be preprocessor hard coded to a ioinfo struct so no startup penalty and 1 C static var is saved? Or does >2005 need to be EOLed by MS first? Or will msvcrt.dll get more members in Win7/8/9? (I dont have any NT 6 machines to get a msvcrt.dll off of them to figure out the size of ioinfo in them) so only VC 2002/2003 can be hard coded safely at this point?
I wouldn't want to rely on VS2008+ not changing the struct more in the future just because things seem to have stabilized for now\, and nor can we even be sure of spotting updates that do change it (if there are any).
p5pRT
commented
11 years ago On Thu\, Oct 31\, 2013 at 11:09 AM\, Steve Hay \steve\.m\.hay@​googlemail\.com wrote:
I wouldn't want to rely on VS2008+ not changing the struct more in the future just because things seem to have stabilized for now\, and nor can we even be sure of spotting updates that do change it (if there are any).
Note though that *all* these approaches always assume that Microsoft will only change/add structure members in the later regions of the struct. If they ever re-arrange the layout of the initial fields\, all of these approaches will fail. We'll just hope that such a failure will be obvious and not subtle and hard to observe.
Cheers\, -Jan
p5pRT
commented
11 years ago On Thu Oct 31 11:10:22 2013\, shay wrote:
I wouldn't want to rely on VS2008+ not changing the struct more in the future just because things seem to have stabilized for now\, and nor can we even be sure of spotting updates that do change it (if there are any).
Patch revised. Some VCs are hard coded\, others use _msize. While developing this patch\, I failed the assert in my_close/my_fclose in cpan/Socket/t/Socket.t because I forgot\, "ioinfo_size /= IOINFO_ARRAY_ELTS;" in Perl_win32_init. I was surprised that sockets are never tested by anything in "/t" too. So jdb's future compatibility concerns are met if you run a DEBUGGING build with a future unknown CRT with WIN32_DYN_IOINFO_SIZE being on. The assert will fail. The assert also catch jdb's https://rt-archive.perl.org/perl5/Ticket/Display.html?id=120091#txn-1266169 what if ioinfo is changed and the first member isn't "intptr_t osfhnd" anymore. The assert works whether WIN32_DYN_IOINFO_SIZE is on or off. The assert won't fail if the bit pattern at the address where the handle swapper thinks the handle is\, happens to be identical to the handle from _get_osfhandle\, but I think the chance of that is very remote. After the 2nd or 3rd run through the handle swapper\, the assert will fail. I can't image the OS handles being identical to the other members in ioinfo\, handle and fd after handle and fd through all of "make test" with DEBUGGING\, and not any other CRT I/O calls going haywire from memory corruption because INVALID_HANDLE_VALUE was written over the wrong member in ioinfo.
I originally wanted w32_ioinfo_size to be of type "unsigned short". The machine code I saw was nasty. An very long 8 byte long movzx to copy and zero extend/cast 16 bits to pointer size. Then a 2-3 bytes (I dont remember) x86 imul. x86 imul only works on pointer size data 16/32/64bits. So just make the C global pointer size so imul can take it direct as an operand. Doing the offset into the ioinfo array as char math won't work because on 32 bits\, ioinfo can be as large as 64 bytes\, and there are 32 of them\, overflow. Native 16 bit math is impossible on x86-32 because the 16 bit opcodes are 32 bit operators in 32 bit mode. 16 bit math usually synthesized using 32 bit operations then truncation by the compiler\, so nothing is saved. Below is the final result "_set_osfhnd(fd\, INVALID_HANDLE_VALUE);" with WIN32_DYN_IOINFO_SIZE (I changed the macro defs to enable WIN32_DYN_IOINFO_SIZE on VC 2003 for testing while making this patch).
8B 15 2C 43 0C 28 mov edx\, ds:__imp____pioinfo 8B CE mov ecx\, fd 83 E1 1F and ecx\, 1Fh 0F AF 0D 88 AF 0E 28 imul ecx\, _w32_ioinfo_size 8B C6 mov eax\, fd C1 F8 05 sar eax\, 5 8B 04 82 mov eax\, [edx+eax*4] 83 0C 01 FF or dword ptr [ecx+eax]\, 0FFFFFFFFh
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
11 years ago p5pRT
commented
11 years ago On 2 November 2013 03:40\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
On Thu Oct 31 11:10:22 2013\, shay wrote:
I wouldn't want to rely on VS2008+ not changing the struct more in the future just because things seem to have stabilized for now\, and nor can we even be sure of spotting updates that do change it (if there are any).
Patch revised. Some VCs are hard coded\, others use _msize.
Thanks\, applied in commit b47a847f62 after testing with:
VC++ 6.0 SP6 x86 Platform SDK for Windows Server 2003 R2 x64 VS .NET 2003 SP1 x86 VS 2005 SP1 x86 VS 2008 SP1 x86 VS 2010 SP1 x86/x64 VS 2012 Update 3 x86 VS 2013 x86 mingw/gcc-4.7.0 x86 mingw-w64/gcc-4.7.1 x64 mingw-w64/gcc-4.8.0 x86
p5pRT
commented
11 years ago Patch now applied\, so closing ticket.
p5pRT
commented
11 years ago @steve-m-hay - Status changed from 'open' to 'resolved'
p5pRT
commented
11 years ago On Wed\, 30 Oct 2013\, Jan Dubois wrote:
On Wed\, Oct 30\, 2013 at 9:11 PM\, bulk88 via RT \perlbug\-followup@​perl\.org wrote:
Yes\, a table will be required but only for VC 2005. The 2005 changes seem to be the biggest problem. The struct never changed in 2008\, 2010\, or 2012\, so its gone stable again. A preprocessor define and a traditional C struct definition of ioinfo struct can deal with everything but VC 2005 builds.
Personally I would see no problem with requiring either VC6\, or VS2008 or later for compiling Perl 5.20. VC6 is special because it links against MSVCRT.dll\, but there is nothing special about VS2005 AFAIK.
Just for reference\, Python requires VS2010 for building Python 3.3 and VS2008 for anything before that.
I guess I better watch commits because VC2005 is what my Windows 2000 smoker runs and I may have to upgrade it if you do so.
-- George Greer
p5pRT
commented
11 years ago On Mon Nov 04 17:35:21 2013\, perl@greerga.m-l.org wrote:
On Wed\, 30 Oct 2013\, Jan Dubois wrote:
Personally I would see no problem with requiring either VC6\, or VS2008 or later for compiling Perl 5.20. VC6 is special because it links against MSVCRT.dll\, but there is nothing special about VS2005 AFAIK.
Just for reference\, Python requires VS2010 for building Python 3.3 and VS2008 for anything before that.
I guess I better watch commits because VC2005 is what my Windows 2000 smoker runs and I may have to upgrade it if you do so.
My day to day C compiler is VC 2003. My more rarely used alternate is VC 2008.
-- bulk88 ~ bulk88 at hotmail.com
p5pRT
commented
10 years ago While doing archeology\, I stumbled onto something that should be documented for this ticket for history reasons.
I never realized it when I wrote this patch/ticket\, but problem that this ticket fixed was actually fixed in Oct 2000 by gsar at http://perl5.git.perl.org/perl.git/commitdiff/a10b7b7eee64efea010bfdba91243503341ba68d . It was removed in http://perl5.git.perl.org/perl.git/commit/9b1f18150a in Dec 2010 by jdb but I didn't see the win32sck.c changes at the end of the diff\, I thought http://perl5.git.perl.org/perl.git/commit/9b1f18150a simply removed the infrastructure for modifying/accessing CRT fd structs which existed for other\, validly removed reasons\, and didnt realize it removed the sockets fix (swap kernel handle in fd struct before closing the socket fd) I was (now) re-adding. rjbs commented on IRC can this be tested for? not really\, it fixed a intermittent test fail race condition in dist\IO\t\cachepropagate-tcp.t from #118059\, but I see no way of recreating the race condition on demand.
TLDR\, not happening without a kernel driver (see htrace feature of WinDbg http://blogs.msdn.com/b/spatdsg/archive/2005/03/23/401020.aspx ) and stepping threads
Recreating the race would mean trying to synchronize step by step asm wise the user mode Microsoft closed source OS sockets library in 2 different threads/psuedo-forks. Basically something in that sockets library\, when you call "closesocket" it lets the other sockets library using threads know the socket handle is dead in user mode or "half dead state" and the other threads's sockets behavior works correctly. If you call closesocket then close/CloseHandle\, something is probably screwed up during the cleanup\, and there is a race condition as the other thread sees a half freed socket and tries to use it. Or the socket reverts from half dead (locked) to invalid (unlocked) to freed (unlockable) and during invalid state the user mode sockets library okays doing things on the socket\, but the kernel side tcp driver doesn't recognize the handle anymore due to the CloseHandle and somewhere in there is a race in all that. I know the win32 Sockets library will do callbacks from kernel mode into user mode to update statuses/events of sockets\, which are user mode structs that get synced with kernel mode structs. Written to a process's VM space from kernel-mode is a bit complicated (threads\, map and lock a user-mode page into kernel vm\, SEGVs\, locking paging\, locking data bus\, etc)\, so running a user mode callback is easiest solution for MS's devs.
-- bulk88 ~ bulk88 at hotmail.com
Migrated from rt.perl.org#120091 (status was 'resolved')
Searchable as RT120091$