Skip Menu |
Report information
Id: 128260
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: dcollinsn [at] gmail.com
Cc:
AdminCc:

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



Subject: Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
Download (untitled) / with headers
text/plain 6.9k
Greetings Porters, I have compiled bleadperl with the afl-gcc compiler using: ./Configure -Dusedevel -Dprefix='/usr/local/perl-afl' -Dcc='ccache afl-gcc' -Uuselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -Dusequadmath -des AFL_HARDEN=1 make && make test And then fuzzed the resulting binary using: AFL_NO_VAR_CHECK=1 afl-fuzz -i in -o out bin/perl @@ After reducing testcases using `afl-tmin` and performing additional minimization by hand, I have located the following testcase that triggers an assert fail in debug buids of the perl interpreter. The testcase is the file below. On normal builds, this runs normally (albeit with an expected warning). On debug builds, this returns an assert fail. dcollins@nightshade64:~/perl$ ./perl -Ilib -e '(vec%0,0,0)=0' Illegal number of bits in vec at -e line 1. dcollins@nightshade64:~/perl$ cd ../perldebug/ dcollins@nightshade64:~/perldebug$ ./perl -Ilib -e '(vec%0,0,0)=0' perl: sv.c:2924: Perl_sv_2pv_flags: Assertion `((svtype)((sv)->sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed. Aborted Debugging tool output is below. A git bisect was performed and reported the following, which appears to be when the assert was initially added. 217f6fa330a187be32a68556507e3165b8747f55 is the first bad commit commit 217f6fa330a187be32a68556507e3165b8747f55 Author: Father Chrysostomos <sprout@cpan.org> Date: Fri Jul 19 08:51:47 2013 -0700 sv.c: Assert that sv_[ivp]v are not passed aggregates The lack of assertions can hide bugs. See 32a609747bffb for instance :100644 100644 3ac0a2bce83bc12406be1fa22acefa8f8a2014c7 daa87f00081d96c8d15f94f02c32cd85e8af0266 M sv.c bisect run success **GDB** dcollins@nightshade64:~/perldebug$ gdb --args ./miniperl -Ilib -e '((vec%0,0,0)=0)' GNU gdb (GDB) 7.10 Copyright (C) 2015 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-unknown-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from ./miniperl...done. (gdb) run Starting program: /home/dcollins/perldebug/miniperl -Ilib -e \(\(vec%0,0,0\)=0\) [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". miniperl: sv.c:2924: Perl_sv_2pv_flags: Assertion `((svtype)((sv)->sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed. Program received signal SIGABRT, Aborted. 0x00007ffff6cf9478 in raise () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) bt #0 0x00007ffff6cf9478 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007ffff6cfa8fa in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x00007ffff6cf23a7 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #3 0x00007ffff6cf2452 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x00000000005b551c in Perl_sv_2pv_flags (sv=0xa959a8, lp=0x7fffffffe1e8, flags=2050) at sv.c:2923 #5 0x0000000000694542 in Perl_do_vecget (sv=0xa959a8, offset=0, size=0) at doop.c:752 #6 0x000000000061d3ab in Perl_pp_vec () at pp.c:3509 #7 0x000000000053fac1 in Perl_runops_debug () at dump.c:2239 #8 0x00000000004482ec in S_run_body (oldscope=1) at perl.c:2517 #9 0x0000000000447917 in perl_run (my_perl=0xa7f010) at perl.c:2440 #10 0x000000000072760b in main (argc=4, argv=0x7fffffffe618, env=0x7fffffffe640) at miniperlmain.c:122 (gdb) q **VALGRIND** No reported memory management errors. **PERL -V** dcollins@nightshade64:~/perldebug$ ./perl -Ilib -V Summary of my perl5 (revision 5 version 25 subversion 2) configuration: Commit id: c29dfc6a6c45f86648c51f961304254cc3c449b9 Platform: osname=linux, osvers=4.5.0-2-amd64, archname=x86_64-linux-ld uname='linux nightshade64 4.5.0-2-amd64 #1 smp debian 4.5.3-2 (2016-05-08) x86_64 gnulinux ' config_args='-Dusedevel -Dprefix=/usr/local/perl-afl -Dcc=ccache gcc-6.1 -Duselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -DDEBUGGING -DPERL_POISON -des' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef use64bitint=define, use64bitall=define, uselongdouble=define usemymalloc=n, bincompat5005=undef Compiler: cc='ccache gcc-6.1', ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-g', cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='', gccversion='6.1.0', 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='ccache gcc-6.1', ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/local/lib/gcc/x86_64-pc-linux-gnu/6.1.0/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 -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.22.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.22' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong' Characteristics of this binary (from libperl): Compile-time options: DEBUGGING 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 May 26 2016 17:57:37 @INC: lib /usr/local/perl-afl/lib/site_perl/5.25.2/x86_64-linux-ld /usr/local/perl-afl/lib/site_perl/5.25.2 /usr/local/perl-afl/lib/5.25.2/x86_64-linux-ld /usr/local/perl-afl/lib/5.25.2 /usr/local/perl-afl/lib/site_perl/5.25.1 /usr/local/perl-afl/lib/site_perl/5.24.0 /usr/local/perl-afl/lib/site_perl .
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 807b
On Thu May 26 18:52:05 2016, dcollinsn@gmail.com wrote: Show quoted text
> A git bisect was performed and > reported the following, which appears to be when the assert was > initially added. > > 217f6fa330a187be32a68556507e3165b8747f55 is the first bad commit > commit 217f6fa330a187be32a68556507e3165b8747f55 > Author: Father Chrysostomos <sprout@cpan.org> > Date: Fri Jul 19 08:51:47 2013 -0700 > > sv.c: Assert that sv_[ivp]v are not passed aggregates > > The lack of assertions can hide bugs. See 32a609747bffb for instance
That assertion was added specifically to find bugs like this: $ perl -we '(vec%0,0,1)=0' Use of uninitialized value within %0 in vec at -e line 1. Can't coerce HASH to string in aassign at -e line 1. That output is nonsensical. Thank you for finding one. :-) -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Thu May 26 21:38:50 2016, sprout wrote: Show quoted text
> On Thu May 26 18:52:05 2016, dcollinsn@gmail.com wrote:
> > A git bisect was performed and > > reported the following, which appears to be when the assert was > > initially added. > > > > 217f6fa330a187be32a68556507e3165b8747f55 is the first bad commit > > commit 217f6fa330a187be32a68556507e3165b8747f55 > > Author: Father Chrysostomos <sprout@cpan.org> > > Date: Fri Jul 19 08:51:47 2013 -0700 > > > > sv.c: Assert that sv_[ivp]v are not passed aggregates > > > > The lack of assertions can hide bugs. See 32a609747bffb for instance
> > That assertion was added specifically to find bugs like this: > > $ perl -we '(vec%0,0,1)=0' > Use of uninitialized value within %0 in vec at -e line 1. > Can't coerce HASH to string in aassign at -e line 1. > > That output is nonsensical. > > Thank you for finding one. :-)
A related issue is that prototype("CORE::vec") returns '$$$', but vec’s first argument must be a valid lvalue, so the prototype is wrong: $ perl -we '(vec $_+1,0,1)=0' Can't modify addition (+) in list assignment at -e line 1, at EOF Execution of -e aborted due to compilation errors. $ perl -we 'vec($_+1,0,1)=0' Can't modify addition (+) in scalar assignment at -e line 1, at EOF Execution of -e aborted due to compilation errors. It’s prototype currently is effectively ‘\[$@%*]$$’, but it should probably be ‘\$$$’. Also, those ‘Can't modify’ messages cite the wrong op (list/scalar assignment instead of vec). -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.8k
On Thu May 26 21:58:20 2016, sprout wrote: Show quoted text
> On Thu May 26 21:38:50 2016, sprout wrote:
> > On Thu May 26 18:52:05 2016, dcollinsn@gmail.com wrote:
> > > A git bisect was performed and > > > reported the following, which appears to be when the assert was > > > initially added. > > > > > > 217f6fa330a187be32a68556507e3165b8747f55 is the first bad commit > > > commit 217f6fa330a187be32a68556507e3165b8747f55 > > > Author: Father Chrysostomos <sprout@cpan.org> > > > Date: Fri Jul 19 08:51:47 2013 -0700 > > > > > > sv.c: Assert that sv_[ivp]v are not passed aggregates > > > > > > The lack of assertions can hide bugs. See 32a609747bffb for > > > instance
> > > > That assertion was added specifically to find bugs like this: > > > > $ perl -we '(vec%0,0,1)=0' > > Use of uninitialized value within %0 in vec at -e line 1. > > Can't coerce HASH to string in aassign at -e line 1. > > > > That output is nonsensical. > > > > Thank you for finding one. :-)
> > A related issue is that prototype("CORE::vec") returns '$$$', but > vec’s first argument must be a valid lvalue, so the prototype is > wrong: > > $ perl -we '(vec $_+1,0,1)=0' > Can't modify addition (+) in list assignment at -e line 1, at EOF > Execution of -e aborted due to compilation errors. > $ perl -we 'vec($_+1,0,1)=0' > Can't modify addition (+) in scalar assignment at -e line 1, at EOF > Execution of -e aborted due to compilation errors. > > It’s prototype currently is effectively ‘\[$@%*]$$’, but it should > probably be ‘\$$$’.
It’s more complicated than that. vec does accept $_+1 if vec itself is in rvalue context, so its actual behaviour cannot be represented perfectly with a prototype. Show quoted text
> Also, those ‘Can't modify’ messages cite the wrong op (list/scalar > assignment instead of vec).
That’s actually a clue to the problem. Scalar assignment dies at compile time, but list assignment goes on to do the wrong thing (and fail an assertion) at run time, because vec’s own type of lvalue context is being propagated, whereas it probably should not be. $ perl -we 'vec(%a,0,1)=0' Can't modify hash dereference in scalar assignment at -e line 1, at EOF Execution of -e aborted due to compilation errors. $ perl -we '(vec%a,0,1)=0' Name "main::a" used only once: possible typo at -e line 1. Use of uninitialized value within %a in vec at -e line 1. Can't coerce HASH to string in aassign at -e line 1. vec is not the only problematic operator. $ ./perl -we '(substr %a,0,1)=0' Name "main::a" used only once: possible typo at -e line 1. Can't coerce HASH to string in list assignment at -e line 1. $ ./perl -we 'substr(%a,0,1)=0' Can't modify hash dereference in scalar assignment at -e line 1, at EOF Execution of -e aborted due to compilation errors. The second one dies at compile time; the first one does the wrong thing at run time. Same problem. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.5k
On Fri May 27 17:55:20 2016, sprout wrote: Show quoted text
> It’s more complicated than that. vec does accept $_+1 if vec itself > is in rvalue context, so its actual behaviour cannot be represented > perfectly with a prototype. >
> > Also, those ‘Can't modify’ messages cite the wrong op (list/scalar > > assignment instead of vec).
> > That’s actually a clue to the problem. Scalar assignment dies at > compile time, but list assignment goes on to do the wrong thing (and > fail an assertion) at run time, because vec’s own type of lvalue > context is being propagated, whereas it probably should not be. > > $ perl -we 'vec(%a,0,1)=0' > Can't modify hash dereference in scalar assignment at -e line 1, at > EOF > Execution of -e aborted due to compilation errors. > $ perl -we '(vec%a,0,1)=0' > Name "main::a" used only once: possible typo at -e line 1. > Use of uninitialized value within %a in vec at -e line 1. > Can't coerce HASH to string in aassign at -e line 1. > > vec is not the only problematic operator. > > $ ./perl -we '(substr %a,0,1)=0' > Name "main::a" used only once: possible typo at -e line 1. > Can't coerce HASH to string in list assignment at -e line 1. > $ ./perl -we 'substr(%a,0,1)=0' > Can't modify hash dereference in scalar assignment at -e line 1, at > EOF > Execution of -e aborted due to compilation errors. > > The second one dies at compile time; the first one does the wrong > thing at run time. Same problem.
Should assignment to substr(keys) be allowed? $ ./perl -le 'substr(keys %h,1) = 3; print "ok"' ok $ ./perl -e '(substr keys %h,1) = 3;' Can't modify keys in list assignment at -e line 1, near "3;" Execution of -e aborted due to compilation errors. (The logic in op.c:op_lvalue_flags is really broken.) Currently assignment to substr(keys) actually does something: $ ./perl -le 'substr(keys %h,1) = 3; use Devel::Peek; Dump %h' ok SV = PVHV(0x7fb14c00aea0) at 0x7fb14c02b7e0 REFCNT = 1 FLAGS = (OOK,SHAREKEYS) AUX_FLAGS = 0 ARRAY = 0x7fb14bc09970 KEYS = 0 FILL = 0 (cached = 0) MAX = 7 RITER = -1 EITER = 0x0 RAND = 0x34097f19 $ ./perl -le 'substr(keys %h,1) = 30; use Devel::Peek; Dump %h' ok SV = PVHV(0x7fa35880aea0) at 0x7fa35882b7e0 REFCNT = 1 FLAGS = (OOK,SHAREKEYS) AUX_FLAGS = 0 ARRAY = 0x7fa358412350 KEYS = 0 FILL = 0 (cached = 0) MAX = 31 RITER = -1 EITER = 0x0 RAND = 0x77bd0a5d (See the MAX line, indicating the highest bucket index.) I just want to know if I should go ahead and break that, which would be the easiest way to fix the other bugs in this ticket. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 277b
On Thu Jun 09 07:03:06 2016, sprout wrote: Show quoted text
> $ ./perl -le 'substr(keys %h,1) = 3; use Devel::Peek; Dump %h' > ok > SV = PVHV(0x7fb14c00aea0) at 0x7fb14c02b7e0
Duh. Ignore the ‘ok’. (Shouldn’t reduce scripts *after* copying and pasting them!) -- Father Chrysostomos
Date: Thu, 9 Jun 2016 16:30:42 -0400
From: Eric Brine <ikegami [...] adaelis.com>
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
To: perlbug-followup <perlbug-followup [...] perl.org>
CC: perl5 porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 392b
On Thu, Jun 9, 2016 at 10:03 AM, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote:
Show quoted text
Should assignment to substr(keys) be allowed?

Is there a reason it shouldn't?

substr(keys)=... isn't too useful, but vec(substr)=... might be useful

(I'm assuming substr/keys/vec/$#a are interchangeable in that question, as they are the ops that return magical values in lvalue context.)

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 702b
On Thu Jun 09 13:31:21 2016, ikegami@adaelis.com wrote: Show quoted text
> On Thu, Jun 9, 2016 at 10:03 AM, Father Chrysostomos via RT < > perlbug-followup@perl.org> wrote: >
> > Should assignment to substr(keys) be allowed? > >
> > Is there a reason it shouldn't? > > substr(keys)=... isn't too useful, but vec(substr)=... might be useful
The fix I have in my head would allow vec(substr)=.... Show quoted text
> > (I'm assuming substr/keys/vec/$#a are interchangeable in that question, as > they are the ops that return magical values in lvalue context.)
keys() is extra extra special. keys() assignment only works in scalar assignment, not list assignment. The whole thing is a kludge, really. :-) -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Thu Jun 09 18:21:57 2016, sprout wrote: Show quoted text
> On Thu Jun 09 13:31:21 2016, ikegami@adaelis.com wrote:
> > On Thu, Jun 9, 2016 at 10:03 AM, Father Chrysostomos via RT < > > perlbug-followup@perl.org> wrote: > >
> > > Should assignment to substr(keys) be allowed? > > >
> > > > Is there a reason it shouldn't? > > > > substr(keys)=... isn't too useful, but vec(substr)=... might be > > useful
> > The fix I have in my head would allow vec(substr)=.... >
> > > > (I'm assuming substr/keys/vec/$#a are interchangeable in that > > question, as > > they are the ops that return magical values in lvalue context.)
> > keys() is extra extra special. keys() assignment only works in scalar > assignment, not list assignment. The whole thing is a kludge, really. > :-)
Hrm, I did not realize this, but foreach(scalar keys %h) gives a magic keys scalar as do foo(scalar keys %h), \scalar keys %h, and grep $_, scalar keys %h. (keys %h) = is not permitted though. Maybe it’s because that is list context. That seems the only logical reason. Only a scalar-context keys can give a magic assignable scalar. I guess I need to make substr(keys)= work. Now that I understand this code better, it shouldn’t be too hard. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Thu Jun 09 18:31:45 2016, sprout wrote: Show quoted text
> Hrm, I did not realize this, but foreach(scalar keys %h) gives a magic > keys scalar as do foo(scalar keys %h), \scalar keys %h, and grep $_, > scalar keys %h. > > (keys %h) = is not permitted though. Maybe it’s because that is > list context. That seems the only logical reason. Only a scalar- > context keys can give a magic assignable scalar. > > I guess I need to make substr(keys)= work. Now that I understand this > code better, it shouldn’t be too hard.
keys(%h) works in some lvalue scalar contexts, but not others: $ ./perl -Ilib -e 'keys(%h).="00"' Can't modify keys in concatenation (.) or string at -e line 1, at EOF Execution of -e aborted due to compilation errors. $ ./perl -Ilib -e 'read FH, keys(%h), 0' Can't modify keys in read at -e line 1, at EOF Execution of -e aborted due to compilation errors. keys has its own list of exceptions for the lvalue type. If I were to make it use the same logic as other ops that make a distinction between scalar and list lvalue context (by calling S_scalar_mod_type), then it would be easier not to break it when fixing other bugs, and I could stop worrying about it. The .= and read examples above would start working, which would be good (though no sane code would do that) since it would be more predictable. -- Father Chrysostomos
CC: perl5-porters [...] perl.org
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
From: Dave Mitchell <davem [...] iabyn.com>
Date: Fri, 10 Jun 2016 09:03:41 +0100
Download (untitled) / with headers
text/plain 484b
On Thu, Jun 09, 2016 at 10:51:12PM -0700, Father Chrysostomos via RT wrote: Show quoted text
> > I guess I need to make substr(keys)= work. Now that I understand this > > code better, it shouldn’t be too hard.
So would substr(keys %h, ...) = foo; be equivalent to: my $k = keys %h; substr($k, ....) = foo; keys %h = $k; -- All wight. I will give you one more chance. This time, I want to hear no Wubens. No Weginalds. No Wudolf the wed-nosed weindeers. -- Life of Brian
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 442b
On Fri Jun 10 01:22:52 2016, davem wrote: Show quoted text
> On Thu, Jun 09, 2016 at 10:51:12PM -0700, Father Chrysostomos via RT wrote:
> > > I guess I need to make substr(keys)= work. Now that I understand this > > > code better, it shouldn’t be too hard.
> > So would > > substr(keys %h, ...) = foo; > > be equivalent to: > > my $k = keys %h; > substr($k, ....) = foo; > keys %h = $k;
It already is. :-) -- Father Chrysostomos
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
From: demerphq <demerphq [...] gmail.com>
CC: Perl5 Porteros <perl5-porters [...] perl.org>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Date: Fri, 10 Jun 2016 19:23:02 +0200
Download (untitled) / with headers
text/plain 838b
On 10 June 2016 at 17:31, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Fri Jun 10 01:22:52 2016, davem wrote:
>> On Thu, Jun 09, 2016 at 10:51:12PM -0700, Father Chrysostomos via RT wrote:
>> > > I guess I need to make substr(keys)= work. Now that I understand this >> > > code better, it shouldn’t be too hard.
>> >> So would >> >> substr(keys %h, ...) = foo; >> >> be equivalent to: >> >> my $k = keys %h; >> substr($k, ....) = foo; >> keys %h = $k;
> > It already is. :-)
So when or if we switch our hash implementation to something where this type of syntax does not make sense what are we going to do with this code? I really feel like this goes in the wrong direction. We should be moving towards making assignment to keys a no-op, not making it work better and in more situations. Yves
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Fri Jun 10 10:23:31 2016, demerphq wrote: Show quoted text
> On 10 June 2016 at 17:31, Father Chrysostomos via RT > <perlbug-followup@perl.org> wrote:
> > On Fri Jun 10 01:22:52 2016, davem wrote:
> >> On Thu, Jun 09, 2016 at 10:51:12PM -0700, Father Chrysostomos via RT > >> wrote:
> >> > > I guess I need to make substr(keys)= work. Now that I > >> > > understand this > >> > > code better, it shouldn’t be too hard.
> >> > >> So would > >> > >> substr(keys %h, ...) = foo; > >> > >> be equivalent to: > >> > >> my $k = keys %h; > >> substr($k, ....) = foo; > >> keys %h = $k;
> > > > It already is. :-)
> > So when or if we switch our hash implementation to something where > this type of syntax does not make sense what are we going to do with > this code? > > I really feel like this goes in the wrong direction. We should be > moving towards making assignment to keys a no-op, not making it work > better and in more situations.
Aha. I thought you were going to speak up along those lines. Until we actually remove keys assignment, I have no intention of accidentally breaking it. By making it share code with other ops, instead of having its own special cases (and this sharing has the side effect of allowing .= etc.), I can stop worrying about it and get on with fixing other bugs. As for what keys(%h)= should do if we want to change that, I have no idea. Nor do I have much interest. All I am doing is changing the code surrounding the *syntax*, not the actual implementation of keys lvalues, in order to make it easier to fix other syntax bugs, such as those discussed earlier in this ticket. Make sense? -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 953b
On Fri Jun 10 13:06:29 2016, sprout wrote, in response to Yves Orton: Show quoted text
> Aha. I thought you were going to speak up along those lines. Until > we actually remove keys assignment, I have no intention of > accidentally breaking it. By making it share code with other ops, > instead of having its own special cases (and this sharing has the side > effect of allowing .= etc.), I can stop worrying about it and get on > with fixing other bugs. > > As for what keys(%h)= should do if we want to change that, I have no > idea. Nor do I have much interest. All I am doing is changing the > code surrounding the *syntax*, not the actual implementation of keys > lvalues, in order to make it easier to fix other syntax bugs, such as > those discussed earlier in this ticket. > > Make sense?
I do hope you respond soon. I have a branch waiting to be merged (smoke-me/128260), but I won’t merge it if there are still objections. -- Father Chrysostomos
To: perl5-porters [...] perl.org
From: Aristotle Pagaltzis <pagaltzis [...] gmx.de>
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
Date: Sat, 11 Jun 2016 05:35:06 +0200
Download (untitled) / with headers
text/plain 1.3k
* demerphq <demerphq@gmail.com> [2016-06-10 19:24]: Show quoted text
> So when or if we switch our hash implementation to something where > this type of syntax does not make sense what are we going to do with > this code?
If assignment to keys() really ought to become a no-op, then it should be made it do nothing silently. It should definitely not throw errors, since it continues to do something useful on old perls and there is no gain from forcing users to go into their code and delete it or version- guard it. Show quoted text
> I really feel like this goes in the wrong direction. We should be > moving towards making assignment to keys a no-op, not making it work > better and in more situations.
I think it’s going in the right direction as well as the wrong direction at the same time. Making assignment to keys less special seems just as useful for making it a silent no-op as for making it work better and in more situations. However, I disagree with the premise. The specific notion of “allocate this number of hash buckets” may make no sense for other implementations but an abstract intent of “pre-allocate extra space in this hash since I am about to add a bunch of data” seems like it would be applicable no matter the implementation chosen, just as the same intent is applicable and expressible for arrays too. Regards, -- Aristotle Pagaltzis // <http://plasmasturm.org/>
Date: Sat, 11 Jun 2016 10:28:50 +0200
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
From: demerphq <demerphq [...] gmail.com>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 1.3k

