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

Substr in encode leaks memory #15871

Closed
p5pRT opened this issue Feb 13, 2017 · 14 comments
Closed

Substr in encode leaks memory #15871

p5pRT opened this issue Feb 13, 2017 · 14 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Feb 13, 2017

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

Searchable as RT130766$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

From arza@arza.us

Created by arza@arza.us

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Perl Info

Flags:
    category=library
    severity=medium
    module=Encode

Site configuration information for perl 5.24.1:

Configured by builduser at Sun Jan 15 14:53:06 CET 2017.

Summary of my perl5 (revision 5 version 24 subversion 1) configuration:

  Platform:
    osname=linux, osvers=4.9.3-1-arch, archname=x86_64-linux-thread-multi
    uname='linux flo-64 4.9.3-1-arch #1 smp preempt thu jan 12 21:34:12 cet
2017 x86_64 gnulinux '
    config_args='-des -Dusethreads -Duseshrplib -Doptimize=-march=x86-64
-mtune=generic -O2 -pipe -fstack-protector-strong -Dprefix=/usr
-Dvendorprefix=/usr -Dprivlib=/usr/share/perl5/core_perl
-Darchlib=/usr/lib/perl5/core_perl -Dsitelib=/usr/share/perl5/site_perl
-Dsitearch=/usr/lib/perl5/site_perl
-Dvendorlib=/usr/share/perl5/vendor_perl
-Dvendorarch=/usr/lib/perl5/vendor_perl -Dscriptdir=/usr/bin/core_perl
-Dsitescript=/usr/bin/site_perl -Dvendorscript=/usr/bin/vendor_perl
-Dinc_version_list=none -Dman1ext=1perl -Dman3ext=3perl
-Dcccdlflags='-fPIC' -Dlddlflags=-shared
-Wl,-O1,--sort-common,--as-needed,-z,relro
-Dldflags=-Wl,-O1,--sort-common,--as-needed,-z,relro'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv
-fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-march=x86-64 -mtune=generic -O2 -pipe
-fstack-protector-strong',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include'
    ccversion='', gccversion='6.3.1 20170109', 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='cc', ldflags ='-Wl,-O1,--sort-common,--as-needed,-z,relro
-fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib
/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1/include-fixed /usr/lib /lib/../lib
/usr/lib/../lib /lib /lib64 /usr/lib64
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
-lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.24.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
-Wl,-rpath,/usr/lib/perl5/core_perl/CORE'
    cccdlflags='-fPIC', lddlflags='-shared
-Wl,-O1,--sort-common,--as-needed,-z,relro -L/usr/local/lib
-fstack-protector-strong'



@INC for perl 5.24.1:
    /usr/lib/perl5/site_perl
    /usr/share/perl5/site_perl
    /usr/lib/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib/perl5/core_perl
    /usr/share/perl5/core_perl


Environment for perl 5.24.1:
    HOME=/home/user
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)

PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

From @jkeenan

On Mon, 13 Feb 2017 02​:11​:21 GMT, arza@​arza.us wrote​:

This is a bug report for perl from arza@​arza.us,
generated with the help of perlbug 1.40 running under perl 5.24.1.

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

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Can you clarify how you detect or record the memory leak?

Thank you very much.

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

From @shlomif

Hi all!

On Mon, 13 Feb 2017 05​:37​:15 -0800, jkeenan wrote​:

On Mon, 13 Feb 2017 02​:11​:21 GMT, arza@​arza.us wrote​:

This is a bug report for perl from arza@​arza.us,
generated with the help of perlbug 1.40 running under perl 5.24.1.

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

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Can you clarify how you detect or record the memory leak?

Thank you very much.

I cannot speak for arza (=the original reporter), but when I run «perl -e 'use Encode; while (1) { encode("ascii", substr("test",1)); }'» from the command line, then​:

1. Using perl-5.22.3-1.mga6 (on mageia x86-64 v6), the RAM consumption in htop stays the same at 0.1% even after waiting a long time.

