FW: Problem with Winsock based Exxx constants in VS2010+

Forwarding some private emails regarding Winsock based Exxx constants to p5p for archiving, as suggested by Jan.
The changes discussed are now committed in ea95436966.


-----Original Message-----
From: Jan Dubois [mailto:jand@activestate.com]
Sent: 14 September 2013 02:10
To: Steve Hay
Subject: Re: Problem with Winsock based Exxx constants in VS2010+

Hi Steve,

Please go ahead and commit as you see fit.  I don't seem to be getting any time to look into this in more details than the high-level feedback I gave you so far.

Also feel free to forward any part of our conversation to p5p if you feel that it should have a permanent record in the archives.

Cheers,
-Jan

----------------------------------------------------------------------
From:	Steve Hay
Sent:	08 September 2013 00:48
To:	Jan Dubois
Subject:	RE: Problem with Winsock based Exxx constants in VS2010+
Attachments:	test.c; test.pl; vc6test.c.txt; vc6test.pl.txt; vc10test.c.txt; vc10test.pl.txt

Jan Dubois wrote on 2013-08-31:
> On Fri, Aug 23, 2013 at 9:51 AM, Steve Hay 
> <Steve.Hay@verosoftware.com>
> wrote:
> [...]
> 
>> To avoid that, I imagine I would have to intercept any assignment to $!
>> and do the WSAExxx -> Exxx mapping there, rather than in a function 
>> only available to the core in win32sck.c as I have currently done. 
>> Does that sound feasible/sensible?
> 
> Yes, I think this is an excellent idea!

I've now implemented the interception of assignment to $! in Perl_magic_set(), 
which does indeed fix the Net::Ping test breakage. 


> 
>> Finally, I also found (after searching for more hard-coded 100xy
>> numbers) that IO::Socket contains a test for $! == 10022, which will 
>> be broken with the new scheme because $! will now be set to 22 rather 
>> than 10022. This is a nasty problem which I haven't yet got a plan 
>> for how to solve. The Errno/POSIX value of EINVAL is and has always 
>> been 22, so although testing against that will work nicely now, no 
>> existing code will be using that because it would have needed to test 
>> for 10022 (which there is no named constant for).
>> 
>> So to check for EINVAL one would have to write something like this:
>> 
>>         if ($! == (($^O eq 'MSWin32' && $] < 5.019004) ? 10022 :
>> EINVAL))
>> 
>> which doesn't seem very friendly.
> 
> It *might* be possible to solve this with overload magic, but I'm 
> afraid that this might require turning $! into a blessed reference (so 
> that the overload code can find the right methods for performing the 
> comparison).  It sounds a bit too convoluted to deal with just a few 
> backward compatibility cases.

I've left this out for now. I agree that the breakage concerned is 
sufficiently minor that it doesn't warrant such complications, but I will keep 
this possible solution in mind for the future -- there's nothing to stop it 
being done at a later date if it turns out to be more of a problem than 
anticipated.


> 
> [...]
> 
> One thing that I'm still wondering about is the stringification of 
> errno values. If you look at win32_strerror, you'll see that we have 
> special code to stringify errno values larger than sys_nerr via a call 
> to FormatMessage() using the system message catalog.
> 
> So I wonder if strerror() in msvcrt.dll will provide the correct 
> stringification for these additional error codes, especially on older 
> operating systems.  I guess this is not an issue if you compile
> *everything* with VC10, but I'm also thinking about mixing compilers 
> for Perl itself and for the modules.

Thank you -- I hadn't been aware of that code. I don't think that it's a 
(new!) problem, though, since it turns out that sys_nerr is still 43 in VC10, 
exactly as it was in VC6->VC9. So, in fact, that function is currently broken 
in VC10 builds anyway because errno.h codes in the range 100->140 get wrongly 
treated as winerror.h codes, e.g. 

perl -le "$!=106; print $!"

prints "Insert the diskette for drive %1." (the message for 
ERROR_SEM_USER_LIMIT) rather than "An established connection was aborted by 
the software in your host machine." (the message for ECONNABORTED, which is 
surely what would have been intended in the context in which the function is 
used).

I've now fixed that in my branch by mapping codes in the range 100->140 back 
to winerror.h/winsock2.h codes for VC10+ (or, in fact, any compiler which 
happens to have 100 in errno.h).


> 
> ActivePerl will be using a Mingw64 based compiler for 5.20 (same as 
> Strawberry), so it will continue to link against msvcrt.dll. I haven't 
> checked if the headers include the new E* constants, but if they do, 
> will they work properly for the version of MSVCRT.dll available on WinXP?

