Bleadperl 5cf4b255 breaks VPIT/Variable-Magic-0.44.tar.gz

[p5pRT]

Migrated from rt.perl.org#78580 (status was 'resolved')

Searchable as RT78580$

[p5pRT]

From @andk

git bisect​:

  commit 5cf4b25
  Author​: Father Chrysostomos <sprout@​cpan.org>
  Date​: Mon Oct 25 10​:00​:47 2010 -0700

  [perl #77498] Assignment ignores magick when the RHS holds a glob

perl -V​:

  Summary of my perl5 (revision 5 version 13 subversion 6) configuration​:
  Commit id​: 5cf4b25
  Platform​:
  osname=linux, osvers=2.6.32-2-amd64, archname=x86_64-linux-thread-multi
  uname='linux k81 2.6.32-2-amd64 #1 smp fri feb 12 00​:01​:47 utc 2010 x86_64 gnulinux '
  config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.13.6-125-g5cf4b25 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Duseithreads -Uuselongdouble'
  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 ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.4', 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 =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=/lib/libc-2.11.2.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.11.2'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

  Characteristics of this binary (from libperl)​:
  Compile-time options​: MULTIPLICITY PERL_DONT_CREATE_GVSV
  PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP PERL_USE_DEVEL
  USE_64_BIT_ALL USE_64_BIT_INT USE_ITHREADS
  USE_LARGE_FILES USE_PERLIO USE_PERL_ATOF
  USE_REENTRANT_API
  Built under linux
  Compiled at Oct 26 2010 23​:10​:00
  @​INC​:
  /home/src/perl/repoperls/installed-perls/perl/v5.13.6-125-g5cf4b25/lib/site_perl/5.13.6/x86_64-linux-thread-multi
  /home/src/perl/repoperls/installed-perls/perl/v5.13.6-125-g5cf4b25/lib/site_perl/5.13.6
  /home/src/perl/repoperls/installed-perls/perl/v5.13.6-125-g5cf4b25/lib/5.13.6/x86_64-linux-thread-multi
  /home/src/perl/repoperls/installed-perls/perl/v5.13.6-125-g5cf4b25/lib/5.13.6
  .

--
andreas

[p5pRT]

From @cpansprout

On Tue Oct 26 14​:28​:16 2010, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect​:

commit 5cf4b255fd1d34956e94675ed15cb7a406e5b3e8
Author&#8203;: Father Chrysostomos \<sprout@&#8203;cpan\.org>
Date&#8203;:   Mon Oct 25 10&#8203;:00&#8203;:47 2010 \-0700

\[perl \#77498\] Assignment ignores magick when the RHS holds a glob

In perl 5.12.0, *{ ... } ignores magic on its argument if it is already
a glob or reference. I fixed that in 5.13.2 by calling SvGETMAGIC up
front. See <http​://perl5.git.perl.org/perl.git/commitdiff/bb1bc619>.

An unintended result is that a plain *a now calls get-magic on *a,
because it is a bit like *{ $​::{a} }. (Now I wonder why we do not just
use an OP_GV and why we need an OP_RV2GV as well, but I digress.) I
consider that a bug.

The change that broke Variable​::Magic fixes another bug​: $a = $b was
ignoring magic on $b if it was a glob.

The result was that VM’s test in t/34-glob.t that ‘local *b = *a’ called
magic once on *a failed, because it was called twice.

I tried modifying pp_rv2gv to skip magic on a glob without the FAKE flag
(which applies to *{ $​::{a} } and *a, but not *{\*a} or *{$b = *a}; see
attached), but other tests in t/34-glob.t fail.

Interestingly, those tests are effectively skipped for perls earlier
than 5.13.2. Variable​::Magic is explicitly testing for the bug I added
in bb1bc61.

So now what?

Should I apply this patch and let Vincent figure out what to do with VM?

[p5pRT]

From @cpansprout

Inline Patch

diff --git a/pp.c b/pp.c
index c99d697..76f353a 100644
--- a/pp.c
+++ b/pp.c
@@ -139,7 +139,7 @@ PP(pp_rv2gv)
 {
     dVAR; dSP; dTOPss;
 
-    SvGETMAGIC(sv);
+    if (!isGV(sv) || SvFAKE(sv)) SvGETMAGIC(sv);
     if (SvROK(sv)) {
       wasref:
 	tryAMAGICunDEREF(to_gv);
diff --git a/t/op/gmagic.t b/t/op/gmagic.t
index 65441a6..bc8a926 100644
--- a/t/op/gmagic.t
+++ b/t/op/gmagic.t
@@ -6,7 +6,7 @@ BEGIN {
     @INC = '../lib';
 }
 
-print "1..22\n";
+print "1..24\n";
 
 my $t = 1;
 tie my $c => 'Tie::Monitor';
@@ -60,6 +60,19 @@ $c = *strat;
 $s = $c;
 ok_string $s, *strat, 1, 1;
 
+# A plain *foo should not call get-magic on *foo.
+# This method of scalar-tying an immutable glob relies on details of the
+# current implementation that are subject to change. This test may need to
+# be rewritten if they do change.
+my $tyre = tie $::{gelp} => 'Tie::Monitor';
+# Compilation of this eval autovivifies the *gelp glob.
+eval '$tyre->init(0); () = \*gelp';
+my($rgot, $wgot) = $tyre->init(0);
+print "not " unless $rgot == 0;
+print "ok ", $t++, " - a plain *foo causes no get-magic\n";
+print "not " unless $wgot == 0;
+print "ok ", $t++, " - a plain *foo causes no set-magic\n";
+
 
 # adapted from Tie::Counter by Abigail
 package Tie::Monitor;

[p5pRT]

From [Unknown Contact. See original ticket]

On Tue Oct 26 14​:28​:16 2010, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect​:

commit 5cf4b255fd1d34956e94675ed15cb7a406e5b3e8
Author&#8203;: Father Chrysostomos \<sprout@&#8203;cpan\.org>
Date&#8203;:   Mon Oct 25 10&#8203;:00&#8203;:47 2010 \-0700

\[perl \#77498\] Assignment ignores magick when the RHS holds a glob

In perl 5.12.0, *{ ... } ignores magic on its argument if it is already
a glob or reference. I fixed that in 5.13.2 by calling SvGETMAGIC up
front. See <http​://perl5.git.perl.org/perl.git/commitdiff/bb1bc619>.

An unintended result is that a plain *a now calls get-magic on *a,
because it is a bit like *{ $​::{a} }. (Now I wonder why we do not just
use an OP_GV and why we need an OP_RV2GV as well, but I digress.) I
consider that a bug.

The change that broke Variable​::Magic fixes another bug​: $a = $b was
ignoring magic on $b if it was a glob.

The result was that VM’s test in t/34-glob.t that ‘local *b = *a’ called
magic once on *a failed, because it was called twice.

I tried modifying pp_rv2gv to skip magic on a glob without the FAKE flag
(which applies to *{ $​::{a} } and *a, but not *{\*a} or *{$b = *a}; see
attached), but other tests in t/34-glob.t fail.

Interestingly, those tests are effectively skipped for perls earlier
than 5.13.2. Variable​::Magic is explicitly testing for the bug I added
in bb1bc61.

So now what?

Should I apply this patch and let Vincent figure out what to do with VM?

[p5pRT]

@cpansprout - Status changed from 'new' to 'open'

[p5pRT]

From perl@profvince.com

On Tue Oct 26 14​:28​:16 2010, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect​:

commit 5cf4b255fd1d34956e94675ed15cb7a406e5b3e8
Author&#8203;: Father Chrysostomos \<sprout@&#8203;cpan\.org>
Date&#8203;:   Mon Oct 25 10&#8203;:00&#8203;:47 2010 \-0700

\[perl \#77498\] Assignment ignores magick when the RHS holds a glob

In perl 5.12.0, *{ ... } ignores magic on its argument if it is already
a glob or reference. I fixed that in 5.13.2 by calling SvGETMAGIC up
front. See <http​://perl5.git.perl.org/perl.git/commitdiff/bb1bc619>.

An unintended result is that a plain *a now calls get-magic on *a,
because it is a bit like *{ $​::{a} }. (Now I wonder why we do not just
use an OP_GV and why we need an OP_RV2GV as well, but I digress.) I
consider that a bug.

Why do you think that way?

The change that broke Variable​::Magic fixes another bug​: $a = $b was
ignoring magic on $b if it was a glob.

The result was that VM’s test in t/34-glob.t that ‘local *b = *a’ called
magic once on *a failed, because it was called twice.

I tried modifying pp_rv2gv to skip magic on a glob without the FAKE flag
(which applies to *{ $​::{a} } and *a, but not *{\*a} or *{$b = *a}; see
attached), but other tests in t/34-glob.t fail.

Interestingly, those tests are effectively skipped for perls earlier
than 5.13.2. Variable​::Magic is explicitly testing for the bug I added
in bb1bc61.

Not exactly. No tests are skipped in this file. The magical behaviour of
globs changed in 5.13.2, so I had to adapt the test.

So now what?

Should I apply this patch and let Vincent figure out what to do with VM?

get magic should be called either zero times (if what you describe is
really a bug) or one (if it's not), but not two (what it currently
does). Hence there's something to fix in the core.

Once this is sorted out, I'll adapt the test once again if necessary.

Vincent.

[p5pRT]

From @cpansprout

On Sun Oct 31 13​:20​:16 2010, perl@​profvince.com wrote​:

Father Chrysostomos wrote​:

In perl 5.12.0, *{ ... } ignores magic on its argument if it is
already
a glob or reference. I fixed that in 5.13.2 by calling SvGETMAGIC up
front. See <http​://perl5.git.perl.org/perl.git/commitdiff/bb1bc619>.

An unintended result is that a plain *a now calls get-magic on *a,
because it is a bit like *{ $​::{a} }. (Now I wonder why we do not
just
use an OP_GV and why we need an OP_RV2GV as well, but I digress.) I
consider that a bug.

Why do you think that way?

A simply $a does not call get-magic on $a, because it is just retrieving
the variable, not accessing its value.

Likewise, I think a simple *a should work the same way.

Another way of putting it​:

*a = *b should only call set-magic on *a, not get-magic as well.

The change that broke Variable​::Magic fixes another bug​: $a = $b was
ignoring magic on $b if it was a glob.

The result was that VM’s test in t/34-glob.t that ‘local *b = *a’
called
magic once on *a failed, because it was called twice.

I tried modifying pp_rv2gv to skip magic on a glob without the FAKE
flag
(which applies to *{ $​::{a} } and *a, but not *{\*a} or *{$b = *a};
see
attached), but other tests in t/34-glob.t fail.

Interestingly, those tests are effectively skipped for perls earlier
than 5.13.2. Variable​::Magic is explicitly testing for the bug I
added
in bb1bc61.

Not exactly. No tests are skipped in this file. The magical behaviour
of
globs changed in 5.13.2, so I had to adapt the test.

That’s why I said ‘effectively’. :-)

So now what?

Should I apply this patch and let Vincent figure out what to do with
VM?

get magic should be called either zero times (if what you describe is
really a bug) or one (if it's not), but not two (what it currently
does).

It’s called twice in that assignment (local *b = *a), because there are
two operators​: first the *{...} (*a) and then the ‘=’.

Because of the assignment, get-magic is called one more time in the
assignment than elsewhere.

Hence there's something to fix in the core.

Like making *a not count as a fetch? :-)

Once this is sorted out, I'll adapt the test once again if necessary.

Vincent.

[p5pRT]

From [Unknown Contact. See original ticket]

On Sun Oct 31 13​:20​:16 2010, perl@​profvince.com wrote​:

Father Chrysostomos wrote​:

In perl 5.12.0, *{ ... } ignores magic on its argument if it is
already
a glob or reference. I fixed that in 5.13.2 by calling SvGETMAGIC up
front. See <http​://perl5.git.perl.org/perl.git/commitdiff/bb1bc619>.

An unintended result is that a plain *a now calls get-magic on *a,
because it is a bit like *{ $​::{a} }. (Now I wonder why we do not
just
use an OP_GV and why we need an OP_RV2GV as well, but I digress.) I
consider that a bug.

Why do you think that way?

A simply $a does not call get-magic on $a, because it is just retrieving
the variable, not accessing its value.

Likewise, I think a simple *a should work the same way.

Another way of putting it​:

*a = *b should only call set-magic on *a, not get-magic as well.

The change that broke Variable​::Magic fixes another bug​: $a = $b was
ignoring magic on $b if it was a glob.

The result was that VM’s test in t/34-glob.t that ‘local *b = *a’
called
magic once on *a failed, because it was called twice.

I tried modifying pp_rv2gv to skip magic on a glob without the FAKE
flag
(which applies to *{ $​::{a} } and *a, but not *{\*a} or *{$b = *a};
see
attached), but other tests in t/34-glob.t fail.

Interestingly, those tests are effectively skipped for perls earlier
than 5.13.2. Variable​::Magic is explicitly testing for the bug I
added
in bb1bc61.

Not exactly. No tests are skipped in this file. The magical behaviour
of
globs changed in 5.13.2, so I had to adapt the test.

That’s why I said ‘effectively’. :-)

So now what?

Should I apply this patch and let Vincent figure out what to do with
VM?

get magic should be called either zero times (if what you describe is
really a bug) or one (if it's not), but not two (what it currently
does).

It’s called twice in that assignment (local *b = *a), because there are
two operators​: first the *{...} (*a) and then the ‘=’.

Because of the assignment, get-magic is called one more time in the
assignment than elsewhere.

Hence there's something to fix in the core.

Like making *a not count as a fetch? :-)

Once this is sorted out, I'll adapt the test once again if necessary.

Vincent.

[p5pRT]

From perl@profvince.com

A simply $a does not call get-magic on $a, because it is just retrieving
the variable, not accessing its value.

Likewise, I think a simple *a should work the same way.

Another way of putting it​:

*a = *b should only call set-magic on *a, not get-magic as well.

Fair enough.

Not exactly. No tests are skipped in this file. The magical behaviour
of
globs changed in 5.13.2, so I had to adapt the test.

That’s why I said ‘effectively’. :-)

What I meant was that it was still testing for 'set' magic. Also, V​::M
doesn't make any kind of guarantee about where the magic is called.
That's why it makes special cases testable by the end user through
compatibility constants.

It’s called twice in that assignment (local *b = *a), because there are
two operators​: first the *{...} (*a) and then the ‘=’.

Because of the assignment, get-magic is called one more time in the
assignment than elsewhere.

Hence there's something to fix in the core.

Like making *a not count as a fetch? :-)

