Skip Menu |
Report information
Id: 128989
Status: rejected
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: andreas.koenig.7os6VVqR [at] franz.ak.mind.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: notabug
Perl Version: (no value)
Fixed In: (no value)



Date: Fri, 19 Aug 2016 00:22:32 +0200
To: perlbug [...] perl.org
Subject: Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
From: Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>
Download (untitled) / with headers
text/plain 3.6k
bisect ------ commit 1d7e6444bd515142acf34ef230b50f9d80ab9017 Author: David Mitchell <davem@iabyn.com> Date: Wed Aug 17 09:52:48 2016 +0100 av_fetch(): use AvFILLp rather than AvFILL diagnostics ----------- http://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c perl -V ------- Summary of my perl5 (revision 5 version 25 subversion 4) configuration: Commit id: cc890588b0ce25c7e37126933582b6d944ad1584 Platform: osname=linux osvers=4.3.0-1-amd64 archname=x86_64-linux-ld uname='linux k83 4.3.0-1-amd64 #1 smp debian 4.3.3-7 (2016-01-19) x86_64 gnulinux ' config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942 -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Uuseithreads -Duselongdouble -DDEBUGGING=-g' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=define 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 -D_FORTIFY_SOURCE=2' optimize='-O2 -g' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='5.3.1 20160121' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='long double' nvsize=16 Off_t='off_t' lseeksize=8 alignbytes=16 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lpthread -lnsl -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.21.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.21' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP PERL_OP_PARENT PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_LONG_DOUBLE USE_PERLIO USE_PERL_ATOF Built under linux Compiled at Aug 17 2016 15:33:39 %ENV: PERL5LIB="" PERL5OPT="" PERL5_CPANPLUS_IS_RUNNING="30984" PERL5_CPAN_IS_RUNNING="30984" PERL_CANARY_STABILITY_NOPROMPT="1" PERL_MM_USE_DEFAULT="1" @INC: /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/site_perl/5.25.4/x86_64-linux-ld /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/site_perl/5.25.4 /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/5.25.4/x86_64-linux-ld /home/sand/src/perl/repoperls/installed-perls/perl/v5.25.3-277-gcc89058/8942/lib/5.25.4 . -- andreas
Date: Thu, 18 Aug 2016 19:42:25 -0300
From: "Vincent Pit (VPIT)" <perl [...] profvince.com>
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
To: perl5-porters [...] perl.org
Le 18/08/2016 à 19:22, (Andreas J. Koenig) (via RT) a écrit : Show quoted text
> # New Ticket Created by (Andreas J. Koenig) > # Please include the string: [perl #128989] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=128989 > > > > bisect > ------ > commit 1d7e6444bd515142acf34ef230b50f9d80ab9017 > Author: David Mitchell <davem@iabyn.com> > Date: Wed Aug 17 09:52:48 2016 +0100 > > av_fetch(): use AvFILLp rather than AvFILL > > diagnostics > ----------- > http://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c >
Thanks for the bisect. I had seen the report, but I did not know exactly which change caused the failure. Now I'm pretty sure the failing test is the one that checks whether len magic triggers for "$a[-1] = 2". I'm too busy right now to check this and cut out a release, but I'll try to find some time for that Monday. Maybe it should be documented somewhere that len magic is practically useless now from an user perspective. Vincent
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
To: "Vincent Pit (VPIT)" <perl [...] profvince.com>
Date: Fri, 19 Aug 2016 12:12:14 +0100
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 1.9k
On Thu, Aug 18, 2016 at 07:42:25PM -0300, Vincent Pit (VPIT) wrote: Show quoted text
> > > Le 18/08/2016 à 19:22, (Andreas J. Koenig) (via RT) a écrit :
> > # New Ticket Created by (Andreas J. Koenig) > > # Please include the string: [perl #128989] > > # in the subject line of all future correspondence about this issue. > > # <URL: https://rt.perl.org/Ticket/Display.html?id=128989 > > > > > > > bisect > > ------ > > commit 1d7e6444bd515142acf34ef230b50f9d80ab9017 > > Author: David Mitchell <davem@iabyn.com> > > Date: Wed Aug 17 09:52:48 2016 +0100 > > > > av_fetch(): use AvFILLp rather than AvFILL > > > > diagnostics > > ----------- > > http://www.cpantesters.org/cpan/report/4145b580-64b0-11e6-a8a1-60d858b9f28c > >
> > Thanks for the bisect. I had seen the report, but I did not know exactly > which change caused the failure. Now I'm pretty sure the failing test is the > one that checks whether len magic triggers for "$a[-1] = 2". > > I'm too busy right now to check this and cut out a release, but I'll try to > find some time for that Monday. > > Maybe it should be documented somewhere that len magic is practically > useless now from an user perspective.
That particular commit changed the code path in av_fetch() which handles negative indices to match the path for +ve indices. So if this commit is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of av_fetch() has been using AvFILLp() rather than AvFILL() back to at least perl5.005_03. av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find tied magic. Perhaps the bug here is that av_fetch() should handle the combination of RMG but not tied specially too? But this combination doesn't seem to make much sense: it says we have to call mg_size() to find the size of the array (in order to check for key being out of bounds), but we then ignore any magic to actually retrieve the element from AvARRAY)av)[key]. -- O Unicef Clearasil! Gibberish and Drivel! -- "Bored of the Rings"
Date: Mon, 22 Aug 2016 09:41:43 +0100
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
To: "Vincent Pit (VPIT)" <perl [...] profvince.com>
Download (untitled) / with headers
text/plain 1.8k
On Fri, Aug 19, 2016 at 12:12:14PM +0100, Dave Mitchell wrote: Show quoted text
> On Thu, Aug 18, 2016 at 07:42:25PM -0300, Vincent Pit (VPIT) wrote:
> > Thanks for the bisect. I had seen the report, but I did not know exactly > > which change caused the failure. Now I'm pretty sure the failing test is the > > one that checks whether len magic triggers for "$a[-1] = 2". > > > > I'm too busy right now to check this and cut out a release, but I'll try to > > find some time for that Monday. > > > > Maybe it should be documented somewhere that len magic is practically > > useless now from an user perspective.
> > That particular commit changed the code path in av_fetch() which handles > negative indices to match the path for +ve indices. So if this commit > is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of > av_fetch() has been using AvFILLp() rather than AvFILL() back to at least > perl5.005_03. > > av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find > tied magic. Perhaps the bug here is that av_fetch() should handle the > combination of RMG but not tied specially too? > But this combination doesn't seem to make much sense: it says we > have to call mg_size() to find the size of the array (in order to check for > key being out of bounds), but we then ignore any magic to actually retrieve > the element from AvARRAY)av)[key].
Indeed, changing $val[-1] to $val[2] in t/22-len.t (which should be equivalent for a 3-element array) makes the test fail with all perls back to 5.8.0. So len magic on arrays has never worked for Variable::Magic, except for negative indices. I'd suggest that Variable::Magic is updated to reflect this reality, rather than making blead support it - unless a strong case can be made otherwise. -- It's not that I'm afraid to die, I just don't want to be there when it happens. -- Woody Allen
Date: Wed, 24 Aug 2016 14:08:14 +0100
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
To: "Vincent Pit (VPIT)" <perl [...] profvince.com>
Download (untitled) / with headers
text/plain 3.9k
On Mon, Aug 22, 2016 at 09:41:43AM +0100, Dave Mitchell wrote: Show quoted text
> On Fri, Aug 19, 2016 at 12:12:14PM +0100, Dave Mitchell wrote:
> > On Thu, Aug 18, 2016 at 07:42:25PM -0300, Vincent Pit (VPIT) wrote:
> > > Thanks for the bisect. I had seen the report, but I did not know exactly > > > which change caused the failure. Now I'm pretty sure the failing test is the > > > one that checks whether len magic triggers for "$a[-1] = 2". > > > > > > I'm too busy right now to check this and cut out a release, but I'll try to > > > find some time for that Monday. > > > > > > Maybe it should be documented somewhere that len magic is practically > > > useless now from an user perspective.
> > > > That particular commit changed the code path in av_fetch() which handles > > negative indices to match the path for +ve indices. So if this commit > > is wrong, the +ve path has been wrong too. AFAIKT, the +ve branch of > > av_fetch() has been using AvFILLp() rather than AvFILL() back to at least > > perl5.005_03. > > > > av_fetch() uses AvFILLp() unless both the RMG flag is set and it can find > > tied magic. Perhaps the bug here is that av_fetch() should handle the > > combination of RMG but not tied specially too? > > But this combination doesn't seem to make much sense: it says we > > have to call mg_size() to find the size of the array (in order to check for > > key being out of bounds), but we then ignore any magic to actually retrieve > > the element from AvARRAY)av)[key].
> > Indeed, changing $val[-1] to $val[2] in t/22-len.t (which should be > equivalent for a 3-element array) makes the test fail with all perls back > to 5.8.0. > > So len magic on arrays has never worked for Variable::Magic, except for > negative indices. > > I'd suggest that Variable::Magic is updated to reflect this reality, > rather than making blead support it - unless a strong case can be made > otherwise.
Interestingly, if I change av_fetch() so that it does AvFILL() (rather than AvFILLp()) for positive indexes, then a bunch of tests in 31-array.t fail; e.g.: # Failed test 'array: assign element to triggers magic correctly' # at t/31-array.t line 27. # Structures begin differing at: # $got->{len} = '1' # $expected->{len} = Does not exist I strongly suspect that that the correct fix is to update V::M's tests and docs to match the new reality in perl. But in the meantime, I've temporarily reverted to the old behaviour with the following commit. Note that that 5.25.4 has just been released with the behaviour that causes t/22-len.t to fail. commit 8d168aaa014262c7f93944b76b84de99af3c5513 Author: David Mitchell <davem@iabyn.com> AuthorDate: Wed Aug 24 13:57:56 2016 +0100 Commit: David Mitchell <davem@iabyn.com> CommitDate: Wed Aug 24 13:57:56 2016 +0100 tmp fix for Bleadperl breaks Variable-Magic RT #128989 Prior to my commit v5.25.3-266-g1d7e644, in the absence of the SVs_RMG flag, av_fetch() used AvFILL() for -ve keys and AvFILLp() for positive keys. That commit changed it so they both use AvFILLp. This has broken Variable::Magic 0.59. As an interim measure, restore the old behaviour. Affected files ... M av.c Differences ... diff --git a/av.c b/av.c index 21828a9..e3c6d5a 100644 --- a/av.c +++ b/av.c @@ -272,7 +272,8 @@ Perl_av_fetch(pTHX_ AV *av, SSize_t key, I32 lval) } neg = (key < 0); - size = AvFILLp(av) + 1; + /* XXX tmp restore old behaviour to make Variable::Magic pass */ + size = (neg ? AvFILL(av): AvFILLp(av)) + 1; key += neg * size; /* handle negative index without using branch */ /* the cast from SSize_t to Size_t allows both (key < 0) and (key >= size) -- "But Sidley Park is already a picture, and a most amiable picture too. The slopes are green and gentle. The trees are companionably grouped at intervals that show them to advantage. The rill is a serpentine ribbon unwound from the lake peaceably contained by meadows on which the right amount of sheep are tastefully arranged." -- Lady Croom, "Arcadia"
To: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
CC: perl5-porters [...] perl.org
From: "Vincent Pit (VPIT)" <perl [...] profvince.com>
Date: Tue, 6 Sep 2016 19:36:20 -0300
Download (untitled) / with headers
text/plain 1.2k
Show quoted text
> > commit 8d168aaa014262c7f93944b76b84de99af3c5513 > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Wed Aug 24 13:57:56 2016 +0100 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Wed Aug 24 13:57:56 2016 +0100 > > tmp fix for Bleadperl breaks Variable-Magic > > RT #128989 > > Prior to my commit v5.25.3-266-g1d7e644, in the absence of the SVs_RMG > flag, av_fetch() used AvFILL() for -ve keys and AvFILLp() for positive > keys. That commit changed it so they both use AvFILLp. This has broken > Variable::Magic 0.59. > > As an interim measure, restore the old behaviour. > > > Affected files ... > M av.c > > Differences ... > > diff --git a/av.c b/av.c > index 21828a9..e3c6d5a 100644 > --- a/av.c > +++ b/av.c > @@ -272,7 +272,8 @@ Perl_av_fetch(pTHX_ AV *av, SSize_t key, I32 lval) > } > > neg = (key < 0); > - size = AvFILLp(av) + 1; > + /* XXX tmp restore old behaviour to make Variable::Magic pass */ > + size = (neg ? AvFILL(av): AvFILLp(av)) + 1; > key += neg * size; /* handle negative index without using branch */ > > /* the cast from SSize_t to Size_t allows both (key < 0) and (key >= size) > >
I've released version 0.60 that skips the test, so you can revert your revert if you want. Vincent
Date: Thu, 8 Sep 2016 09:07:33 +0100
To: "Vincent Pit (VPIT)" <perl [...] profvince.com>
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #128989] Bleadperl v5.25.3-266-g1d7e644 breaks VPIT/Variable-Magic-0.59.tar.gz
Download (untitled) / with headers
text/plain 316b
On Tue, Sep 06, 2016 at 07:36:20PM -0300, Vincent Pit (VPIT) wrote: Show quoted text
> I've released version 0.60 that skips the test, so you can revert your > revert if you want.
Ok thanks. Now unreverted. -- Counsellor Troi states something other than the blindingly obvious. -- Things That Never Happen in "Star Trek" #16


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org