MinGW-w64/gcc-4.7.1 does not contain errnos higher than 42. I have no idea if 
they might change that in the future, but given that msvcr100.dll's strerror() 
doesn't provide messages for codes above 42 anyway (I tried it out in a 
standalone C program, btw: strerror(106) just returns "Unknown error") I don't 
think it will be an issue: the solution that I've implemented should work 
whether errno.h contains the extra errnos or not, regardless of which C run-
time is being used. 


[...]
> 
> PS: I found the corresponding mapping function in the Tcl source, in 
> case you want to take a look for comparison:
> 
> http://core.tcl.tk/tcl/artifact/055bdd9a7b71a8b90490e87da97623686d73b4 
> 1
> 4

I had a look at that, and also a (surprisingly-similar-to-perl's!) function in 
Ruby's win32/win32.c, and I don't think they cover anything relevant that I've 
missed. (They both handle more winerror.h error constants than just the 
winsock2.h ones, but we only seem to assign sockets errors to $! in perl, so 
I've ignored the rest.)

I have attached a script which shows what number ends up in $!, and how that 
sringifies, when all relevant values are assigned to $!, together with the 
outputs in a VC6 build and a VC10 build -- both on Windows 7, but I also tried 
on Windows XP and got identical results. I also tried a MinGW-w64 (gcc-4.7.1) 
build on both machines and got identical results to the VC6 builds.

The results are all as I would now expect, except for one minor fly in the 
ointment, namely that this:

perl -le "$!=10022; print $!"

now prints "Invalid argument" (the strerror() string for 22, which is actually 
what gets assigned to $!) rather than "An invalid argument was supplied." (the 
FormatMessage() string for 10022), but given that perl now assigns errno.h 
values rather than winsock2.h values to $! I don't think this really matters. 
(The handling of this $!=10022 case is only for backwards-compatibility with 
user code that may still be doing that.)

My branch also fixes the problems that I currently have with mod_perl. I've 
also attached a C program that outputs all relevant Exxx constants after 
including the Perl headers in the manner of XS extensions, plus its output in 
VC6 and VC10 builds (and again, MinGW-w64/gcc-4.7.1 is the same as VC6). The 
output is also as I would now expect, with ECONNABORTED being 106 rather than 
10053.

Many thanks for your comments on all this. All the changes can be seen in my 
updated smoke-me/steveh/errno branch (commit a52ed91023) if you have any time 
to look.

If all goes well with the smoking then I would like to commit this soon (in 
time for 5.19.4 on 20th would be nice, but is not essential), unless you would 
prefer that I hold off until you have a chance to take a closer look yourself?

Cheers,
Steve

----------------------------------------------------------------------
From:	Jan Dubois
Sent:	31 August 2013 02:19
To:	Steve Hay
Subject:	Re: Problem with Winsock based Exxx constants in VS2010+

On Fri, Aug 23, 2013 at 9:51 AM, Steve Hay <Steve.Hay@verosoftware.com> wrote:
> Hi Jan,
>
> Sorry to bother you again, but I was wondering if you'd had a chance 
> to give this any consideration?

Hi Steve,

sorry for being so slow in getting back to you! I'm always hoping to get some 
time to experiment with this myself, but this never actually happens...

[...]

> To avoid that, I imagine I would have to intercept any assignment to $!
> and do the WSAExxx -> Exxx mapping there, rather than in a function 
> only available to the core in win32sck.c as I have currently done. 
> Does that sound feasible/sensible?

Yes, I think this is an excellent idea!

> Finally, I also found (after searching for more hard-coded 100xy
> numbers) that IO::Socket contains a test for $! == 10022, which will 
> be broken with the new scheme because $! will now be set to 22 rather 
> than 10022. This is a nasty problem which I haven't yet got a plan for 
> how to solve. The Errno/POSIX value of EINVAL is and has always been 
> 22, so although testing against that will work nicely now, no existing 
> code will be using that because it would have needed to test for 10022 
> (which there is no named constant for).
>
> So to check for EINVAL one would have to write something like this:
>
>         if ($! == (($^O eq 'MSWin32' && $] < 5.019004) ? 10022 :
> EINVAL))
>
> which doesn't seem very friendly.

It *might* be possible to solve this with overload magic, but I'm afraid that 
this might require turning $! into a blessed reference (so that the overload 
code can find the right methods for performing the comparison).  It sounds a 
bit too convoluted to deal with just a few backward compatibility cases.

[...]

> I've pushed the patch that I sent you before, plus a couple of ugly 
> hacks to Net::Ping and IO::Socket to workaround the problems described 
> above (which both need sorting out properly somehow), to 
> smoke-me/steveh/errno. This branch passes all tests for me with VC10 
> and currently has four PASSes on linux at 
> http://perl.develop-help.com/?b=smoke-me/steveh/errno too.

I wanted to try it out, but still haven't gotten around to it yet.

One thing that I'm still wondering about is the stringification of errno 
values. If you look at win32_strerror, you'll see that we have special code to 
stringify errno values larger than sys_nerr via a call to FormatMessage() 
using the system message catalog.

So I wonder if strerror() in msvcrt.dll will provide the correct 
stringification for these additional error codes, especially on older 
operating systems.  I guess this is not an issue if you compile
*everything* with VC10, but I'm also thinking about mixing compilers for Perl 
itself and for the modules.

ActivePerl will be using a Mingw64 based compiler for 5.20 (same as 
Strawberry), so it will continue to link against msvcrt.dll. I haven't checked 
if the headers include the new E* constants, but if they do, will they work 
properly for the version of MSVCRT.dll available on WinXP?

Sorry for the rambling, haven't had time to properly organize my thoughts.

> Apologies for the long email, and thank you if you've read this far! 
> To summarize, my questions are:
>
> 1. Is the general approach taken ok?

Yes

> 2. Is it feasible/sensible to try to intercept all assignments to $! 
> to stop third-party code assigning WSAExxx values?

Yes, all $! assignments go through Perl_magic_set().  I was wondering if you 
should put the intercept into the SETERRNO() macro, but I don't think it is 
needed.  Only our socket code, or Perl level assignment to $! should need the 
translation.

> 3. Is the backwards-compatibility breakage regarding EINVAL and other 
> Exxx values < 100 acceptable for 5.20.0?

This is clearly a judgement call. I find it acceptable, given that leaving 
things as they are results in a bunch of broken code too.

Cheers,
-Jan

PS: I found the corresponding mapping function in the Tcl source, in case you 
want to take a look for comparison:

http://core.tcl.tk/tcl/artifact/055bdd9a7b71a8b90490e87da97623686d73b414

----------------------------------------------------------------------
From:	Steve Hay
Sent:	23 August 2013 17:52
To:	Jan Dubois
Subject:	RE: Problem with Winsock based Exxx constants in VS2010+

Hi Jan,

Sorry to bother you again, but I was wondering if you'd had a chance to give 
this any consideration?

I have since looked at this some more myself and found the cause of the Net-
Ping test failure. The problem is in its tcp_connect() function, which does 
this to fetch a socket-level error code:

	unpack("i", getsockopt($self->{"fh"}, SOL_SOCKET, SO_ERROR))

It assigns the result (which is a WSAExxx value) to $!, which now causes 
problems because in the new scheme of things we map WSAExxx values Exxx values 
for $! and accordingly define Errno's and POSIX's Exxx values as the errno.h 
values, hence the subsequent test for $! == ECONNREFUSED in ping_tcp() is now 
comparing 10061 to 140.

This could be an issue in other third-party code too: anyone can assign 
anything to $! (or errno in XS code), and until now people will have been used 
to assigning WSAExxx values (possibly from WSAGetLastError() or maybe using 
getsockopt() as above), but that's wrong now since Errno's and POSIX's Exxx 
values are now the errno.h values rather than winsock2.h values.

To avoid that, I imagine I would have to intercept any assignment to $! and do 
the WSAExxx -> Exxx mapping there, rather than in a function only available to 
the core in win32sck.c as I have currently done. Does that sound 
feasible/sensible?

I also noticed that Net/Ping.pm contains a bunch of hard-coded 100xy numbers, 
but they appear harmless: they are only used to set the likes of ECONNREFUSED 
if they are not already set, but AFAIK any remotely modern Perl will already 
have them (in this case, imported from POSIX).

Finally, I also found (after searching for more hard-coded 100xy numbers) that 
IO::Socket contains a test for $! == 10022, which will be broken with the new 
scheme because $! will now be set to 22 rather than 10022. This is a nasty 
problem which I haven't yet got a plan for how to solve. The Errno/POSIX value 
of EINVAL is and has always been 22, so although testing against that will 
work nicely now, no existing code will be using that because it would have 
needed to test for 10022 (which there is no named constant for).

So to check for EINVAL one would have to write something like this:

	if ($! == (($^O eq 'MSWin32' && $] < 5.019004) ? 10022 : EINVAL))

