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.

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

BEGIN blocks have wrong caller package #15597

Open
p5pRT opened this issue Sep 10, 2016 · 10 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Sep 10, 2016

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

Searchable as RT129239$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 10, 2016

From @mauke

Created by @mauke

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
  printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.24.0:

Configured by mauke at Mon May  9 21:21:33 CEST 2016.

Summary of my perl5 (revision 5 version 24 subversion 0) configuration:
   
  Platform:
    osname=linux, osvers=4.4.5-1-arch, archname=i686-linux
    uname='linux simplicio 4.4.5-1-arch #1 smp preempt thu mar 10 07:54:30 cet 2016 i686 gnulinux '
    config_args=''
    hint=previous, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -flto',
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion='', gccversion='6.1.1 20160501', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12, longdblkind=3
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags ='-fstack-protector-strong -L/usr/local/lib -flto'
    libpth=/usr/local/lib /usr/lib/gcc/i686-pc-linux-gnu/6.1.1/include-fixed /usr/lib /lib /usr/local/lib /usr/lib/gcc/i686-pc-linux-gnu/6.1.1/include-fixed /usr/lib
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.23.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.23'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -flto -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.24.0:
    /home/mauke/usr/lib/perl5/site_perl/5.24.0/i686-linux
    /home/mauke/usr/lib/perl5/site_perl/5.24.0
    /home/mauke/usr/lib/perl5/5.24.0/i686-linux
    /home/mauke/usr/lib/perl5/5.24.0
    .


Environment for perl 5.24.0:
    HOME=/home/mauke
    LANG=en_US.UTF-8
    LANGUAGE=en_US
    LC_COLLATE=C
    LC_MONETARY=de_DE.UTF-8
    LC_TIME=de_DE.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/mauke/perl5/perlbrew/bin:/home/mauke/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
    PERLBREW_BASHRC_VERSION=0.73
    PERLBREW_HOME=/home/mauke/.perlbrew
    PERLBREW_ROOT=/home/mauke/perl5/perlbrew
    PERL_BADLANG (unset)
    PERL_UNICODE=SAL
    SHELL=/bin/bash

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 10, 2016

From @cpansprout

On Sat Sep 10 02​:13​:50 2016, mauke- wrote​:

This is a bug report for perl from l.mai@​web.de,
generated with the help of perlbug 1.40 running under perl 5.24.0.

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

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

I’m not entirely sure I would expect that. I’m not sure what to expect. After all, ‘calling’ BEGIN in this case has not started running yet.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 10, 2016

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 10, 2016

From @mauke

On Sat Sep 10 07​:31​:47 2016, sprout wrote​:

On Sat Sep 10 02​:13​:50 2016, mauke- wrote​:

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

I’m not entirely sure I would expect that. I’m not sure what to
expect. After all, ‘calling’ BEGIN in this case has not started
running yet.

I don't understand your last sentence but the reported location of the call (which is the "}" of the BEGIN block) is in package Mtfnpy, not main. At minimum it's inconsistent.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 12, 2016

From @iabyn

On Sat, Sep 10, 2016 at 07​:47​:42AM -0700, l.mai@​web.de via RT wrote​:

On Sat Sep 10 07​:31​:47 2016, sprout wrote​:

On Sat Sep 10 02​:13​:50 2016, mauke- wrote​:

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

I’m not entirely sure I would expect that. I’m not sure what to
expect. After all, ‘calling’ BEGIN in this case has not started
running yet.

I don't understand your last sentence but the reported location of the call (which is the "}" of the BEGIN block) is in package Mtfnpy, not main. At minimum it's inconsistent.

What's happening is as follows​:

At compile time, 'package Foo' sets PL_curstash to point to %Foo​::.
Then any subsequent COPs (i.e. OP_NEXTSTATE) after that have their
CopSTASH(cop) value set to the current value of PL_curstash.

At run time, each nextstate op that is executed, sets PL_curcop to point
to the currently executing COP.

When a scope (such as a sub) is entered, the current value of PL_curcop
is pushed onto the context stack.

caller() uses the saved PL_curcop's on the context stack to extract out
the stash name, line number etc.

When BEGIN is called we're not actually in runtime, so PL_curcop points to
&PL_compiling, rather than the last executed cop.