Sorry. Yes i understand now. Please go ahead, i just get grumpy about this abstraction violation, but you are right to want the syntax to be bug free. Ill try to restrain my grumpy side in the future. :-)

On 11 Jun 2016 05:02, "Father Chrysostomos via RT" <perlbug-followup@perl.org> wrote:
Show quoted text
On Fri Jun 10 13:06:29 2016, sprout wrote, in response to Yves Orton:
> Aha.  I thought you were going to speak up along those lines.  Until
> we actually remove keys assignment, I have no intention of
> accidentally breaking it.  By making it share code with other ops,
> instead of having its own special cases (and this sharing has the side
> effect of allowing .= etc.), I can stop worrying about it and get on
> with fixing other bugs.
>
> As for what keys(%h)= should do if we want to change that, I have no
> idea.  Nor do I have much interest.  All I am doing is changing the
> code surrounding the *syntax*, not the actual implementation of keys
> lvalues, in order to make it easier to fix other syntax bugs, such as
> those discussed earlier in this ticket.
>
> Make sense?

I do hope you respond soon.  I have a branch waiting to be merged (smoke-me/128260), but I won’t merge it if there are still objections.

--

Father Chrysostomos


---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=128260
Date: Sat, 11 Jun 2016 10:55:14 +0200
CC: Perl5 Porteros <perl5-porters [...] perl.org>
To: Aristotle Pagaltzis <pagaltzis [...] gmx.de>
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 2.4k


