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

chop(@x =~ tr///) #15738

Closed
p5pRT opened this issue Nov 28, 2016 · 9 comments
Closed

chop(@x =~ tr///) #15738

p5pRT opened this issue Nov 28, 2016 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 28, 2016

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

Searchable as RT130198$

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2016

From @hvds

Created by @hvds

In [perl #124368], Brian reports a second unrelated AFL-generated case
that happens to hit the same assertion in Perl_sv_2pv_flags(); in short​:

% perl -e 'chop(@​x =~ tr/1/1/)'
perl​: sv.c​:2941​: 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 (core dumped)
%

Without -DDEBUGGING​:

% ./miniperl -wle '@​x=(1..11); print scalar @​x; print @​x =~ tr/1/1/; print chop(@​x =~ tr/1/1/)'
Applying transliteration (tr///) to @​x will act on scalar(@​x) at -e line 1.
Applying transliteration (tr///) to @​x will act on scalar(@​x) at -e line 1.
11
2
Use of uninitialized value in transliteration (tr///) at -e line 1.
0
%

The same works fine on a string, apparently happy to chop the temp scalar​:

% ./miniperl -wle '$s="11"; print $s; print $s =~ tr/1/1/; print chop($s =~ tr/1/1/)'
11
2
2
%

.. but I note that if you throw a scalar() in there (which I'd have expected to have no effect), both cases complain (and in rather dodgy style)​:

% ./miniperl -wle '@​x=(1..11); chop(scalar(@​x) =~ tr/1/1/)'
Can't modify scalar in chop at -e line 1, near "tr/1/1/)
"
Execution of -e aborted due to compilation errors.
% ./miniperl -wle '$s="11"; chop(scalar($s) =~ tr/1/1/)'
Can't modify scalar in chop at -e line 1, near "tr/1/1/)
"
Execution of -e aborted due to compilation errors.
%

I'm not sure now what correct behaviour would be here, or how to make it so.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.25.7:

Configured by hv at Wed Nov  9 13:58:26 GMT 2016.

Summary of my perl5 (revision 5 version 25 subversion 7) configuration:
  Commit id: 392582f8c00033c1199d338baf72f42bbd9ef2dc
  Platform:
    osname=linux
    osvers=3.13.0-37-generic
    archname=x86_64-linux
    uname='linux shad2 3.13.0-37-generic #64-ubuntu smp mon sep 22 21:28:38 utc 2014 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dcc=gcc -Dprefix=/opt/blead-d0 -Doptimize=-g -O0 -DDEBUGGING -Dusedevel -Uversiononly'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    bincompat5005=undef
  Compiler:
    cc='gcc'
    ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-g -O0'
    cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion=''
    gccversion='4.8.4'
    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='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='gcc'
    ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.8/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 /lib64 /usr/lib64
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.19.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.19'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -g -O0 -L/usr/local/lib -fstack-protector'



@INC for perl 5.25.7:
    /opt/blead-d0/lib/perl5/site_perl/5.25.7/x86_64-linux
    /opt/blead-d0/lib/perl5/site_perl/5.25.7
    /opt/blead-d0/lib/perl5/5.25.7/x86_64-linux
    /opt/blead-d0/lib/perl5/5.25.7


Environment for perl 5.25.7:
    HOME=/home/hv
    LANG=C
    LANGUAGE=en_GB:en
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/hv/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2016

From @jkeenan

On Mon, 28 Nov 2016 11​:32​:38 GMT, hv wrote​:

This is a bug report for perl from hv@​crypt.org,
generated with the help of perlbug 1.40 running under perl 5.25.7.

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

In [perl #124368], Brian reports a second unrelated AFL-generated case
that happens to hit the same assertion in Perl_sv_2pv_flags(); in
short​:

% perl -e 'chop(@​x =~ tr/1/1/)'
perl​: sv.c​:2941​: 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 (core dumped)
%

Without -DDEBUGGING​:

% ./miniperl -wle '@​x=(1..11); print scalar @​x; print @​x =~ tr/1/1/;
print chop(@​x =~ tr/1/1/)'
Applying transliteration (tr///) to @​x will act on scalar(@​x) at -e
line 1.
Applying transliteration (tr///) to @​x will act on scalar(@​x) at -e
line 1.
11
2
Use of uninitialized value in transliteration (tr///) at -e line 1.
0
%

The same works fine on a string, apparently happy to chop the temp
scalar​:

% ./miniperl -wle '$s="11"; print $s; print $s =~ tr/1/1/; print
chop($s =~ tr/1/1/)'
11
2
2
%

.. but I note that if you throw a scalar() in there (which I'd have
expected to have no effect), both cases complain (and in rather dodgy
style)​:

% ./miniperl -wle '@​x=(1..11); chop(scalar(@​x) =~ tr/1/1/)'
Can't modify scalar in chop at -e line 1, near "tr/1/1/)
"
Execution of -e aborted due to compilation errors.
% ./miniperl -wle '$s="11"; chop(scalar($s) =~ tr/1/1/)'
Can't modify scalar in chop at -e line 1, near "tr/1/1/)
"
Execution of -e aborted due to compilation errors.
%

I'm not sure now what correct behaviour would be here, or how to make
it so.

In 'perldoc -f tr', I see no documentation of anything like​:

#####
@​x =~ tr/1/1/
#####

So if that is essentially undefined behavior, then,

#####
chop(@​x =~ tr/1/1/)
#####

would be undefined as well.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2016

From @hvds

On Fri, 30 Dec 2016 16​:08​:57 -0800, jkeenan wrote​:

In 'perldoc -f tr', I see no documentation of anything like​:

#####
@​x =~ tr/1/1/
#####

So if that is essentially undefined behavior [...]

Well the docs say it expects to be supplied a string, and standard perl behaviours in such circumstances would be either to convert what is supplied into a string as best it can, or to give an error.

Consider the analagous behaviour for a pattern match​:

% perl -wle '@​x=(91..95); $y = (@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
5 (1)
% perl -wle '@​x=(91..95); $y = chop(@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
Can't modify pattern match (m//) in chop at -e line 1, near "/(\d+)/)"
Execution of -e aborted due to compilation errors.
%

Hugo

@p5pRT
Copy link
Author

p5pRT commented Dec 31, 2016

From [Unknown Contact. See original ticket]

On Fri, 30 Dec 2016 16​:08​:57 -0800, jkeenan wrote​:

In 'perldoc -f tr', I see no documentation of anything like​:

#####
@​x =~ tr/1/1/
#####

So if that is essentially undefined behavior [...]

Well the docs say it expects to be supplied a string, and standard perl behaviours in such circumstances would be either to convert what is supplied into a string as best it can, or to give an error.

Consider the analagous behaviour for a pattern match​:

% perl -wle '@​x=(91..95); $y = (@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
5 (1)
% perl -wle '@​x=(91..95); $y = chop(@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
Can't modify pattern match (m//) in chop at -e line 1, near "/(\d+)/)"
Execution of -e aborted due to compilation errors.
%

Hugo

@p5pRT
Copy link
Author

p5pRT commented Jan 2, 2017

From @iabyn

On Sat, Dec 31, 2016 at 12​:45​:21AM -0800, Hugo van der Sanden via RT wrote​:

On Fri, 30 Dec 2016 16​:08​:57 -0800, jkeenan wrote​:

In 'perldoc -f tr', I see no documentation of anything like​:

#####
@​x =~ tr/1/1/
#####

So if that is essentially undefined behavior [...]

Well the docs say it expects to be supplied a string, and standard perl behaviours in such circumstances would be either to convert what is supplied into a string as best it can, or to give an error.

Consider the analagous behaviour for a pattern match​:

% perl -wle '@​x=(91..95); $y = (@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
5 (1)
% perl -wle '@​x=(91..95); $y = chop(@​x =~ /(\d+)/); print "$1 ($y)" if $y'
Applying pattern match (m//) to @​x will act on scalar(@​x) at -e line 1.
Can't modify pattern match (m//) in chop at -e line 1, near "/(\d+)/)"
Execution of -e aborted due to compilation errors.

This issue should be fixed by the following commit.

commit 2108cbc
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Mon Jan 2 16​:37​:27 2017 +0000
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Mon Jan 2 16​:52​:34 2017 +0000

  Handle chop(@​a =~ tr///)
 
  RT #130198
 
  'chop(@​x =~ tr/1/1/)' crashed with an assertion failure. Ditto for chomp.
 
  There are two quirks which together cause this. First, the op tree for
  a tr// is different from other bind ops​:
 
  $ perl -MO=Concise -e'$x =~ m/a/'
  5 <@​> leave[1 ref] vKP/REFC ->(end)
  1 <0> enter ->2
  2 <;> nextstate(main 1 -e​:1) v​:{ ->3
  4 </> match(/"a"/) vKS ->5
  - <1> ex-rv2sv sK/1 ->4
  3 <#> gvsv[*x] s ->4
 
  $ perl -MO=Concise -e'$x =~ tr/a/b/'
  5 <@​> leave[1 ref] vKP/REFC ->(end)
  1 <0> enter ->2
  2 <;> nextstate(main 1 -e​:1) v​:{ ->3
  - <1> null vKS/2 ->5
  - <1> ex-rv2sv sKRM/1 ->4
  3 <#> gvsv[*x] s ->4
  4 <"> trans sS ->5
 
  Note that the argument for the match is a child of the match, while the
  arg of the trans is an (earlier) sibing of the trans (linked by a common
  null parent).
 
  The normal code path that croaks when e.g. a match is seen in an lvalue
  context,
 
  $ perl -e'chop(@​a =~ /a/)'
  Can't modify pattern match (m//) in chop at -e line 1, near "/a/)
 
  is skipped, since lvalue() is only called for the first child of a null op.
 
  Fixing this is as simple as calling lvalue() on the RHS too if the RHS is
  a trans op.
 
  The second issue is that chop and chomp are special-cased not to flatten
  an array; so
 
  @​b = 10..99;
  chop $a, @​b, $c;
 
  pushes 3 items on the stack to pass to pp_chop, rather than 102. pp_chop()
  itself then iterates over any array args.
 
  The compiler was seeing the rv2av op in chop(@​a =~ tr///) and was setting
  the OPf_REF (don't flatten) flag on it. Which then caused pp_trans to
  panic when its arg was an AV rather than a string.
 
  This second issue is now moot, since after the fix suggested above, we
  will have croaked before we reach the place where OPf_REF would be set.
 
  This commit adds lots of tests, since tr/a/a/ and tr/a/b/r are
  special-cased in terms of whether they are regarded as modifying the
  var they are bound to.

Affected files ...
  M op.c
  M t/op/tr.t

Differences ...

Inline Patch
diff --git a/op.c b/op.c
index 394efef..339a9ce 100644
--- a/op.c
+++ b/op.c
@@ -3164,9 +3164,32 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
 	    goto nomod;
 	else if (!(o->op_flags & OPf_KIDS))
 	    break;
+
 	if (o->op_targ != OP_LIST) {
-	    op_lvalue(cBINOPo->op_first, type);
-	    break;
+            OP *sib = OpSIBLING(cLISTOPo->op_first);
+            /* OP_TRANS and OP_TRANSR with argument have a weird optree
+             * that looks like
+             *
+             *   null
+             *      arg
+             *      trans
+             *
+             * compared with things like OP_MATCH which have the argument
+             * as a child:
+             *
+             *   match
+             *      arg
+             *
+             * so handle specially to correctly get "Can't modify" croaks etc
+             */
+
+            if (sib && (sib->op_type == OP_TRANS || sib->op_type == OP_TRANSR))
+            {
+                /* this should trigger a "Can't modify transliteration" err */
+                op_lvalue(sib, type);
+            }
+            op_lvalue(cBINOPo->op_first, type);
+            break;
 	}
 	/* FALLTHROUGH */
     case OP_LIST:
diff --git a/t/op/tr.t b/t/op/tr.t
index 47acd9e..2ef2a68 100644
--- a/t/op/tr.t
+++ b/t/op/tr.t
@@ -13,7 +13,7 @@ BEGIN {
 
 use utf8;
 
-plan tests => 166;
+plan tests => 214;
 
 # Test this first before we extend the stack with other operations.
 # This caused an asan failure due to a bad write past the end of the stack.
@@ -656,4 +656,48 @@ for ("", nullrocow) {
     is($string, "A", 'tr// of \N{name} works for upper-Latin1');
 }
 
+# RT #130198
+# a tr/// that is cho(m)ped, possibly with an array as arg
+
+{
+    use warnings;
+
+    my ($s, @a);
+
+    my $warn;
+    local $SIG{__WARN__ } = sub { $warn .= "@_" };
+
+    for my $c (qw(chop chomp)) {
+        for my $bind ('', '$s =~ ', '@a =~ ') {
+            for my $arg2 (qw(a b)) {
+                for my $r ('', 'r') {
+                    $warn = '';
+                    # tr/a/b/ modifies its LHS, so if the LHS is an
+                    # array, this should die. The special cases of tr/a/a/
+                    # and tr/a/b/r don't modify their LHS, so instead
+                    # we croak because cho(m)p is trying to modify it.
+                    #
+                    my $exp =
+                        ($r eq '' && $arg2 eq 'b' && $bind =~ /\@a/)
+                            ? qr/Can't modify private array in transliteration/
+                            : qr{Can't modify transliteration \(tr///\) in $c};
+
+                    my $expr = "$c(${bind}tr/a/$arg2/$r);";
+                    eval $expr;
+                    like $@, $exp, "RT #130198 eval: $expr";
+
+                    $exp =
+                        $bind =~ /\@a/
+                         ? qr{^Applying transliteration \(tr///\) to \@a will act on scalar\(\@a\)}
+                         : qr/^$/;
+                    like $warn, $exp, "RT #130198 warn: $expr";
+                }
+            }
+        }
+    }
+
+
+}
+
+
 1;


-- 

Justice is when you get what you deserve.
Law is when you get what you pay for.

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2017

@iabyn - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

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.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant