Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

require returns 1 on invalid Perl file with local %INC #17428

Closed
rkleemann opened this issue Jan 17, 2020 · 12 comments · Fixed by #17479
Closed

require returns 1 on invalid Perl file with local %INC #17428

rkleemann opened this issue Jan 17, 2020 · 12 comments · Fixed by #17479

Comments

@rkleemann
Copy link

I've found an interesting situation where require doesn't do the right thing. I've tested this from Perl v5.12.5 through v5.30.0 with the same result. It only seems to occur when local %INC is used (even though it's preserving the state of %INC as it would if it were not used).

Digging into it, it appears that %INC is set correctly for the filename, it's set to undef.

#! /usr/bin/env perl

use strict;
use warnings;

use feature qw( say state );

sub _require;
sub _require {
    state $i = 0;
    local $_   = shift if @_;
    local %INC = %INC;          # With this line: "Return value is 1\n"
                                # Without:
                                # "9: Attempt to reload $_ aborted.\n"
                                # "9: Compilation failed in require at ...\n"

    if ( my $return = eval {require} ) {
        say "Return value is ", $return;
        return $return;
    } elsif ($@) {
        return unless ++$i < 10;
        foreach my $line ( split /\n/, $@ ) {
            say $i, ": ", $line;
        }
        _require;
    } else {
        say "False value from require, but \$@ is also false";
    }
}

$_ = @ARGV ? shift : "./t/Testing-failure.pm";
_require;

# t/Testing-failure.pm is below __END__

__END__
# An invalid Perl file
-

Flags:
category=core
severity=low

Site configuration information for perl 5.18.2:

Configured by root at Sun Feb 24 21:26:39 PST 2019.

Summary of my perl5 (revision 5 version 18 subversion 2) configuration:

Platform:
osname=darwin, osvers=17.0, archname=darwin-thread-multi-2level
uname='darwin osx217.apple.com 17.0 darwin kernel version 16.1.0: mon oct 9 13:05:29 pdt 2017; root:xnu-3789.21.4.1.7~1development_x86_64 x86_64 '
config_args='-ds -e -Dprefix=/usr -Dccflags=-g -pipe -Dldflags= -Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none -Dcc=cc'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=define, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-arch x86_64 -arch i386 -g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing -fstack-protector',
optimize='-Os',
cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing -fstack-protector'
ccversion='', gccversion='4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='cc', ldflags ='-arch x86_64 -arch i386 -fstack-protector'
libpth=/usr/lib /usr/local/lib
libs=
perllibs=
libc=, so=dylib, useshrplib=true, libperl=libperl.dylib
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags='-arch x86_64 -arch i386 -bundle -undefined dynamic_lookup -fstack-protector'

Locally applied patches:
/Library/Perl/Updates/ comes before system perl directories
installprivlib and installarchlib points to the Updates directory

@inc for perl 5.18.2:
/Users/bkleemann/lib/perl/darwin-thread-multi-2level
/Users/bkleemann/lib/perl/darwin-thread-multi-2level
/Users/bkleemann/lib/perl
/Library/Perl/5.18/darwin-thread-multi-2level
/Library/Perl/5.18
/Network/Library/Perl/5.18/darwin-thread-multi-2level
/Network/Library/Perl/5.18
/Library/Perl/Updates/5.18.2/darwin-thread-multi-2level
/Library/Perl/Updates/5.18.2
/System/Library/Perl/5.18/darwin-thread-multi-2level
/System/Library/Perl/5.18
/System/Library/Perl/Extras/5.18/darwin-thread-multi-2level
/System/Library/Perl/Extras/5.18
.

Environment for perl 5.18.2:
DYLD_LIBRARY_PATH (unset)
HOME=/Users/bkleemann
LANG=en_US.UTF-8
LANGUAGE (unset)
LC_TERMINAL=iTerm2
LC_TERMINAL_VERSION=3.3.7
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/usr/local/Cellar/plenv/2.3.0/libexec:/Users/bkleemann/.plenv/shims:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/bkleemann/bin:.
PERL5LIB=/Users/bkleemann/lib/perl/darwin-thread-multi-2level:/Users/bkleemann/lib/perl
PERL_BADLANG (unset)
SHELL=/bin/bash
@tonycoz
Copy link
Contributor

tonycoz commented Jan 20, 2020

When require fails on a file, its entry in %INC has its value set to &PL_sv_undef, not just an SV with the undef value, but the specific SV, and the code that checks for entries in %INC for require (near the start of S_require_file()) also checks for that specific SV.