Like that, yes. Whichever you prefer.

Vincent.

[p5pRT]

From @cpansprout

On Sun Oct 31 15​:22​:23 2010, perl@​profvince.com wrote​:

Like making *a not count as a fetch? :-)

Like that, yes. Whichever you prefer.

Now done as f64c9ac.

[p5pRT]

@cpansprout - Status changed from 'open' to 'resolved'

[p5pRT]

From @andk

On Sun, 31 Oct 2010 17​:28​:39 -0700, "Father Chrysostomos via RT" <perlbug-followup@​perl.org> said​:

  > On Sun Oct 31 15​:22​:23 2010, perl@​profvince.com wrote​:

Like making *a not count as a fetch? :-)

Like that, yes. Whichever you prefer.

  > Now done as f64c9ac.

Would you mind explaining what's left to do to close this ticket?
Variable​::Magic currently is one of the modules with the most
dependencies on CPAN, so I'd love to see it working again.

--
andreas

[p5pRT]

From @cpansprout

On Sun Nov 14 23​:28​:18 2010, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

On Sun, 31 Oct 2010 17​:28​:39 -0700, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> said​:

On Sun Oct 31 15​:22​:23 2010, perl@​profvince.com wrote​:

Like making *a not count as a fetch? :-)

Like that, yes. Whichever you prefer.

Now done as f64c9ac.

Would you mind explaining what's left to do to close this ticket?
Variable​::Magic currently is one of the modules with the most
dependencies on CPAN, so I'd love to see it working again.

Variable​::Magic’s tests need to be adjusted.

Whether the VMG_COMPAT_GLOB_GET constant should be removed, renamed or
left as it is, I don’t know.

As far as I understand it, it was supposed to indicate that any mention
of *a will call get-magic on *a. But that was a bug, which has been
fixed. It may be necessary to have a constant indicating that = calls
get-magic on its RHS, regardless of what it is (another fixed bug).