which doesn't seem very friendly.

This is really a problem with how things currently are (and have long been), 
namely that $! is being set to winsock2.h values, but if there is a 
corresponding errno.h value and it is < 100 then *that* is used for the 
Errno/POSIX constant, so authors have no good way of testing for these $! 
values. The new scheme corrects that by consistently using the same values for 
$! and for Errno/POSIX (except in the case of third-party code setting its own 
$!/errno values mentioned above), but nastily breaks backwards-compatibility.

(Ironically, Exxx values >= 100 (the ones which caused all this trouble in the 
first place!) don't have this problem as long as code uses the Exxx constants 
rather than the hard-coded 100xy numbers. For example, if code tests $! == 
ECONNABORTED, where $! and ECONNABORTED were both previously set to winsock2.h 
values then the same test will still work, except that now $! and ECONNABORTED 
are both set to errno.h values in VC10+ (or both left as winsock2.h values 
upto VC9). It does still break if any code is directly testing $! == 10053, 
but that's not such a problem: that's really broken code, which should be 
using the constant instead.)

I've pushed the patch that I sent you before, plus a couple of ugly hacks to 
Net::Ping and IO::Socket to workaround the problems described above (which 
both need sorting out properly somehow), to smoke-me/steveh/errno. This branch 
passes all tests for me with VC10 and currently has four PASSes on linux at 
http://perl.develop-help.com/?b=smoke-me/steveh/errno too.