When you do:

local %INC = %INC;

the undef for that key is copied to a new SV, so the entry for your file is no longer that special SV, but some generic undef SV and so the entry in %INC is treated as if a successful require has been done.

If I replace those checks with SvOK() instead of comparisons against &PL_sv_undef I get more reasonable results:

$ perl/perl -Iperl/lib 17428.pl
1: syntax error at ./test17428.pm line 1, at EOF
1: Compilation failed in require at 17428.pl line 17.
2: Attempt to reload ./test17428.pm aborted.
2: Compilation failed in require at 17428.pl line 17.
3: Attempt to reload ./test17428.pm aborted.
3: Compilation failed in require at 17428.pl line 17.
4: Attempt to reload ./test17428.pm aborted.
4: Compilation failed in require at 17428.pl line 17.
5: Attempt to reload ./test17428.pm aborted.
5: Compilation failed in require at 17428.pl line 17.
6: Attempt to reload ./test17428.pm aborted.
6: Compilation failed in require at 17428.pl line 17.
7: Attempt to reload ./test17428.pm aborted.
7: Compilation failed in require at 17428.pl line 17.
8: Attempt to reload ./test17428.pm aborted.
8: Compilation failed in require at 17428.pl line 17.
9: Attempt to reload ./test17428.pm aborted.
9: Compilation failed in require at 17428.pl line 17.

I'm not sure that just switching to bare SvOK() here is reasonable, if some user adds magic to %INC (which is about as reasonable as local %INC to me) then we also need to SvGETMAGIC().

Both strike me a bit as "Doctor, it hurts when I do this".

@demerphq
Copy link
Collaborator

demerphq commented Jan 20, 2020 via email

@rkleemann
Copy link
Author

Based on @tonycoz's comments, it appears that manually setting $INC{$_} = undef; would produce the Return value is 1 result. Checking this, that is correct, whether or not local is used. The same thing happens if I do the following:

# ... # Code that manipulates %INC
my %inc = %INC;
%INC = %inc;

But interestingly, this ends with 9: syntax error at ...: %INC =()= %INC;

As to why I'm using local %INC, I'm using this in some tests for my module, filename, which is essentially a compile-time require. At the moment, it appears that there is no way for my require to manipulate %INC in a manner consistent with CORE::require (but only for this case).

I agree with @demerphq, it appears that the magic should be removed from %INC, and it behaves in a manner in which seems consistent with any other hash.

@demerphq
Copy link
Collaborator

demerphq commented Jan 21, 2020 via email

tonycoz added a commit to tonycoz/perl5 that referenced this issue Jan 21, 2020
@tonycoz
test case for GH Perl#17428
0d3341f
tonycoz added a commit to tonycoz/perl5 that referenced this issue Jan 21, 2020
@tonycoz
always treat undef in %INC as a failed require
d7e134e 
Previously require would check for the specific \&PL_sv_undef
SV in %INC, this meant that if %INC was copied, or undef
assigned to a member the entry would erroneously be treated as if
a previous require of that file was successful.

So check for SvOK() instead, with appropriate magic tests.

fixes Perl#17428
@tonycoz tonycoz mentioned this issue Jan 21, 2020
Merged
@rkleemann
Copy link
Author

Try:

undef $INC{$_};

Instead.

That doesn't seem to work. I get "Return value is 1" response whether I do it with $INC{$_} = undef; or undef($INC{$_});. Are there any other ways I can get the right "magic" undef value without delving into the XS level?

tonycoz added a commit to tonycoz/perl5 that referenced this issue Jan 21, 2020
@tonycoz
test case for GH Perl#17428, an interaction between undef in %INC and…
4bd290e 
… require
tonycoz added a commit to tonycoz/perl5 that referenced this issue Jan 21, 2020
@tonycoz
always treat undef in %INC as a failed require
3faaec1 
Previously require would check for the specific \&PL_sv_undef
SV in %INC, this meant that if %INC was copied, or undef
assigned to a member the entry would erroneously be treated as if
a previous require of that file was successful.

So check for SvOK() instead, with appropriate magic tests.

fixes Perl#17428
@demerphq
Copy link
Collaborator

demerphq commented Jan 22, 2020 via email

rkleemann added a commit to rkleemann/filename that referenced this issue Jan 22, 2020
@rkleemann
Work around issue in CORE::require with Array::RefElem
cc8a1b6 
 * See Perl/perl5#17428
@rkleemann
Copy link
Author

my next thing to check would be various tricks with Array::RefElem. Maybe
something like hv_store(%INC, $_, undef)

