Closed p5pRT closed 11 years ago
(no detailed Perl -V information because the bug is on Windows which I do not run... discovered by cpantesters)
The following script catches the die() used by Carp::confess. Only on Windows\, the $! get descroyed when confess is used with "\n".
#!/usr/bin/perl use warnings; use strict;
use Carp;
$SIG{__DIE__} = sub { print "@_: $! (rc=".($!+0).")\n" };
print "--> without \\n\n"; eval { $! = 3; confess "oeps" }; print "\n--> with \\n\n"; eval { $! = 3; confess "oeps\n" };
Output on all platforms except Windows (afaik) as expected:
--> without \n oeps at /tmp/y line 10 eval {...} called at /tmp/y line 10 : No such process (rc=3)
--> with \n oeps at /tmp/y line 12 eval {...} called at /tmp/y line 12 : No such process (rc=3)
Output when run on Strawberry Perl 5.12.1.0 (x64) (and other versions of Windows)
--> without \n
oeps at y line 10
eval {...} called at y line 10
: No such process (rc=3)
--> with \n
oeps
at y line 12
eval {...} called at y line 12
: (rc=0)
-- Regards\, MarkOv
Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net
Output when run on Strawberry Perl 5.12.1.0 (x64) (and other versions of Windows)
--> without \n oeps at y line 10 eval {...} called at y line 10 : No such process (rc=3)
--> with \n oeps at y line 12 eval {...} called at y line 12 : (rc=0)
The same output with all: - strawberry perl 5.10.1 / 5.12.1 - active perl 5.10.1 / 5.12.1
-- kmx
The RT System itself - Status changed from 'new' to 'open'
On Wed Jan 12 04:29:05 2011\, kmxx wrote:
Output when run on Strawberry Perl 5.12.1.0 (x64) (and other versions of Windows)
--> without \n oeps at y line 10 eval {...} called at y line 10 : No such process (rc=3)
--> with \n oeps at y line 12 eval {...} called at y line 12 : (rc=0)
The same output with all: - strawberry perl 5.10.1 / 5.12.1 - active perl 5.10.1 / 5.12.1
5.18:
C:\Users\tony\play>perl -E "$SIG{__DIE__} = sub { say qq(error: $!) }; use Carp 'confess'; $!=3; confess qq'foo\n'" error: foo at -e line 1.
blead:
C:\Users\tony\play>\testperlx\bin\perl -E "$SIG{__DIE__} = sub { say qq(error: $ !) }; use Carp 'confess'; $!=3; confess qq'foo\n'" error: No such process foo at -e line 1.
Nothing leaps out at me as the cause of the fix though.
Tony
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~ s/([[:cntrl:]]|[[:^ascii:]])/sprintf("\\x{%x}"\,ord($1))/eg;
On recent versions of Windows\, any call to atoi clears $! (and possibly sets it in case of overflow). When the match succeeds in the line above\, evaluation of the subst makes a call to atoi (in mg.c:Perl_magic_get because of the use of $1) and thus resets $!. The presence of \n is what triggers the match.
Thread 116118 got somewhat sidetracked by other considerations but I would love to see some discussion of the fix I proposed and of the alternative suggested by bulk88 (resp. on 11 and 12 jan 2013).
Christian
Le 27/08/2013 08:17\, Tony Cook via RT a écrit :
On Wed Jan 12 04:29:05 2011\, kmxx wrote:
Output when run on Strawberry Perl 5.12.1.0 (x64) (and other versions of Windows)
--> without \n oeps at y line 10 eval {...} called at y line 10 : No such process (rc=3)
--> with \n oeps at y line 12 eval {...} called at y line 12 : (rc=0)
The same output with all: - strawberry perl 5.10.1 / 5.12.1 - active perl 5.10.1 / 5.12.1
5.18:
C:\Users\tony\play>perl -E "$SIG{__DIE__} = sub { say qq(error: $!) }; use Carp 'confess'; $!=3; confess qq'foo\n'" error: foo at -e line 1.
blead:
C:\Users\tony\play>\testperlx\bin\perl -E "$SIG{__DIE__} = sub { say qq(error: $ !) }; use Carp 'confess'; $!=3; confess qq'foo\n'" error: No such process foo at -e line 1.
Nothing leaps out at me as the cause of the fix though.
Tony
--- via perlbug: queue: perl5 status: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=81586
On Tue\, Aug 27\, 2013 at 11:20:56AM +0200\, Christian Millour wrote:
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~ s/\(\[\[​:cntrl​:\]\]|\[\[​:^ascii​:\]\]\)/sprintf\("\\\\x\{%x\}"\,ord\($1\)\)/eg;
On recent versions of Windows\, any call to atoi clears $! (and possibly sets it in case of overflow). When the match succeeds in the line above\,
I'm curious - is a violation of the C standard? Although I agree with your assessment in the other thread that it's likely here to stay.
evaluation of the subst makes a call to atoi (in mg.c:Perl_magic_get because of the use of $1) and thus resets $!. The presence of \n is what triggers the match.
Thread 116118 got somewhat sidetracked by other considerations but I would love to see some discussion of the fix I proposed and of the alternative suggested by bulk88 (resp. on 11 and 12 jan 2013).
I missed that thread - I'll have proper look now.
Nicholas Clark
On Tue\, Aug 27\, 2013 at 10:30:21AM +0100\, Nicholas Clark wrote:
On Tue\, Aug 27\, 2013 at 11:20:56AM +0200\, Christian Millour wrote:
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~ s/\(\[\[​:cntrl​:\]\]|\[\[​:^ascii​:\]\]\)/sprintf\("\\\\x\{%x\}"\,ord\($1\)\)/eg;
On recent versions of Windows\, any call to atoi clears $! (and possibly sets it in case of overflow). When the match succeeds in the line above\,
I'm curious - is a violation of the C standard? Although I agree with your assessment in the other thread that it's likely here to stay.
Yes\, it's a violation under both C89 and C99.
Standard C library functions may not set errno to 0 (except temporarily\, internally.)
Tony
Le 27/08/2013 11:43\, Tony Cook a écrit :
On Tue\, Aug 27\, 2013 at 10:30:21AM +0100\, Nicholas Clark wrote:
On Tue\, Aug 27\, 2013 at 11:20:56AM +0200\, Christian Millour wrote:
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~ s/\(\[\[​:cntrl​:\]\]|\[\[​:^ascii​:\]\]\)/sprintf\("\\\\x\{%x\}"\,ord\($1\)\)/eg;
On recent versions of Windows\, any call to atoi clears $! (and possibly sets it in case of overflow). When the match succeeds in the line above\,
I'm curious - is a violation of the C standard? Although I agree with your assessment in the other thread that it's likely here to stay.
Yes\, it's a violation under both C89 and C99.
Standard C library functions may not set errno to 0 (except temporarily\, internally.)
Tony
[Tony\, sorry for the duplicate response. I'll try to be more careful]
I can't find the relevant quote in C89 or C99 but on Linux\, man 3 errno reads
The \<errno.h> header file defines the integer variable errno\, which is set by system calls and some library functions in the event of an error to indicate what went wrong. Its value is significant only when the return value of the call indicated an error (i.e.\, -1 from most system calls; -1 or NULL from most library functions); a function that succeeds is allowed to change errno.
From the first sentence it seems that the boundary between syscalls and library function is rather fuzzy. And the last sentence states that atoi could start resetting errno on linux and still be compliant.
Christian
Tony Cook via RT wrote:
Nothing leaps out at me as the cause of the fix though.
I wouldn't call this a "fix". Carp has never guaranteed to preserve $!\, or other state variables\, from invocation of croak/confess to their invocation of die.
-zefram
Le 27/08/2013 12:49\, Zefram a écrit :
Tony Cook via RT wrote:
Nothing leaps out at me as the cause of the fix though.
I wouldn't call this a "fix". Carp has never guaranteed to preserve $!\, or other state variables\, from invocation of croak/confess to their invocation of die.
-zefram
opinions on this seem to differ. As mentioned in #116118 :
Note incidentally that dist/Carp/t/Carp.t explicitly checks that croak() and confess() don't clobber $!. It tests it badly\, as it turns out\, on MSWin32\, as the test would fail if the actual message contained any character matching ([[:cntrl:]]|[[:^ascii:]]).
Since I tend to use the tests as supplemental/definitive documentation for any APIs\, I had inferred that Carp indeed made some efforts to preserve $!. And\, apart from this bug triggered by the uncommon behavior of atoi on Windows\, it would seem that it generally indeed does not clobber it.
Christian
Le 27/08/2013 13:18\, Christian Millour a écrit :
Le 27/08/2013 12:49\, Zefram a écrit :
Tony Cook via RT wrote:
Nothing leaps out at me as the cause of the fix though.
I wouldn't call this a "fix". Carp has never guaranteed to preserve $!\, or other state variables\, from invocation of croak/confess to their invocation of die.
-zefram
opinions on this seem to differ. As mentioned in #116118 :
Note incidentally that dist/Carp/t/Carp.t explicitly checks that croak() and confess() don't clobber $!. It tests it badly\, as it turns out\, on MSWin32\, as the test would fail if the actual message contained any character matching ([[:cntrl:]]|[[:^ascii:]]).
Since I tend to use the tests as supplemental/definitive documentation for any APIs\, I had inferred that Carp indeed made some efforts to preserve $!. And\, apart from this bug triggered by the uncommon behavior of atoi on Windows\, it would seem that it generally indeed does not clobber it.
Christian
To expand on this\, I believe it is essential that croak and confess do
not clobber $! when you need to support different perl versions or
different locales\, and want to recover from transient errors reported
with $! via croak/confess. With various versions/os/locales\, you have no
control over the error message (e.g. ENOSPC: "no space left on device"
might morph into "Pas de place sur le périphérique" on a french box).
You thus cannot rely on matching even approximate strings to pinpoint
the error and take appropriate action (which might be different between
e.g. EACCES and ENOSPC)\, so the message produced by
die "oops: $!"
or
croak "oops: $!"
are essentially useless for programmatic recovery. What you need instead
is the numerical value of errno\, which is part of the dualvar $!. If you
use a module that dies on error\, you can catch the error\, immediately
inspect the numerical value of $!\, and act accordingly. If the module
croaks instead\, shouldn't you be able to do the same ?
For the same reason I believe that croak and confess should not clobber $^E either. At this point I have no opinion regarding other state variables.
Christian
Christian Millour wrote:
If you use a module that dies on error\, you can catch the error\, immediately inspect the numerical value of $!\, and act accordingly.
Interesting argument. You've neatly addressed the obvious issue that the $SIG{__DIE__} handler seems the wrong place to capture $!.
For the same reason I believe that croak and confess should not clobber $^E either.
If this is to be a deliberate feature\, it should be documented\, it should be tested more thoroughly\, and it should be implemented explicitly by something like "local($!\, $^E);" in longmess and shortmess.
-zefram
Le 27/08/2013 15:11\, Zefram a écrit :
Christian Millour wrote:
If you use a module that dies on error\, you can catch the error\,
immediately inspect the numerical value of $!\, and act accordingly.
Interesting argument. You've neatly addressed the obvious issue that the $SIG{__DIE__} handler seems the wrong place to capture $!.
For the same reason I believe that croak and confess should not clobber $^E either.
If this is to be a deliberate feature\, it should be documented\, it should be tested more thoroughly\, and it should be implemented explicitly by something like "local($!\, $^E);" in longmess and shortmess.
That is certainly a quick fix\, one that I suggested in #116118 and have implemented in my home-patched perls. It does not address the larger problem of whether perl should try to preserve $! and $^E in general.
The clobbering of $! by croak/confess is caused by the new behavior of recent versions of atoi on windows. This has other repercussions\, for instance the fact that you cannot mix $! and numbered match vars in the same expression (see http://www.nntp.perl.org/group/perl.perl5.porters/2013/01/msg197262.html). Whether those are serious or ignorable should be discussed. In any case I would prefer to address the root of the problem\, and would contend that perl might be better served by using its own atoi (at least on windows).
Regarding thorough tests\, I absolutely concur. One (small) problem is that I see no obvious location in the current source to host them. For instance\, the link above contains a test for mixing match variables and $!. Where should it go\, assuming it is valid ? Perl currently assumes that atoi does not clobber $!. This is currently not tested\, but could be to prevent future surprises. Where should such a test go ?
Christian
On Tue\, Aug 27\, 2013 at 12:24:44PM +0200\, Christian Millour wrote:
Le 27/08/2013 11:43\, Tony Cook a écrit :
On Tue\, Aug 27\, 2013 at 10:30:21AM +0100\, Nicholas Clark wrote:
On Tue\, Aug 27\, 2013 at 11:20:56AM +0200\, Christian Millour wrote:
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~ s/\(\[\[​:cntrl​:\]\]|\[\[​:^ascii​:\]\]\)/sprintf\("\\\\x\{%x\}"\,ord\($1\)\)/eg;
I think that it may be possible to eliminate the calls to atoi() for getting and setting $1 etc\, and (effectively) move them to compile time in gv.c
On recent versions of Windows\, any call to atoi clears $! (and possibly sets it in case of overflow). When the match succeeds in the line above\,
I'm curious - is a violation of the C standard? Although I agree with your assessment in the other thread that it's likely here to stay.
Yes\, it's a violation under both C89 and C99.
Standard C library functions may not set errno to 0 (except temporarily\, internally.)
I can't find the relevant quote in C89 or C99 but on Linux\, man 3 errno reads
The \<errno.h> header file defines the integer variable errno\, which is set by system calls and some library functions in the event of an error to indicate what went wrong. Its value is significant only when the return value of the call indicated an error (i.e.\, -1 from most system calls; -1 or NULL from most library functions); a function that succeeds is allowed to change errno.
From the first sentence it seems that the boundary between syscalls and library function is rather fuzzy. And the last sentence states that atoi could start resetting errno on linux and still be compliant.
The value of errno is zero at program startup\, but is never set to zero by any library function.176) The value of errno may be set to nonzero by a library function call whether or not there is an error\, provided the use of errno is not documented in the description of the function in this International Standard.
7.5.3 in a late draft of C99. Same in a draft of C11\, except that the footnote number has changed.
Nicholas Clark
On Wed Aug 28 04:43:57 2013\, nicholas wrote:
On Tue\, Aug 27\, 2013 at 12:24:44PM +0200\, Christian Millour wrote:
Le 27/08/2013 11:43\, Tony Cook a �crit :
On Tue\, Aug 27\, 2013 at 10:30:21AM +0100\, Nicholas Clark wrote:
On Tue\, Aug 27\, 2013 at 11:20:56AM +0200\, Christian Millour wrote:
I believe this was diagnosed in #116118. To summarize\, the cause is probably the following line in Carp::format_args:
$arg =~
s/([[:cntrl:]]|[[:^ascii:]])/sprintf("\\x{%x}"\,ord($1))/eg;
I think that it may be possible to eliminate the calls to atoi() for getting and setting $1 etc\, and (effectively) move them to compile time in gv.c
Still\, shouldn’t we wrap atoi on non-compliant systems? Otherwise we will just keep finding this bug elsewhere (or not finding it and having things fail).
--
Father Chrysostomos
On Thu\, Aug 29\, 2013 at 9:02 AM\, Father Chrysostomos via RT \< perlbug-followup@perl.org> wrote:
Still\, shouldn’t we wrap atoi on non-compliant systems? Otherwise we will just keep finding this bug elsewhere (or not finding it and having things fail).
I agree. This is just waiting to bite us somewhere else.
Leon
Le 29/08/2013 11:29\, Leon Timmermans a écrit :
On Thu\, Aug 29\, 2013 at 9:02 AM\, Father Chrysostomos via RT \<perlbug-followup@perl.org \mailto​:perlbug\-followup@​perl\.org> wrote:
Still\, shouldn’t we wrap atoi on non\-compliant systems? Otherwise we will just keep finding this bug elsewhere \(or not finding it and having things fail\)\.
I agree. This is just waiting to bite us somewhere else.
Leon
Which leaves only the problem of how best to wrap it. The difficulty with the standard atoi is that it has no way to report error except by setting errno. Which means that a careful coder concerned with possible overflow should reset errno before calling atoi :
{ errno = 0; result = atoi(whatever); /* handle overflow if errno is nonzero */ }
So Windows is not so terribly wrong in bundling the reset of errno with the actual call itself.
If the same coder is intent on preserving the current errno (because she might be in the process of reporting it\, or for other reasons) she should probably use atoi as follows :
{ int olderr = errno; errno = 0; result = atoi(whatever); /* * at this point handle overflow if errno is nonzero * and if such handling is relevant */ errno = olderr; }
This mess is not Windows-specific BTW. So maybe the best solution for the wrapper would be to use a different signature :
int Perl_atoi(char const *s\, int * overflow_p) { int olderr\, res; olderr = errno; *overflow_p = 0; /* redundant on WIN 6+ */ res = atoi(s); *overflow_p = errno; errno = olderr; return res; }
possibly together with an optimized version for those users that do not care about overflow :
int Perl_atoi_ignore_overflow(char const *s) { int olderr\, res; olderr = errno; res = atoi(s); errno = olderr; return res; }
So that the responsibility to handle overflow would be put clearly in the hands of the caller.
This would allow Windows XS code to use the native atoi\, warts and all\, and spell out the decisions taken wrt overflow in core uses of atoi (e.g. in mg.c to parse numbered match vars and $&\, in regcomp.c to parse backrefs and quantifiers\, in malloc.c\, etc).
Of course this is pretty disgusting (and there might be a much better way to handle it) but the root objective\, which is giving the end-users a reliable access to errors so that thay can handle those\, is a very fundamental and worthy one. What we see here are unforeseen interactions between apparently unrelated components of Perl that conspire against this objective\, in part because those components make no effort to preserve $! (and $^E). They may be correct in not doing so individually\, for performance reasons : if one needs to use atoi (or any other op or sub) in a tight loop within which errors do not matter\, it is much more efficient to save/restore errno once ouside of the loop than inside it for each iteration. The problem with this approach is that all possible interactions need then to be considered when building an error reporting mechanism such as Carp. And this is quite hard to do. I can predict right now that if a future code path in Carp ever has to require a currently unloaded module\, and does not take precautions\, it will be screwed on the first invocation (because require clears $! -- together with doing unspeakable things to $^E -- on success).
It is all a matter of choosing the way we will get bitten in the future ;-)
On Thu\, Aug 29\, 2013 at 2:30 PM\, Christian Millour \cm\.perl@​abtela\.comwrote:
Which leaves only the problem of how best to wrap it. The difficulty with the standard atoi is that it has no way to report error except by setting errno. Which means that a careful coder concerned with possible overflow should reset errno before calling atoi :
{ errno = 0; result = atoi(whatever); /* handle overflow if errno is nonzero */ }
So Windows is not so terribly wrong in bundling the reset of errno with the actual call itself.
atoi() is not documented to use errno to indicate errors\, in fact "If the value cannot be represented\, the behavior is undefined." And even if it was\, "any function of the standard C library can modify its value to some value different from zero".
If the same coder is intent on preserving the current errno (because she might be in the process of reporting it\, or for other reasons) she should probably use atoi as follows :
{ int olderr = errno; errno = 0; result = atoi(whatever); /* * at this point handle overflow if errno is nonzero * and if such handling is relevant */ errno = olderr; }
That's a perfectly sensible approach using strtol and friends\, as they're actually documented to use errno.
Leon
Le 29/08/2013 15:03\, Leon Timmermans a écrit :
On Thu\, Aug 29\, 2013 at 2:30 PM\, Christian Millour \<cm.perl@abtela.com \mailto​:cm\.perl@​abtela\.com> wrote:
Which leaves only the problem of how best to wrap it\. The difficulty with the standard atoi is that it has no way to report error except by setting errno\. Which means that a careful coder concerned with possible overflow should reset errno before calling atoi : \{ errno = 0; result = atoi\(whatever\); /\* handle overflow if errno is nonzero \*/ \} So Windows is not so terribly wrong in bundling the reset of errno with the actual call itself\.
atoi() is not documented to use errno to indicate errors\, in fact "If the value cannot be represented\, the behavior is undefined." And even if it was\, "any function of the standard C library can modify its value to some value different from zero".
If the same coder is intent on preserving the current errno \(because she might be in the process of reporting it\, or for other reasons\) she should probably use atoi as follows : \{ int olderr = errno; errno = 0; result = atoi\(whatever\); /\* \* at this point handle overflow if errno is nonzero \* and if such handling is relevant \*/ errno = olderr; \}
That's a perfectly sensible approach using strtol and friends\, as they're actually documented to use errno.
Leon
I stand corrected\, thank you.
Makes me wonder though whether the use of atoi instead of strtol in core (mg.c\, regcomp.c\, malloc.c\, etc) is a simple historical oversight\, or whether it is really meant to state a complete disregard for out-of-range cases ?
Christian
Christian Millour wrote:
The problem
with this approach is that all possible interactions need then to be considered when building an error reporting mechanism such as Carp.
With $RefArgFormatter and CARP_TRACE\, Carp now explicitly invokes arbitrary code while constructing its error messages\, so wrapping atoi() calls is about as much use as applying a sticking plaster to a decapitation. Carp needs an explicit "local($!\, $^E);". Nothing short of that will suffice; and once it's got that it really doesn't matter what atoi gets up to.
-zefram
Le 29/08/2013 17:56\, Zefram a écrit :
Christian Millour wrote:
The problem
with this approach is that all possible interactions need then to be considered when building an error reporting mechanism such as Carp.
With $RefArgFormatter and CARP_TRACE\, Carp now explicitly invokes arbitrary code while constructing its error messages\, so wrapping atoi() calls is about as much use as applying a sticking plaster to a decapitation. Carp needs an explicit "local($!\, $^E);". Nothing short of that will suffice; and once it's got that it really doesn't matter what atoi gets up to.
-zefram
I agree fully. However what atoi gets up to might still matter in other contexts. All of the tests below\, unrelated to Carp\, currently fail on WIN6+ with Strawberry and ActiveState perls (can't check latest blead right now).
Taisha:/cygdrive/g/perls/blead/git/win32 $ cat ../../../myt/atoi_clobbering_errno.t use strict; use warnings; use Test::More tests => 7;
# from use of atoi in mg.c { local $!; my $x = q{x}; $x =~ m/(?\.)/; # set match vars
$! = 99; my $estr = "$!"; is "$1$!"\, "$x$estr"\, 'can use $1 and $! in same expression';
$! = 99; is "$&$!"\, "$x$estr"\, 'can use $& and $! in same expression';
$! = 99; my $a = $&; is 0+$!\, 99\, 'getting $& does not clobber $!';
$! = 99; $a = $1; is 0+$!\, 99\, 'getting $1 does not clobber $!';
$! = 99; eval { $1 = 0 }; # croaks\, but *after* atoi is called is 0+$!\, 99\, 'attempting to set $1 does not clobber $!'; }
# from use of atoi in regcomp.c { local $! = 99; my $res = q{(.)\1}; my $re = qr{$res}; is 0+$!\, 99\, 'dynamic qr// with backrefs does not clobber $!';
$! = 99; $res = q{.{0\,1}}; $re = qr{$res}; is 0+$!\, 99\, 'dynamic qr// with quantifiers does not clobber $!'; }
I wrote:
If this is to be a deliberate feature\, it should be documented\, it should be tested more thoroughly\, and it should be implemented explicitly by something like "local($!\, $^E);" in longmess and shortmess.
Done as cbd58baf5927dd469f38f80a7c76c8011150b6c5.
-zefram
On Thu Aug 29 14:04:13 2013\, zefram@fysh.org wrote:
I wrote:
If this is to be a deliberate feature\, it should be documented\, it should be tested more thoroughly\, and it should be implemented explicitly by something like "local($!\, $^E);" in longmess and shortmess.
Done as cbd58baf5927dd469f38f80a7c76c8011150b6c5.
Hence\, I am marking this as resolved\, since this ticket is about Carp specifically.
--
Father Chrysostomos
@cpansprout - Status changed from 'open' to 'resolved'
Migrated from rt.perl.org#81586 (status was 'resolved')
Searchable as RT81586$