Apologies for the long email, and thank you if you've read this far! To 
summarize, my questions are:

1. Is the general approach taken ok?
2. Is it feasible/sensible to try to intercept all assignments to $! to stop 
third-party code assigning WSAExxx values?
3. Is the backwards-compatibility breakage regarding EINVAL and other Exxx 
values < 100 acceptable for 5.20.0?

Thanks,
Steve


> -----Original Message-----
> From: Steve Hay
> Sent: 07 August 2013 22:55
> To: Jan Dubois
> Subject: RE: Problem with Winsock based Exxx constants in VS2010+
> 
> Hi Jan,
> 
> I've had a crack at doing this, and am attaching a patch (plus a new 
> file which the diff didn't pick up) of what I have so far to see if 
> you think I'm on the right lines before I spend any more time on it.
> 
> The main change is to use a new function throughout win32/win32sck.c 
> to convert WSAGetLastError() values into errno.h constants before 
> assigning to errno. I've mapped every possible WSAExxx value (as 
> documented by MSDN) to an Exxx value, but left the possibility of 
> other WSAxxx values cropping up and getting assigned to errno 
> unchanged because this has never been otherwise (and any non-existent 
> Exxx values get mapped back to WSAExxx values anyway...).
> 
> We must then ensure that all of those Exxx values are defined, which 
> is now done (in the new file, win32/include/sys/errno2.h) in a 
> friendlier manner, being careful not to redefine any (specifically the 
> new ones in
> VC++ 2010 and above) which already exist. The rest are defined as the
> WSAExxx values, as mentioned above.
> 
> Finally, we need the Errno module to know about these values, which 
> I've done by having it include that same new header file to ensure 
> that it gets the same definitions, rather than having to play its own games.
> I'm undecided whether that's a good move or not (and I'm uneasy about 
> my choice of location/name for the new file). What do you think? The 
> new header is also used in POSIX, which similarly wants definitions of 
> as many as possible of its hard-coded list of Exxx values, and I think 
> the change here is an improvement on the mass of duplication that 
> would otherwise have been involved since it didn't do its own mapping 
> as slickly as Errno did.
> 
> This builds successfully (and installing does install the new header 
> file, as required) and all tests pass except for one failure in Net- 
> Ping/t/450_service.t which I will look at in due course.
> 
> Any comments would be very welcome.
> 
> Thanks,
> Steve
> 
> 
> > -----Original Message-----
> > From: Jan Dubois
> > Sent: 31 July 2013 21:03
> > To: Steve Hay
> > Subject: Re: Problem with Winsock based Exxx constants in VS2010+
> >
> > Hi Steve,
> >
> > I think the proper way to deal with this will be to translate the
> WSA*
> > error codes to the corresponding errno values before storing them in 
> > errno.
> >
> > This may break modules with hard-coded errno values, but I think 
> > this will be unavoidable if we want to get to a sane state.  I think 
> > I had see some constants in autodie, but can't find it anymore.
> >
> > Changing the $! values is an incompatible change, so should probably 
> > not be done for 5.18.x, so I'm not sure where that leaves you with 
> > mod_perl.
> >
> > Let me know what you think, and if you are going to tackle the 
> > WSAGetLastError -> errno remapping, or if you want some help from me!
> >
> > Cheers,
> > -Jan

• FW: Problem with Winsock based Exxx constants in VS2010+ by Steve Hay