PL_compiling gets its cop_line etc fields updated every time the toker
reads in a new line (or sees #line), but a 'package' declaration doesn't
update CopSTASH(&PL_compiling), when perhaps it should??

--
In economics, the exam questions are the same every year.
They just change the answers.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 12, 2016

From @cpansprout

On Mon Sep 12 03​:57​:32 2016, davem wrote​:

PL_compiling gets its cop_line etc fields updated every time the toker
reads in a new line (or sees #line), but a 'package' declaration
doesn't
update CopSTASH(&PL_compiling), when perhaps it should??

PL_curstash is used at compile time instead of CopSTASH(&PL_compiling). I have always assumed it was for efficiency’s sake. But, come to think of it, if PL_compiling is a fixed buffer in the interpreter struct, then #defining PL_curstash to CopSTASH(&PL_compiling) would make no speed difference on threaded builds, but I don’t know about non-threaded. Then again, I may be wrong about the reason why PL_curstash is separate and CopSTASH(&PL_compiling) is unused.

Hmm, maybe it’s because cops do not contain stash pointers under threads. Maybe the solution is to have pp_caller check whether its cop == &PL_compiling and use PL_curstash instead.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 13, 2016

From @mauke

On Mon Sep 12 03​:57​:32 2016, davem wrote​:

On Sat, Sep 10, 2016 at 07​:47​:42AM -0700, l.mai@​web.de via RT wrote​:

On Sat Sep 10 07​:31​:47 2016, sprout wrote​:

On Sat Sep 10 02​:13​:50 2016, mauke- wrote​:

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

I’m not entirely sure I would expect that. I’m not sure what to
expect. After all, ‘calling’ BEGIN in this case has not started
running yet.

I don't understand your last sentence but the reported location of
the call (which is the "}" of the BEGIN block) is in package Mtfnpy,
not main. At minimum it's inconsistent.

What's happening is as follows​:

At compile time, 'package Foo' sets PL_curstash to point to %Foo​::.
Then any subsequent COPs (i.e. OP_NEXTSTATE) after that have their
CopSTASH(cop) value set to the current value of PL_curstash.

At run time, each nextstate op that is executed, sets PL_curcop to
point
to the currently executing COP.

When a scope (such as a sub) is entered, the current value of
PL_curcop
is pushed onto the context stack.

caller() uses the saved PL_curcop's on the context stack to extract
out
the stash name, line number etc.

When BEGIN is called we're not actually in runtime, so PL_curcop
points to
&PL_compiling, rather than the last executed cop.

PL_compiling gets its cop_line etc fields updated every time the toker
reads in a new line (or sees #line), but a 'package' declaration
doesn't
update CopSTASH(&PL_compiling), when perhaps it should??

Thanks for this detailed explanation! I discovered this issue because I have a C module that calls a Perl sub at parse time. That Perl sub always saw the wrong calling package and I didn't understand why. So I thought I'd check how perl itself handles that situation in BEGIN blocks (and maybe steal the code).

As it turned out, BEGIN blocks have exactly the same issue. Based on your explanation I've come up with the following workaround​:

  assert(PL_curcop == &PL_compiling);
  COP curcop_with_stash = PL_compiling;
  CopSTASH_set(&curcop_with_stash, PL_curstash);
  PL_curcop = &curcop_with_stash;
  n = call_sv(sv, G_SCALAR);
  PL_curcop = &PL_compiling;

It seems to pass all tests. :-)

(I don't know what the best way is to fix this in core.)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 21, 2016

From @mauke

On Sat Sep 10 02​:13​:50 2016, mauke- wrote​:

#!perl
use strict;
use warnings;

package Mtfnpy;
#line 100 "ABCDE"
BEGIN {
printf "package=%s, file=%s, line=%d\n", caller;
}

__END__

Output​:
package=main, file=ABCDE, line=102

Expected output​:
package=Mtfnpy, file=ABCDE, line=102

Here's a patch that adds a TODO test for now.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 21, 2016

From @mauke

0001-t-op-caller.t-add-a-TODO-test-for-RT-129239.patch
From bb3329a391fce27c00dd55aeaf5357337aa09192 Mon Sep 17 00:00:00 2001
From: Lukas Mai <l.mai@web.de>
Date: Fri, 21 Oct 2016 15:18:38 +0200
Subject: [PATCH] t/op/caller.t: add a TODO test for RT #129239

---
 t/op/caller.t | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/t/op/caller.t b/t/op/caller.t
index 3017465..1ffb5b3 100644
--- a/t/op/caller.t
+++ b/t/op/caller.t
@@ -5,7 +5,7 @@ BEGIN {
     chdir 't' if -d 't';
     require './test.pl';
     set_up_inc('../lib');
-    plan( tests => 97 ); # some tests are run in a BEGIN block
+    plan( tests => 100 ); # some tests are run in a BEGIN block
 }
 
 my @c;
@@ -352,3 +352,17 @@ EOP
 $::testing_caller = 1;
 
 do './op/caller.pl' or die $@;
+
+{
+    package RT129239;
+    BEGIN {
+        my ($pkg, $file, $line) = caller;
+        ::is $file, 'virtually/op/caller.t', "BEGIN block sees correct caller filename";
+        ::is $line, 12345,                   "BEGIN block sees correct caller line";
+        TODO: {
+            local $::TODO = "BEGIN blocks have wrong caller package [perl #129239]";
+            ::is $pkg, 'RT129239',               "BEGIN block sees correct caller package";
+        }
+#line 12345 "virtually/op/caller.t"
+    }
+}
-- 
2.10.0

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 2, 2017

From @KES777

This issue seems is related​: https://rt.perl.org/Public/Bug/Display.html?id=127083
to the current one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.