On 11 Jun 2016 05:35, "Aristotle Pagaltzis" <pagaltzis@gmx.de> wrote:
Show quoted text

>
> * demerphq <demerphq@gmail.com> [2016-06-10 19:24]:
> > So when or if we switch our hash implementation to something where
> > this type of syntax does not make sense what are we going to do with
> > this code?
>
> If assignment to keys() really ought to become a no-op, then it should
> be made it do nothing silently. It should definitely not throw errors,
> since it continues to do something useful on old perls and there is no
> gain from forcing users to go into their code and delete it or version-
> guard it.

While i have sympathy about the point about back compat and guarding i still think assignment to keys should be replaced by a generic tool in hash::util that would be responsible for configuring our associative arrays.

If things were done carefully you would be able to write new style code that worked fine on older perls.  For instance imagine:

conf_hash (%hash, min_keys => 1000, some_new_prop => 5);

Which on older perls would be the same as assigning 1000 to keys, and on newer perls would set some other property of the algorithm.

> > I really feel like this goes in the wrong direction. We should be
Show quoted text

> > moving towards making assignment to keys a no-op, not making it work
> > better and in more situations.
>
> I think it’s going in the right direction as well as the wrong direction
> at the same time. Making assignment to keys less special seems just as
> useful for making it a silent no-op as for making it work better and in
> more situations.
>
> However, I disagree with the premise. The specific notion of “allocate
> this number of hash buckets” may make no sense for other implementations
> but an abstract intent of “pre-allocate extra space in this hash since
> I am about to add a bunch of data” seems like it would be applicable no
> matter the implementation chosen, just as the same intent is applicable
> and expressible for arrays too.