Nothing else worked, but that did the trick. Thanks, I never would have figured that out on my own!

@demerphq
Copy link
Collaborator

demerphq commented Jan 23, 2020 via email

tonycoz added a commit that referenced this issue Jan 26, 2020
@tonycoz
test case for GH #17428, an interaction between undef in %INC and req…
50f6cde 
…uire
tonycoz added a commit that referenced this issue Jan 26, 2020
@tonycoz
always treat undef in %INC as a failed require
4b004c4 
Previously require would check for the specific \&PL_sv_undef
SV in %INC, this meant that if %INC was copied, or undef
assigned to a member the entry would erroneously be treated as if
a previous require of that file was successful.

So check for SvOK() instead, with appropriate magic tests.

fixes #17428
@KES777
Copy link
Contributor

KES777 commented Jun 22, 2020 •
edited
Loading

Hi. I am perlbrew user. I use perl 5.24.4, perl 5.18.4 and perl 5.30.3

I can switch between perls and run my project without problem. But when I install perl 5.32.0 and switch to it.
I get next error:

http://paste.scsys.co.uk/591331

$ make debug
PERLDB_OPTS="white_box" PERL5DB="use DB::Hooks qw'::Terminal NonStop'" perl -d $(which morbo) -w /home/kes/work/projects/tucha/monkeyman/public/maitre_d/api-v1.yaml \
-w lib/ -w templates/ /home/kes/work/projects/tucha/monkeyman/app/maitre_d/run.pl
Can't load '/home/kes/work/projects/tucha/monkeyman/local/lib/perl5/x86_64-linux/auto/List/Util/Util.so' for module List::Util: /home/kes/work/projects/tucha/monkeyman/local/lib/perl5/x86_64-linux/auto/List/Util/Util.so: undefined symbol: PL_sv_undef at /home/kes/perl5/perlbrew/perls/perl-5.32.0/lib/5.32.0/XSLoader.pm line 93.
 at /home/kes/work/projects/tucha/monkeyman/local/lib/perl5/x86_64-linux/List/Util.pm line 23.
Compilation failed in require at /home/kes/work/projects/tucha/monkeyman/local/lib/perl5/x86_64-linux/Scalar/Util.pm line 23.
Compilation failed in require at /home/kes/perl5/perlbrew/perls/perl-5.32.0/lib/site_perl/5.32.0/DB/Utils.pm line 223.
BEGIN failed--compilation aborted at /home/kes/perl5/perlbrew/perls/perl-5.32.0/lib/site_perl/5.32.0/DB/Utils.pm line 223.
Compilation failed in require at /home/kes/perl5/perlbrew/perls/perl-5.32.0/lib/site_perl/5.32.0/DB/Hooks.pm line 61.
BEGIN failed--compilation aborted at /home/kes/perl5/perlbrew/perls/perl-5.32.0/lib/site_perl/5.32.0/DB/Hooks.pm line 61.
Compilation failed in require.
BEGIN failed--compilation aborted.
Makefile:111: recipe for target 'debug' failed
make: *** [debug] Error 255

I do nothing special for 5.32.0. Same as for others perls:

  1. perlbrew install perl-XXX
  2. perlbrew switch perl-XXX
  3. cd <project>
  4. carton
  5. make debug

in change log I have found this:

Previously "require" in perlfunc would only treat the special built-in SV &PL_sv_undef as a value in %INC as if a previous require has failed, treating other undefined SVs as if the previous require has succeeded. This could cause unexpected success from require e.g., on local %INC = %INC;. This has been fixed. [GH #17428]

and come here.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 22, 2020 via email

@KES777
Copy link
Contributor

KES777 commented Jun 22, 2020

I just want to point that new 5.32.0 perl with just builded modules do not load them because of PL_sv_undef
The report is not specific to DB::Hooks or make debug.

And probably I found source of problem. Just give me some time. I rebuild perl and modules one time more.
I will answer as finish that

@KES777
Copy link
Contributor

KES777 commented Jun 22, 2020

The issue I was faces because perl version was harcoded inside local/bin/carton script.

#!/home/kes/perl5/perlbrew/perls/perl-5.24.1/bin/perl

It was run instead of perlbrew's version because of PATH

so despite on I run perl-5.32.0 modules was build by old perl version. sorry, for disturb.
at the moment of reporting PL_sv_undef was the only clue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

always treat undef in %INC as a failed require tonycoz/perl5
5 participants
@jkeenan @demerphq @tonycoz @KES777 @rkleemann