2. The RAM consumption of the perl process in htop using perl-5.24.1 from perlbrew consistently grows - I killed it after it occupied over 10% of my 8 GB of RAM. It appears that both VIRT and RES are steadily growing.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

From @jkeenan

On Mon, 13 Feb 2017 15​:02​:00 GMT, shlomif wrote​:

Hi all!

On Mon, 13 Feb 2017 05​:37​:15 -0800, jkeenan wrote​:

On Mon, 13 Feb 2017 02​:11​:21 GMT, arza@​arza.us wrote​:

This is a bug report for perl from arza@​arza.us,
generated with the help of perlbug 1.40 running under perl 5.24.1.

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

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) {
encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Can you clarify how you detect or record the memory leak?

Thank you very much.

I cannot speak for arza (=the original reporter), but when I run «perl
-e 'use Encode; while (1) { encode("ascii", substr("test",1)); }'»
from the command line, then​:

1. Using perl-5.22.3-1.mga6 (on mageia x86-64 v6), the RAM consumption
in htop stays the same at 0.1% even after waiting a long time.

2. The RAM consumption of the perl process in htop using perl-5.24.1
from perlbrew consistently grows - I killed it after it occupied over
10% of my 8 GB of RAM. It appears that both VIRT and RES are steadily
growing.

Shlomi, thanks! Confirmed. Since this is a regression, I'm marking the ticket as a blocker for next 5.24 and 5.26 releases.

Thank you very much.

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 13, 2017

From @jkeenan

On Mon, 13 Feb 2017 02​:11​:21 GMT, arza@​arza.us wrote​:

This is a bug report for perl from arza@​arza.us,
generated with the help of perlbug 1.40 running under perl 5.24.1.

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

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

I have not been able to figure out a way to use Porting/bisect.pl to bisect this problem, but manual bisection suggests that the bug emerged between these two commits in Jan 2016.

b0248db looks okay
5c1db56 looks bad

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 14, 2017

From @jkeenan

On Mon, 13 Feb 2017 21​:55​:11 GMT, jkeenan wrote​:

On Mon, 13 Feb 2017 02​:11​:21 GMT, arza@​arza.us wrote​:

This is a bug report for perl from arza@​arza.us,
generated with the help of perlbug 1.40 running under perl 5.24.1.

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

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

I have not been able to figure out a way to use Porting/bisect.pl to
bisect this problem, but manual bisection suggests that the bug
emerged between these two commits in Jan 2016.

b0248db looks okay
5c1db56 looks bad

Manual bisection suggests that the following is the commit where, following Shlomi's suggestion, memory starts to leak​:

#####
$ git show |cat
commit 5c1db56
Author​: Tony Cook <tony@​develop-help.com>
Date​: Tue Dec 8 11​:19​:48 2015 +1100

  [perl #126633] copy anything gmagical on the right
 
  It could retrieve something we're setting on the left.

Inline Patch
diff --git a/pp_hot.c b/pp_hot.c
index 650f06b..b80efae 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1173,7 +1173,7 @@ S_aassign_copy_common(pTHX_ SV **firstlelem, SV **lastlelem,
         svr = *relem;
         assert(svr);
 
-        if (UNLIKELY(SvFLAGS(svr) & SVf_BREAK || copy_all)) {
+        if (UNLIKELY(SvFLAGS(svr) & (SVf_BREAK|SVs_GMG) || copy_all)) {
 
 #ifdef DEBUGGING
             if (fake) {
@@ -1265,6 +1265,10 @@ PP(pp_aassign)
 
     /* at least 2 LH and RH elements, or commonality isn't an issue */
     if (firstlelem < lastlelem && firstrelem < lastrelem) {
+        for (relem = firstrelem+1; relem <= lastrelem; relem++) {
+            if (SvGMAGICAL(*relem))
+                goto do_scan;
+        }
         for (lelem = firstlelem; lelem <= lastlelem; lelem++) {
             if (*lelem && SvSMAGICAL(*lelem))
                 goto do_scan;
diff --git a/t/op/aassign.t b/t/op/aassign.t
index d6a1a42..03cc84c 100644
--- a/t/op/aassign.t
+++ b/t/op/aassign.t
@@ -345,9 +345,10 @@ SKIP: {
 
 { # magic handling, see #126633
     use v5.22;
+    my $set;
     package ArrayProxy {
         sub TIEARRAY { bless [ $_[1] ] }
-        sub STORE { $_[0][0]->[$_[1]] = $_[2] }
+        sub STORE { $_[0][0]->[$_[1]] = $_[2]; $set = 1 }
         sub FETCH { $_[0][0]->[$_[1]] }
         sub CLEAR { @{$_[0][0]} = () }
         sub EXTEND {}
@@ -363,9 +364,7 @@ SKIP: {
     @real = @base;
     @real[0, 1] = @proxy[1, 0];
     is($real[0], "b", "tied right first");
-    { local $::TODO = "#126633";
     is($real[1], "a", "tied right second");
-    }
     @real = @base;
     @proxy[0, 1] = @proxy[1, 0];
     is($real[0], "b", "tied both first");
@@ -379,6 +378,10 @@ SKIP: {
     @real = @base;
     ($temp, @proxy) = @proxy[1, 0];
     is($real[0], "a", "scalar/array tied both");
+    $set = 0;
+    my $orig;
+    ($proxy[0], $orig) = (1, $set);
+    is($orig, 0, 'previous value of $set');
 }
 
 done_testing();
#####

Thank you very much.

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 14, 2017

From @tonycoz

On Sun, 12 Feb 2017 18​:11​:21 -0800, arza@​arza.us wrote​:

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Encode isn't needed​:

  ./perl -e 'sub foo { my ($x, $y) = @​_; } while (1) { foo("abc", substr("test", 1)) }'

produces the same leak, but​:

  ./perl -e 'while (1) { my ($x, $y) = (substr("test", 1), "abc") }'

doesn't leak which makes me suspect the lvalue created by pp_substr, though I haven't found the cause.

Tony

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 15, 2017

From arza@arza.us

On Mon, 13 Feb 2017 22​:32​:09 -0800, tonyc wrote​:

On Sun, 12 Feb 2017 18​:11​:21 -0800, arza@​arza.us wrote​:

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Encode isn't needed​:

./perl -e 'sub foo { my ($x, $y) = @​_; } while (1) { foo("abc",
substr("test", 1)) }'

produces the same leak, but​:

./perl -e 'while (1) { my ($x, $y) = (substr("test", 1), "abc") }'

doesn't leak which makes me suspect the lvalue created by pp_substr,
though I haven't found the cause.

Tony

The position matters​:

perl -e 'sub foo { my ($t1, $t2, $t3) = (shift, "", ""); } while (1) { foo(substr("test",1)); }' - no leak
perl -e 'sub foo { my ($t1, $t2, $t3) = ("", shift, ""); } while (1) { foo(substr("test",1)); }' - leak
perl -e 'sub foo { my ($t1, $t2, $t3) = ("", "", shift); } while (1) { foo(substr("test",1)); }' - leak

perl -e 'sub foo { my ($t1, $t2, $t3) = ("", "", shift(@​_)); } while (1) { foo(substr("test",1)); }' - no leak
perl -e 'sub foo { my ($t1, $t2, $t3) = ("", "", $_[0]); } while (1) { foo(substr("test",1)); }' - no leak
perl -e 'sub foo { my $t = join(" ", @​_); } while (1) { foo("abc", substr("test",1)); }' - no leak

-arza

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 15, 2017

From @iabyn

On Mon, Feb 13, 2017 at 10​:32​:10PM -0800, Tony Cook via RT wrote​:

On Sun, 12 Feb 2017 18​:11​:21 -0800, arza@​arza.us wrote​:

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Encode isn't needed​:

./perl -e 'sub foo { my ($x, $y) = @​_; } while (1) { foo("abc", substr("test", 1)) }'

produces the same leak, but​:

./perl -e 'while (1) { my ($x, $y) = (substr("test", 1), "abc") }'

doesn't leak which makes me suspect the lvalue created by pp_substr,
though I haven't found the cause.

Now fixed by the following, which I've just pushed. A good candidate for
back-porting to 5.24.x.

commit 1050723
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed Feb 15 15​:58​:24 2017 +0000
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Wed Feb 15 16​:08​:13 2017 +0000

  avoid a leak in list assign from/to magic values
 
  RT #130766
 
  A leak in list assignment was introduced by v5.23.6-89-gbeb08a1 and
  extended with v5.23.6-90-g5c1db56.
 
  Basically the code in S_aassign_copy_common() which does a mark-and-sweep
  looking for common vars by temporarily setting SVf_BREAK on LHS SVs then
  seeing if that flag was present on RHS vars, very temporarily removed that
  flag from the RHS SV while mortal copying it, then set it again. After
  those two commits, the "resetting" code could set SVf_BREAK on the RHS SV
  even when it hadn't been been present earlier.
 
  This meant that on exit from S_aassign_copy_common(), some SVs could be
  left with SVf_BREAK on. When that SV was freed, the SVf_BREAK flag meant
  that the SV head wasn't planted back in the arena (but PL_sv_count was
  still decremented). This could lead to slow growth of the SV HEAD arenas.
 
  The two circumstances that could trigger the leak were​:
 
  1) An SMG var on the LHS and a temporary on the RHS, e.g.
 
  use Tie​::Scalar;
  my ($s, $t);
  tie $s, 'Tie​::StdScalar'; # $s has set magic
  while (1) {
  ($s, $t) = ($t, map 1, 1, 2); # the map returns temporaries
  }
 
  2) A temporary on the RHS which has GMG, e.g.
 
  my $s = "abc";
  pos($s) = 1;
  local our ($x, $y);
  while (1) {
  my $pr = \pos($s); # creates a ref to a TEMP with get magic
  ($x, $y) = (1, $$pr);
  }
 
  Strictly speaking a TEMP isn't required for either case; just a situation
  where there's always a fresh SV on the RHS for each iteration that will
  soon get freed and thus leaked.
 
  This commit doesn't include any tests since I can't think of a way of
  testing it. svleak.t relies on PL_sv_count, which in this case doesn't
  show the leak.

--
Monto Blanco... scorchio!

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 15, 2017

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 16, 2017

From @jmdh

On Wed, Feb 15, 2017 at 08​:27​:15AM -0800, Dave Mitchell via RT wrote​:

On Mon, Feb 13, 2017 at 10​:32​:10PM -0800, Tony Cook via RT wrote​:

On Sun, 12 Feb 2017 18​:11​:21 -0800, arza@​arza.us wrote​:

Leaks memory​: perl -e 'use Encode; while (1) { encode("ascii",
substr("test",1)); }'
Doesn't leak memory​: perl -e 'use Encode; while (1) { encode("ascii",
lc(substr("test",1))); }'
Reproduced in 5.23.7, not in 5.23.6.

Encode isn't needed​:

./perl -e 'sub foo { my ($x, $y) = @​_; } while (1) { foo("abc", substr("test", 1)) }'

produces the same leak, but​:

./perl -e 'while (1) { my ($x, $y) = (substr("test", 1), "abc") }'

doesn't leak which makes me suspect the lvalue created by pp_substr,
though I haven't found the cause.

Now fixed by the following, which I've just pushed. A good candidate for
back-porting to 5.24.x.

I can confirm that this applies cleanly and fixes the leak on Debian's
5.24.1 build.

Cheers,
Dominic.

@p5pRT
Copy link
Author

@p5pRT 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 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