require should not clobber $! and $^E on success

[p5pRT]

Migrated from rt.perl.org#119555 (status was 'open')

Searchable as RT119555$

[p5pRT]

From cm.perl@abtela.com

Created by cm.perl@abtela.com

On success\, require resets $! and clobbers $^E (on platforms where $^E is distinct from $!\, such as Windows). It would be better to restore the previous values of $! and $^E.

In the process of foraging through @​INC\, require may try to open several files\, and when it succeeds on step N\, $! and $^E carry the failure codes for step N-1. Such failures are however part of the normal working of require. There is no reason to report them to the user. This consideration might have motivated the resetting of $! (although the exact ratinale is unclear\, see http​://www.nntp.perl.org/group/perl.perl5.porters/2012/12/msg197015.html). In any case $^E was left alone\, and for the sake of consistency we might consider resetting it also.

But resetting $! (and $^E) is a *bad thing* as it interferes with error reporting. A number of core and CPAN modules use the following idiom (or variants thereof) to implement 'lazy loading' of Carp :

sub croak {   require Carp;   goto &Carp​::croak; }

The problem here is that on first invocation (assuming Carp has not been loaded elsewhere)\, $! and $^E will have been corrupted to unusable values\, as the following demonstrates :

Taisha​:/cygdrive/d/perls/blead/perl-git/win32 $ cat e​:/cm/perl/tests/badcarp/require_clobbering_errno.pl use strict; use warnings;

sub croak {   require Carp;   goto &Carp​::croak; }

for (1 .. 3) {   eval {   rmdir 'notempty'   or croak '$! = '\, 0+$!\, '\, $^E = '\, 0+$^E\, "\n";   1;   } or do {   my @​msg = split /\n/\, $@​;   print '$@​ : "'\, $msg[0]\, '"'\, "\n";   print '$! : '\, 0+$! \, " ($!)\n";   print '$^E : '\, 0+$^E\, " ($^E)\n";   print "-----\n";   } } Taisha​:/cygdrive/d/perls/blead/perl-git/win32 $ ../perl e​:/cm/perl/tests/badcarp/require_clobbering_errno.pl $@​ : "$! = 41\, $^E = 145" $! : 0 () $^E : 6 (Descripteur non valide) ----- $@​ : "$! = 41\, $^E = 145" $! : 41 (Directory not empty) $^E : 145 (Le répertoire n'est pas vide) ----- $@​ : "$! = 41\, $^E = 145" $! : 41 (Directory not empty) $^E : 145 (Le répertoire n'est pas vide) ----- Taisha​:/cygdrive/d/perls/blead/perl-git/win32 $

Recent work on Carp (commit cbd58baf5927dd469f38f80a7c76c8011150b6c5) ensured that croak and confess hand out uncorrupted values of $! and $^E\, thus giving access to their numerical values for programmatic error recovery (see http​://www.nntp.perl.org/group/perl.perl5.porters/2013/08/msg206736.html and http​://www.nntp.perl.org/group/perl.perl5.porters/2012/12/msg197017.html for a rationale).

In line with this effort\, and considering the problem outlined above\, require should do the right thing and restore on success the values of $! and $^E as they were on invocation.

ALTERNATIVE

If there is a compelling reason to keep having require corrupt^Hreset $!\, then $^E should probably also be reset for consistency (and both resets should be documented). In addition\, the idiom above should be recast as

sub croak {   { local ($!\, $^E); require Carp }   goto &Carp​::croak; }

here is an indicative list of core files to patch : dist/SelfLoader/lib/SelfLoader.pm lib/POSIX.pm lib/open.pm lib/File/Compare.pm lib/File/Copy.pm lib/File/Path.pm lib/attributes.pm lib/_charnames.pm lib/Tie/Memoize.pm lib/SelfLoader.pm lib/Term/Cap.pm cpan/File-Path/lib/File/Path.pm cpan/Term-Cap/Cap.pm ext/attributes/attributes.pm ext/POSIX/lib/POSIX.pm ext/Tie-Memoize/lib/Tie/Memoize.pm

WORKAROUND

The problem above can be avoided by loading Carp as early as possible in the main script (which I have been doing for years\, after having been bitten by this exact bug).

Perl Info

``` Flags: category=core severity=low Site configuration information for perl 5.19.4: Configured by cm at Mon Sep 2 01:34:49 2013. Summary of my perl5 (revision 5 version 19 subversion 4) configuration: Commit id: cbd58baf5927dd469f38f80a7c76c8011150b6c5 Platform: osname=MSWin32, osvers=4.0, archname=MSWin32-x64-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -fno-strict-aliasing -mms-bitfields', optimize='-s -O2', cppflags='-DWIN32' ccversion='', gccversion='4.6.3', gccosandvers='' intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long long', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='g++', ldflags ='-s -L"d:\perls\blead\perl\lib\CORE" -L"C:\MinGW\lib"' libpth=C:\MinGW\lib libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 libc=, so=dll, useshrplib=true, libperl=libperl519.a gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-mdll -s -L"d:\perls\blead\perl\lib\CORE" -L"C:\MinGW\lib"' @INC for perl 5.19.4: d:/perls/blead/perl/site/lib d:/perls/blead/perl/lib . Environment for perl 5.19.4: CYGWIN=nodosfilewarning HOME=e:/cm LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;c:\strawberry-perl-5.16.1.1-64bit-portable\c\bin PERL_BADLANG (unset) SHELL (unset) ```

[p5pRT]

From cm.perl@abtela.com

Fist a big thanks to whoever took the pain to properly resend this ticket to p5p (it apparently got lost). For the sake of those following this on rt.perl.org\, hereafter is a copy of a clarification exchange I had with Zefram.

Le 02/09/2013 12​:55\, Zefram a écrit :

Christian Millour wrote​:

In line with this effort\, and considering the problem outlined above\, require should do the right thing and restore on success the values of $! and $^E as they were on invocation.

This argument can be used to justify saving and restoring $! and $^E around *any* operation. With Carp there was some historical justification that it had attempted to preserve $!\, and I was willing to make that work properly. require has never attempted to preserve $!\, and in fact the clearing suggests that it's actively attempting to signal its own status through $!\, which restoration would break.

-zefram

Damn. I have spent ages trying to try and craft this ticket so to avoid this misunderstanding and didn't succeed . Sorry if I was unclear. I am not advocating saving and restoring $! and $^E blindly within require\, but only when require is *successful*.

If the pseudocode in the pod is to be trusted\, require signals errors by *dying*\, and obviously in that case $! and $^E sould *not* be restored\, so that the calling code may access those and react accordingly (see http​://www.nntp.perl.org/group/perl.perl5.porters/2012/12/msg196988.html).

I am specifically *not* advocating using "local ($!\, $^E);" at the start of require\, but rather the following pseudocode

sub require {   my ($filename) = @​_;   my @​saveerrs = ($!\, $^E); # *added*   if (exists $INC{$filename}) {   return 1 if $INC{$filename};   die "Compilation failed in require";   }   my ($realfilename\,$result); ITER​: {   foreach $prefix (@​INC) {   $realfilename = "$prefix/$filename";   if (-f $realfilename) {   $INC{$filename} = $realfilename;   $result = do $realfilename;   last ITER;   }   }   die "Can't find $filename in \@​INC";   }   if ($@​) {   $INC{$filename} = undef;   die $@​;   } elsif (!$result) {   delete $INC{$filename};   die "$filename did not return true value";   } else {   ($!\, $^E) = @​saveerrs; # *added*   return $result;   } }

which restores $! and $^E on success\, and leaves them untouched for inspection on failure.

Le 02/09/2013 13​:40\, Zefram a écrit :

Christian Millour wrote​:

     I am not advocating saving and restoring $\! and $^E blindly

within require\, but only when require is *successful*.

Thanks for the clarification. I did miss that detail of your original message (which on rereading I can now see there). But it's still an awfully broadly applicable argument. What else should work to preserve $! on success? Or rather\, what should not?

-zefram

for those on p5p who wonder what the hell we are talking about\, and awaiting sync between perlbug and p5p\, the original report can be found at https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119555

To answer your question\, my main focus with #116118 has been on trying to ensure that croak/confess\, as an error reporting mechanism\, indeed report faithfully on the error rather than corrupting it (in the sense that $! and $^E are still fully available\, especially their numerical values\, when the error is caught).

What I am really after is the following strategy for programmatic error handling (see #116118 for the whole story) :

eval { some_sub_that_might_die_or_croak; 1 } or do {   my ($evalerr\, $e\, $se) = ($@​\, $!\, $^E);   if (ref $evalerr) {   ... # deal with exception object   } elsif (I_can_make_sense_of_this_error_string($evalerr)) {   ... # deal with error string   } else { # deal with numerical errors   if ($e == ENOENT) {   ...   } elsif {$e == EACCES) {   ...   etc.   } };

[the underlying issue being that it is often impossible to make sense of $evalerr above when you have to support different OS\, versions of perl\, and locales\, as the strings handed back by die/croak "oops​: $!\, $^E" are then essentially unpredictable]

Your "local ($!\, $^E);" patch on longmess/shortmess went a long way towards that goal. #119555 is intended as a complement\, to prevent the lurking bug currently inherent with the 'lazy loading of Carp' idiom.

One remaining problem is as follows​: Le 31/12/2012 18​:27\, Jan Dubois a écrit :

Even then you still have the problem that any DESTROY method run while unwinding the stack back to the eval() handler may modify $! and $^E again. So these values would also need to be handled similar to $@​.

Once this is solved I believe my concerns with error reporting will have been handled\, so the argument might be less broadly applicable as you think.

As a general rule\, I think ops and subs and constructs should not clobber (or reset) $! and $^E when successfull. But I also understand that such a requirement might be overkill and too costly to be applied bluntly. Still\, I hope to raise some awareness of the utility of not corrupting gratuitously $! and $^E.

In the specific case of require\, I believe that fixing it as suggested is easier and on the long term better than hunting for all instances of the 'lazy loading of Carp' idiom. YMMV.

Christian

[p5pRT]

From cm.perl@abtela.com

Le 02/09/2013 11​:28\, Christian Millour (via RT) a écrit :

----------------------------------------------------------------- [Please describe your issue here]

On success\, require resets $! and clobbers $^E (on platforms where $^E is distinct from $!\, such as Windows). It would be better to restore the previous values of $! and $^E.

Let me restate this\, and add some motivation. To recall\, the problem arises when you try to deal with the numerical values or $! and $^E for programmatic error handling\, rather than with their string counterpart.

Let us start with the baseline sample using die :

[cm@​COS63 ~/blead/perl-git]$ ./perl -E 'eval { $! = 21; die "fail​:$!"; 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 21

So far so good.

We are taught about using Carp​::croak instead of die when authoring modules. Thanks to some foresight from authors of Carp\, and later work by its maintainer (thanks\, Zefram :-) )\, Carp​::croak (and confess) do attempt to preserve $!\, so the error recovery strategy outlined above\, i.e. dealing with the numerical value of $!\, can be used :

[cm@​COS63 ~/blead/perl-git]$ ./perl -E 'eval { use Carp; $! = 21; croak "fail​:$!"; 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 21

what the croaking code cannot do however is 'autouse Carp'\, even though this is the very example provided in the perldoc for autouse :

[cm@​COS63 ~/blead/perl-git]$ ./perl -E 'eval { use autouse Carp => qw(croak); $! = 21; croak "fail​:$!"; 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 0

The problem arises through no fault of Carp or autouse\, but because require resets $! on success. This may be worked around by preloading Carp​:

[cm@​COS63 ~/blead/perl-git]$ ./perl -MCarp -E 'eval { use autouse Carp => qw(croak); $! = 21; croak "fail​:$!"; 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 21

but that pretty much defeats the purpose of autouse\, and makes for a nasty trap for the unwary. And any pre-autouse code that uses a similar mechanism\, as in

package Whatever; sub croak {   require Carp;   goto &Carp​::croak; }

suffers from the same problem.

Possibly more serious\, given e.g. how the latest edition of "Modern Perl" by chromatic embraces autodie (http​://www.modernperlbooks.com/mt/2012/10/why-the-modern-perl-book-uses-autodie.html)\, the croaking code cannot 'use autodie' either​:

[cm@​COS63 ~/blead/perl-git]$ ./perl -MCarp -E 'eval { use autodie qw(open); open (A\, ">"\, "."); 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 0

again through no fault of autodie\, but for the same reason as before (require resetting $! on success). This may currently be worked around by preloading autodie​::exception :

[cm@​COS63 ~/blead/perl-git]$ ./perl -Mautodie​::exception -E 'eval { use autodie qw(open); open (A\, ">"\, "."); 1 } or do { say "must handle numerical error "\, 0+$!}' must handle numerical error 21

but is fragile and IMHO raises obscurity to a new level of darkness :-/

Actually\, if the croaking code uses autodie\, then the client code may access the numerical value of $! from the autodie​::exception instance :

[cm@​COS63 ~/blead/perl-git]$ ./perl -E 'eval {use autodie qw(open); open (A\, ">"\, "."); 1 } or do { say "must handle numerical error "\, 0+$@​->errno}' must handle numerical error 21

unfortunately the doc for errno in autodie​::exception states that 'This method will leave the main autodie​::exception class' so it cannot be relied on. Furthermore\, autodie makes currently no provision for $^E.

On some systems $^E may be more specific than $!\, and deserves to be preserved as well. The examples above concentrated on $! for concision but could obviously have been written with $^E instead.

----------- SO ? --------------

I can't think of a compelling reason for require to reset $! (and corrupt $^E) when successful\, instead of restoring their values on entry. And indeed it seems to me that dSAVE_ERRNO\, SAVE_ERRNO and RESTORE_ERRNO exist in perl.h to deal with exactly such situations (and furthermore DTRT wrt. $^E). An analysis together with a tentative patch have been proposed here : https://rt.perl.org/Public/Bug/Display.html?id=116118#txn-1181050 \, and no objection were raised.

Could this tentative patch please be updated as needed and applied/smoked ? I can formalize a patch if needed but would rather have the original author (Craig) get the credit.

Thanks for your time and consideration.

Christian

[p5pRT]

From @craigberry

On Wed\, Jul 23\, 2014 at 4​:00 AM\, Christian Millour \cm\.perl@​abtela\.com wrote​:

I can't think of a compelling reason for require to reset $! (and corrupt $^E) when successful\, instead of restoring their values on entry. And indeed it seems to me that dSAVE_ERRNO\, SAVE_ERRNO and RESTORE_ERRNO exist in perl.h to deal with exactly such situations (and furthermore DTRT wrt. $^E). An analysis together with a tentative patch have been proposed here : https://rt.perl.org/Public/Bug/Display.html?id=116118#txn-1181050 \, and no objection were raised.

Could this tentative patch please be updated as needed and applied/smoked ? I can formalize a patch if needed but would rather have the original author (Craig) get the credit.

Thanks for your time and consideration.

The problem is my patch doesn't actually do what you want. It does successfully restore any errno that is set in pp_require while rifling through @​INC trying to find the file being required (assuming the file is eventually found and successfully opened).

But after it's opened\, the file is sent to S_doeval to be compiled\, and somewhere in there errno gets cleared again. I haven't figured out exactly how or where yet. There is a call to CLEAR_ERRSV()\, which has been there in some form since Perl 5.000. That might be what's clearing errno\, or not; I don't know. It doesn't look like it would\, but maybe there's magic on PL_errgv or something.

On the surface\, it sounds reasonable that a successful require should leave errno in the state it was in prior to the require. But nothing resembling a complete or well-though-out implementation has been proposed\, and any change in this area will involve touching sensitive bits of the core that have been as they are for twenty years. A reformulated patch is attached\, but at best it's a partial solution.

For future reference\, here's what you get with my patch​:

$ type foo.t @​INC = qw(Perl Rules ../lib);

$e = $! = 13; $se = $^E = 13; print '# $! is ' . (0+$!) . ' $^E is ' . (0+$^E) . "\n"; # Should look in 'Perl'\, then 'Rules' (unsuccessfully)\, then '../lib' require 'version.pm'; print '# $! is ' . (0+$!) . ' $^E is ' . (0+$^E) . "\n"; $ perl foo.t # $! is 13 $^E is 13 # $! is 0 $^E is 0

So you can see that errno is still cleared in require even though I restore it after finding version.pm.

[p5pRT]

From @craigberry

0001-Restore-errno-after-INC-search-in-pp_require.patch

```diff From 39094c4e6cf941efd0be9bc0d77eda99230dfd05 Mon Sep 17 00:00:00 2001 From: "Craig A. Berry" Date: Sat, 26 Jul 2014 12:44:42 -0500 Subject: [PATCH] Restore errno after @INC search in pp_require. This addresses [perl #119555]. It's quite natural when looking through @INC for a module that we don't always find it in the first place we look and errno will have been set by a failed filesystem lookup. Historically we've always cleared errno when the lookup is eventually successful so that we don't have a misleading value hanging around. This patch changes that behavior so we restore errno to what it was before starting the @INC search rather than clearing it. --- pp_ctl.c | 11 ++++++----- t/op/require_errors.t | 10 +++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index 7d098b7..887e05d 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3676,8 +3676,9 @@ PP(pp_require) SV *hook_sv = NULL; SV *encoding; OP *op; - int saved_errno; + int doopen_errno; bool path_searchable; + dSAVE_ERRNO; sv = POPs; if ( (SvNIOKp(sv) || SvVOK(sv)) && PL_op->op_type != OP_DOFILE) { @@ -4022,13 +4023,13 @@ PP(pp_require) } } } - saved_errno = errno; /* sv_2mortal can realloc things */ + doopen_errno = errno; /* sv_2mortal can realloc things */ sv_2mortal(namesv); if (!tryrsfp) { if (PL_op->op_type == OP_REQUIRE) { - if(saved_errno == EMFILE || saved_errno == EACCES) { + if(doopen_errno == EMFILE || doopen_errno == EACCES) { /* diag_listed_as: Can't locate %s */ - DIE(aTHX_ "Can't locate %s: %s", name, Strerror(saved_errno)); + DIE(aTHX_ "Can't locate %s: %s", name, Strerror(doopen_errno)); } else { if (namesv) { /* did we lookup @INC? */ AV * const ar = GvAVn(PL_incgv); @@ -4072,7 +4073,7 @@ PP(pp_require) RETPUSHUNDEF; } else - SETERRNO(0, SS_NORMAL); + RESTORE_ERRNO; /* Assume success here to prevent recursive requirement. */ /* name is never assigned to again, so len is still strlen(name) */ diff --git a/t/op/require_errors.t b/t/op/require_errors.t index a152d2d..09a7d67 100644 --- a/t/op/require_errors.t +++ b/t/op/require_errors.t @@ -7,7 +7,7 @@ BEGIN { require './test.pl'; } -plan(tests => 17); +plan(tests => 19); my $nonfile = tempfile(); @@ -133,3 +133,11 @@ like $@, qr/^Can't locate strict\.pm\\0invalid: /, 'do nul check'; eval "require strict\0::invalid;"; like $@, qr/^syntax error at \(eval \d+\) line 1/, 'parse error with \0 in barewords module names'; +{ + local ($!, $^E) = my ($e, $se) = (1, 1); + # Should look in 'Perl', then 'Rules' (unsuccessfully), then '../lib' + require 'version.pm'; + is (0+$! , $e , q{require does not obviously clobber $!}); + is (0+$^E, $se, q{require does not obviously clobber $^E}) + or diag ("BTW, " . (0+$^E) . " is '$^E'"); +} -- 1.8.4.2 ```

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From cm.perl@abtela.com

Le 28/07/2014 05​:01\, Craig A. Berry a écrit :

The problem is my patch doesn't actually do what you want. It does successfully restore any errno that is set in pp_require while rifling through @​INC trying to find the file being required (assuming the file is eventually found and successfully opened).

But after it's opened\, the file is sent to S_doeval to be compiled\, and somewhere in there errno gets cleared again. I haven't figured out exactly how or where yet. There is a call to CLEAR_ERRSV()\, which has been there in some form since Perl 5.000. That might be what's clearing errno\, or not; I don't know. It doesn't look like it would\, but maybe there's magic on PL_errgv or something.

On the surface\, it sounds reasonable that a successful require should leave errno in the state it was in prior to the require. But nothing resembling a complete or well-though-out implementation has been proposed\, and any change in this area will involve touching sensitive bits of the core that have been as they are for twenty years. A reformulated patch is attached\, but at best it's a partial solution.

Thank you for your time and efforts on this.

pp_require is indeed quite involved and I fully understand anyone being wary of touching it.

Still\, according to the documentation\, require dies on error\, and otherwise survives. So irrespective of the internal causes of changes to $! and $^E\, it should be OK to restore their values on entry in those cases where require does not die. In Perl :

sub new_require {   my ($e\, $se) = ($!\, $^E);   old_require $_[0]; # dies on error...   ($!\, $^E) = ($e\, $se); # ... and otherwise restore $! and $^E }

While not strictly equivalent\, and as an experiment\, I wrapped the existing pp_require\, renamed pp_require1 but otherwise untouched\, into a new pp_require tasked with saving/restoring $! and $^E :

PP(pp_require1) { ... existing code } PP(pp_require) {   OP *op;   dSAVE_ERRNO;   op = Perl_pp_require1( #ifndef PERL_IS_MINIPERL   my_perl #endif   );   if ((*_errno()) == 0)   RESTORE_ERRNO;   return op; }

However crude and ill-mannered\, this hack seemed to work. A variation on the above might be the best solution. A similar idea\, implemented in the attached patch\, is to dSAVE_ERRNO as you did\, but to RESTORE_ERRNO before any return point that is not a DIE. I find four of those : two RETPUSHYES\, one RETPUSHUNDEF\, and the final return. I can't see how those RESTORE_ERRNOs could damage the working of pp_require\, even with the recursive calls that seem to occur (via S_doeval). Note that the SETERRNO(0\, SS_NORMAL) introduced in \<http​://perl5.git.perl.org/perl.git/commit/d8bfb8bddf933a815b590823bd52295534e6ded0?f=pp_ctl.c> has been removed because 1) it seems unrelated to the internal mechanism of pp_require\, and 2) its effect would be nullified by the RESTORE_ERRNOs (at least on success).

This seems to work (one.pm is successfully required\, without clobbering $! and $^E) :

blead(w32) $ echo 0 > zero.pm; echo 1 > one.pm blead(w32) $ ../perl -IPerl -IRule -e 'for (@​ARGV) { $! = $^E = 1; eval {require "$_"; 1} or print $@​; print $_\,q{ ==> $! = }\, 0+$!\, q{\, $^E = }\, 0+$^E\, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm Can't locate doesnotexist.pm in @​INC (you may need to install the doesnotexist module) (@​INC contains​: Perl Rule D​:/perls/blead/perl-git2/lib .) at -e line 1. doesnotexist.pm ==> $! = 2\, $^E = 2 [Le fichier sp cifi est introuvable] zero.pm did not return a true value at -e line 1. zero.pm ==> $! = 1\, $^E = 1 [Fonction incorrecte] one.pm ==> $! = 1\, $^E = 1 [Fonction incorrecte] blead(w32) $

This is better than the stock perl (here strawberry perl portable 5.20)

Perl_5.20.0 $ echo 0 > zero.pm; echo 1 > one.pm Perl_5.20.0 $ perl -IPerl -IRule -e 'for (@​ARGV) { $! = $^E = 1; eval {require "$_"; 1} or print $@​; print $_\,q{ ==> $! = }\, 0+$!\, q{\, $^E = }\, 0+$^E\, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm Can't locate doesnotexist.pm in @​INC (you may need to install the doesnotexist module) (@​INC contains​: Perl Rule C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib/MSWin32-x86-multi-thread-64int C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/vendor/lib C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/lib .) at -e line 1. doesnotexist.pm ==> $! = 2\, $^E = 2 [Le fichier spécifié est introuvable] zero.pm did not return a true value at -e line 1. zero.pm ==> $! = 0\, $^E = 6 [Descripteur non valide] one.pm ==> $! = 0\, $^E = 6 [Descripteur non valide] Perl_5.20.0 $

In case of failure the values of $! and $^E do not seem much worse with the patch than without (given that for "did not return a true value" they are not set anyway\, so may be a bit random...).

Comments/critics/smokes warmly welcome :-) Best regards\,

Christian

[p5pRT]

From cm.perl@abtela.com

0001-Restore-errno-on-succesfull-require-199555.patch

```diff From e6eea131fb645accf4e2d8cea693ad1c7709fbe7 Mon Sep 17 00:00:00 2001 From: Christian Millour Date: Mon, 28 Jul 2014 23:34:22 +0200 Subject: [PATCH] Restore errno on succesfull require (#199555) Historically, require used to clear errno ($!) on success. While errno (and on some platforms $^E) might be set several times during a require (such as when searching through @INC and failing on the first directories), such temporary errors should not be visible to the user when require finally succeeds. Clearing errno is wrong however, as this produces pernicious action-at-a-distance effects (see #199555). Not doing anything, as was the case with $^E, was no better as it left $^E in a semi-random state. This patch addresses these issues by saving $! and $^E on entry to require, and restoring their values on successfull exit. --- pp_ctl.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index 7d098b7..9975be2 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3676,8 +3676,9 @@ PP(pp_require) SV *hook_sv = NULL; SV *encoding; OP *op; - int saved_errno; + int doopen_errno; bool path_searchable; + dSAVE_ERRNO; sv = POPs; if ( (SvNIOKp(sv) || SvVOK(sv)) && PL_op->op_type != OP_DOFILE) { @@ -3735,6 +3736,7 @@ PP(pp_require) } } + RESTORE_ERRNO; RETPUSHYES; } name = SvPV_const(sv, len); @@ -3777,9 +3779,10 @@ PP(pp_require) SV * const * const svp = hv_fetch(GvHVn(PL_incgv), unixname, unixlen, 0); if ( svp ) { - if (*svp != &PL_sv_undef) + if (*svp != &PL_sv_undef) { + RESTORE_ERRNO; RETPUSHYES; - else + } else DIE(aTHX_ "Attempt to reload %s aborted.\n" "Compilation failed in require", unixname); } @@ -4022,13 +4025,13 @@ PP(pp_require) } } } - saved_errno = errno; /* sv_2mortal can realloc things */ + doopen_errno = errno; /* sv_2mortal can realloc things */ sv_2mortal(namesv); if (!tryrsfp) { if (PL_op->op_type == OP_REQUIRE) { - if(saved_errno == EMFILE || saved_errno == EACCES) { + if(doopen_errno == EMFILE || doopen_errno == EACCES) { /* diag_listed_as: Can't locate %s */ - DIE(aTHX_ "Can't locate %s: %s", name, Strerror(saved_errno)); + DIE(aTHX_ "Can't locate %s: %s", name, Strerror(doopen_errno)); } else { if (namesv) { /* did we lookup @INC? */ AV * const ar = GvAVn(PL_incgv); @@ -4069,10 +4072,9 @@ PP(pp_require) } CLEAR_ERRSV(); + RESTORE_ERRNO; RETPUSHUNDEF; } - else - SETERRNO(0, SS_NORMAL); /* Assume success here to prevent recursive requirement. */ /* name is never assigned to again, so len is still strlen(name) */ @@ -4131,6 +4133,7 @@ PP(pp_require) LOADED_FILE_PROBE(unixname); + RESTORE_ERRNO; return op; } -- 1.7.4 ```

[p5pRT]

From cm.perl@abtela.com

minor edit on the patch file to reference the proper RT ticket (119... instead of 199...). Sorry for the noise.

Christian

Le 29/07/2014 00​:46\, Christian Millour a écrit :

Le 28/07/2014 05​:01\, Craig A. Berry a écrit :

The problem is my patch doesn't actually do what you want. It does successfully restore any errno that is set in pp_require while rifling through @​INC trying to find the file being required (assuming the file is eventually found and successfully opened).

But after it's opened\, the file is sent to S_doeval to be compiled\, and somewhere in there errno gets cleared again. I haven't figured out exactly how or where yet. There is a call to CLEAR_ERRSV()\, which has been there in some form since Perl 5.000. That might be what's clearing errno\, or not; I don't know. It doesn't look like it would\, but maybe there's magic on PL_errgv or something.

On the surface\, it sounds reasonable that a successful require should leave errno in the state it was in prior to the require. But nothing resembling a complete or well-though-out implementation has been proposed\, and any change in this area will involve touching sensitive bits of the core that have been as they are for twenty years. A reformulated patch is attached\, but at best it's a partial solution.

Thank you for your time and efforts on this.

pp_require is indeed quite involved and I fully understand anyone being wary of touching it.

Still\, according to the documentation\, require dies on error\, and otherwise survives. So irrespective of the internal causes of changes to $! and $^E\, it should be OK to restore their values on entry in those cases where require does not die. In Perl :

sub new_require { my ($e\, $se) = ($!\, $^E); old_require $_[0]; # dies on error... ($!\, $^E) = ($e\, $se); # ... and otherwise restore $! and $^E }

While not strictly equivalent\, and as an experiment\, I wrapped the existing pp_require\, renamed pp_require1 but otherwise untouched\, into a new pp_require tasked with saving/restoring $! and $^E :

PP(pp_require1) { ... existing code } PP(pp_require) { OP *op; dSAVE_ERRNO; op = Perl_pp_require1( #ifndef PERL_IS_MINIPERL my_perl #endif ); if ((*_errno()) == 0) RESTORE_ERRNO; return op; }

However crude and ill-mannered\, this hack seemed to work. A variation on the above might be the best solution. A similar idea\, implemented in the attached patch\, is to dSAVE_ERRNO as you did\, but to RESTORE_ERRNO before any return point that is not a DIE. I find four of those : two RETPUSHYES\, one RETPUSHUNDEF\, and the final return. I can't see how those RESTORE_ERRNOs could damage the working of pp_require\, even with the recursive calls that seem to occur (via S_doeval). Note that the SETERRNO(0\, SS_NORMAL) introduced in \<http​://perl5.git.perl.org/perl.git/commit/d8bfb8bddf933a815b590823bd52295534e6ded0?f=pp_ctl.c> has been removed because 1) it seems unrelated to the internal mechanism of pp_require\, and 2) its effect would be nullified by the RESTORE_ERRNOs (at least on success).

This seems to work (one.pm is successfully required\, without clobbering $! and $^E) :

blead(w32) $ echo 0 > zero.pm; echo 1 > one.pm blead(w32) $ ../perl -IPerl -IRule -e 'for (@​ARGV) { $! = $^E = 1; eval {require "$_"; 1} or print $@​; print $_\,q{ ==> $! = }\, 0+$!\, q{\, $^E = }\, 0+$^E\, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm Can't locate doesnotexist.pm in @​INC (you may need to install the doesnotexist module) (@​INC contains​: Perl Rule D​:/perls/blead/perl-git2/lib .) at -e line 1. doesnotexist.pm ==> $! = 2\, $^E = 2 [Le fichier sp cifi est introuvable] zero.pm did not return a true value at -e line 1. zero.pm ==> $! = 1\, $^E = 1 [Fonction incorrecte] one.pm ==> $! = 1\, $^E = 1 [Fonction incorrecte] blead(w32) $

This is better than the stock perl (here strawberry perl portable 5.20)

Perl_5.20.0 $ echo 0 > zero.pm; echo 1 > one.pm Perl_5.20.0 $ perl -IPerl -IRule -e 'for (@​ARGV) { $! = $^E = 1; eval {require "$_"; 1} or print $@​; print $_\,q{ ==> $! = }\, 0+$!\, q{\, $^E = }\, 0+$^E\, qq{ [$^E]\n}}' doesnotexist.pm zero.pm one.pm Can't locate doesnotexist.pm in @​INC (you may need to install the doesnotexist module) (@​INC contains​: Perl Rule C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib/MSWin32-x86-multi-thread-64int C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/site/lib C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/vendor/lib C​:/strawberry-perl-5.20.0.1-32bit-portable-PDL/perl/lib .) at -e line 1. doesnotexist.pm ==> $! = 2\, $^E = 2 [Le fichier spécifié est introuvable] zero.pm did not return a true value at -e line 1. zero.pm ==> $! = 0\, $^E = 6 [Descripteur non valide] one.pm ==> $! = 0\, $^E = 6 [Descripteur non valide] Perl_5.20.0 $

In case of failure the values of $! and $^E do not seem much worse with the patch than without (given that for "did not return a true value" they are not set anyway\, so may be a bit random...).

Comments/critics/smokes warmly welcome :-) Best regards\,

Christian

[p5pRT]

From cm.perl@abtela.com

0001-Restore-errno-on-succesfull-require-119555.patch

```diff From ef0be7397327c777298a079d398b86062869b1b6 Mon Sep 17 00:00:00 2001 From: Christian Millour Date: Mon, 28 Jul 2014 23:34:22 +0200 Subject: [PATCH] Restore errno on succesfull require (#119555) Historically, require used to clear errno ($!) on success. While errno (and on some platforms $^E) might be set several times during a require (such as when searching through @INC and failing on the first directories), such temporary errors should not be visible to the user when require finally succeeds. Clearing errno is wrong however, as this produces pernicious action-at-a-distance effects (see #119555). Not doing anything, as was the case with $^E, was no better as it left $^E in a semi-random state. This patch addresses these issues by saving $! and $^E on entry to require, and restoring their values on successfull exit. --- pp_ctl.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index 7d098b7..9975be2 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3676,8 +3676,9 @@ PP(pp_require) SV *hook_sv = NULL; SV *encoding; OP *op; - int saved_errno; + int doopen_errno; bool path_searchable; + dSAVE_ERRNO; sv = POPs; if ( (SvNIOKp(sv) || SvVOK(sv)) && PL_op->op_type != OP_DOFILE) { @@ -3735,6 +3736,7 @@ PP(pp_require) } } + RESTORE_ERRNO; RETPUSHYES; } name = SvPV_const(sv, len); @@ -3777,9 +3779,10 @@ PP(pp_require) SV * const * const svp = hv_fetch(GvHVn(PL_incgv), unixname, unixlen, 0); if ( svp ) { - if (*svp != &PL_sv_undef) + if (*svp != &PL_sv_undef) { + RESTORE_ERRNO; RETPUSHYES; - else + } else DIE(aTHX_ "Attempt to reload %s aborted.\n" "Compilation failed in require", unixname); } @@ -4022,13 +4025,13 @@ PP(pp_require) } } } - saved_errno = errno; /* sv_2mortal can realloc things */ + doopen_errno = errno; /* sv_2mortal can realloc things */ sv_2mortal(namesv); if (!tryrsfp) { if (PL_op->op_type == OP_REQUIRE) { - if(saved_errno == EMFILE || saved_errno == EACCES) { + if(doopen_errno == EMFILE || doopen_errno == EACCES) { /* diag_listed_as: Can't locate %s */ - DIE(aTHX_ "Can't locate %s: %s", name, Strerror(saved_errno)); + DIE(aTHX_ "Can't locate %s: %s", name, Strerror(doopen_errno)); } else { if (namesv) { /* did we lookup @INC? */ AV * const ar = GvAVn(PL_incgv); @@ -4069,10 +4072,9 @@ PP(pp_require) } CLEAR_ERRSV(); + RESTORE_ERRNO; RETPUSHUNDEF; } - else - SETERRNO(0, SS_NORMAL); /* Assume success here to prevent recursive requirement. */ /* name is never assigned to again, so len is still strlen(name) */ @@ -4131,6 +4133,7 @@ PP(pp_require) LOADED_FILE_PROBE(unixname); + RESTORE_ERRNO; return op; } -- 1.7.4 ```

[toddr]

This patch no longer applies. Does anyone else have an idea or do we call this a "feature" of Windows?