While i concede that some of the algorithms we might choose could have such a config parameter i think its just as likely or more that we would choose an algorithm, like a balanced tree, where it does not make sense, and/or where there might be more than one option you can twiddle.

Making all this explicit and future back compat proof seems to me to be long term preferable to preserving this language eccentricity.

I will try to find time soon to start a fresh thread that summarizes my thoughts on this so we can discuss it properly.

Yves

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 828b
On Sat Jun 11 01:29:31 2016, demerphq wrote: Show quoted text
> Sorry. Yes i understand now. Please go ahead,
Now pushed. e4fc70828d2 makes keys consistent with other ops with regard to list vs scalar lvalue context. 1a3e97240 is a refactoring to prepare for the following commit. 79409ac83 fixes the assertion failure originally reported in this ticket, making it a compile-time error, and also fixes the wording in the error messages to say ‘in vec’ and ‘in substr’. Show quoted text
> i just get grumpy about this > abstraction violation,
In fact, it has never really made much sense. keys %h = keys %h ought to be a no-op, but keys assignment specifies *buckets* whereas keys returns *keys*. If keys %h = 200 simply hinted that there would be about 200 keys soon, it would not be an abstraction violation, would it? -- Father Chrysostomos
CC: Perl5 Porteros <perl5-porters [...] perl.org>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128260] Assert fail in Perl_sv_2pv_flags: '(vec%0,0,0)=0'
Date: Sun, 12 Jun 2016 09:32:25 +0200
Download (untitled) / with headers
text/plain 1.5k
On 11 June 2016 at 16:27, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Sat Jun 11 01:29:31 2016, demerphq wrote:
>> Sorry. Yes i understand now. Please go ahead,
> > Now pushed. > > e4fc70828d2 makes keys consistent with other ops with regard to list vs scalar lvalue context. > > 1a3e97240 is a refactoring to prepare for the following commit. > > 79409ac83 fixes the assertion failure originally reported in this ticket, making it a compile-time error, and also fixes the wording in the error messages to say ‘in vec’ and ‘in substr’. >
>> i just get grumpy about this >> abstraction violation,
> > In fact, it has never really made much sense. keys %h = keys %h ought to be a no-op, but keys assignment specifies *buckets* whereas keys returns *keys*.
Yeah, I agree. Syntactically it is very odd. Show quoted text
> If keys %h = 200 simply hinted that there would be about 200 keys soon, it would not be an abstraction violation, would it?
Effectively that is what it does. With our current implementation it says "make sure there are at least 200 buckets", and that is it. And I suppose you are right, if you think of it as a hint of that sort then I guess it is harmless. I guess we don't have to get rid of it, we can just add something new that supplants it. It still bugs me that people have to waste their time fixing something that shouldn't really exist. But I shouldn't let that frustration get me annoyed with people like you who are just dealing with what we have. I apologise for that. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 387b
On Sat Jun 11 07:27:38 2016, sprout wrote: Show quoted text
> 79409ac83 fixes the assertion failure originally reported in this > ticket, making it a compile-time error,
Except I fell into the same trap as the original bug, and that is to allow an HV or AV to pass to SV-only functions, by still propagating the outer lvalue context for \substr %h. I fixed that in 33a10326b. -- Father Chrysostomos
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


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