Perl / perl5

馃惇 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

recursion in overload sub results in segfault #16333

Open p5pRT opened 6 years ago

p5pRT commented 6 years ago

Migrated from rt.perl.org#132638 (status was 'open')

Searchable as RT132638$

p5pRT commented 6 years ago

From data@cpan.org

Created by data@cpan.org

Hi\,

i've run accross a segmentation fault with the following script​:

package Clock; use 5.010; use strict;

use overload '+' => \&add_time\, '-' => \&sub_time\, 'eq' => \&equals_to\, '""' => \&to_string;

sub equals_to{ $_[0] eq $_[1] } sub to_string { sprintf '%02d​:%02d'\, $_[0][0] // 0\, $_[0][1] // 0; } sub new { bless( $_[1]\, $_[0] ); }

package main; Clock->new([8\,0]) eq "08​:00" __END__

The SIGSEGV goes away when I change equals_to to​:

  sub equals_to{ $_[0]->to_string eq $_[1] }

bye\,

Danijel

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.26.1: Configured by danielt at Thu Dec 21 07:13:42 CET 2017. Summary of my perl5 (revision 5 version 26 subversion 1) configuration: Platform: osname=linux osvers=4.9.57 archname=x86_64-linux uname='linux banjo.rbfh.de 4.9.57 #1 smp sat oct 21 15:46:05 cest 2017 x86_64 gnulinux ' config_args='-de -Dprefix=/opt/perl/perls/perl-5.26.1 -Aeval:scriptdir=/opt/perl/perls/perl-5.26.1/bin' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='' gccversion='4.7.2' 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 =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.7/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 -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc= so=so useshrplib=false libperl=libperl.a gnulibc_version='2.13' 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' @INC for perl 5.26.1: /opt/perl/perls/perl-5.26.1/lib/site_perl/5.26.1/x86_64-linux /opt/perl/perls/perl-5.26.1/lib/site_perl/5.26.1 /opt/perl/perls/perl-5.26.1/lib/5.26.1/x86_64-linux /opt/perl/perls/perl-5.26.1/lib/5.26.1 Environment for perl 5.26.1: HOME=/home/danielt LANG=C LANGUAGE=C LC_CTYPE=de_DE.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/opt/perl/bin:/opt/perl/perls/perl-5.26.1/bin:/usr/local/go/bin:/home/danielt/.nvm/v8.1.4/bin:/opt/python/2_007_008/bin:/opt/aperl/bin:/home/danielt/bin:/home/danielt/unix/bin:/usr/opt/bin:/usr/local/bin:/usr/local/sbin:/usr/X11R6/bin/xscreensaver-hacks/:/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/bin:/usr/X11R6/bin:/home/danielt/bin:/usr/pkg/bin:/usr/pkg/sbin:/home/danielt/.rvm/bin PERLBREW_BASHRC_VERSION=0.73 PERLBREW_HOME=/home/danielt/.perlbrew PERLBREW_MANPATH=/opt/perl/perls/perl-5.26.1/man PERLBREW_PATH=/opt/perl/bin:/opt/perl/perls/perl-5.26.1/bin PERLBREW_PERL=perl-5.26.1 PERLBREW_ROOT=/opt/perl PERLBREW_SHELLRC_VERSION=0.82 PERLBREW_VERSION=0.82 PERL_BADLANG (unset) PERL_CPANM_OPT=--skip-installed --prompt --mirror http://cpan.cpantesters.org SHELL=/bin/zsh ```
p5pRT commented 6 years ago

From @jkeenan

On Thu\, 21 Dec 2017 09​:52​:59 GMT\, DaTa wrote​:

This is a bug report for perl from data@​cpan.org\, generated with the help of perlbug 1.40 running under perl 5.26.1.

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

Hi\,

i've run accross a segmentation fault with the following script​:

package Clock; use 5.010; use strict;

use overload '+' => \&add_time\, '-' => \&sub_time\, 'eq' => \&equals_to\, '""' => \&to_string;

sub equals_to{ $_[0] eq $_[1] } sub to_string { sprintf '%02d​:%02d'\, $_[0][0] // 0\, $_[0][1] // 0; } sub new { bless( $_[1]\, $_[0] ); }

package main; Clock->new([8\,0]) eq "08​:00" __END__

The SIGSEGV goes away when I change equals_to to​:

sub equals_to{ $_[0]->to_string eq $_[1] }

bye\,

Danijel

Confirmed. Note that the problem is older than the perl-5.010 needed to make use of the '//' operator in sub to_string(). If we switch those to '||'\, we can reproduce the segfault going back at least to perl 5.6. See attached.

Thank you very much.

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

p5pRT commented 6 years ago

From @jkeenan

132638-double-bar.pl

p5pRT commented 6 years ago

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

p5pRT commented 6 years ago

From zefram@fysh.org

Danijel Tasov wrote​:

use overload 'eq' => \&equals_to\, ... sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to its operands\, which invokes the "eq" overload in an infinite recursion. The segv is just C stack overflow. You probably want to write the "eq" overload as

  sub equals_to{ "$_[0]" eq $_[1] }

or just remove that overload and enable overload fallback\, so that Perl will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

p5pRT commented 6 years ago

From @jkeenan

On Sat\, 23 Dec 2017 07​:56​:23 GMT\, zefram@​fysh.org wrote​:

Danijel Tasov wrote​:

use overload 'eq' => \&equals_to\, ... sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to its operands\, which invokes the "eq" overload in an infinite recursion. The segv is just C stack overflow. You probably want to write the "eq" overload as

sub equals\_to\{ "$\_\[0\]" eq $\_\[1\] \}

or just remove that overload and enable overload fallback\, so that Perl will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

There may not be a bug in perl here -- but there's certainly a deficiency in our documentation.

I've been writing Perl for a long time but have never had occasion to 'use overload'. I stared at the original poster's code and couldn't see anything obviously wrong. I then went and read the documentation. Again\, I couldn't find anything that would lead me to guess that the OP's code would segfault.

So we have to improve the documentation. Otherwise\, people will repeatedly stumble into this problem. How should we change the docs?

Thank you very much.

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

p5pRT commented 6 years ago

From @eserte

"James E Keenan via RT" \perlbug\-followup@​perl\.org writes​:

On Sat\, 23 Dec 2017 07​:56​:23 GMT\, zefram@​fysh.org wrote​:

Danijel Tasov wrote​:

use overload 'eq' => \&equals_to\, ... sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to its operands\, which invokes the "eq" overload in an infinite recursion. The segv is just C stack overflow. You probably want to write the "eq" overload as

sub equals\_to\{ "$\_\[0\]" eq $\_\[1\] \}

or just remove that overload and enable overload fallback\, so that Perl will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

There may not be a bug in perl here -- but there's certainly a deficiency in our documentation.

I've been writing Perl for a long time but have never had occasion to 'use overload'. I stared at the original poster's code and couldn't see anything obviously wrong. I then went and read the documentation. Again\, I couldn't find anything that would lead me to guess that the OP's code would segfault.

So we have to improve the documentation. Otherwise\, people will repeatedly stumble into this problem. How should we change the docs?

With "use warnings" the script emits

  Useless use of string eq in void context at ...   Deep recursion on subroutine "Clock​::equals_to" at ...

At least the "deep recursion" warning should be a note to the experienced programmer that something's going wrong here\, and out of memory errors and stack overflows are to be expected.

But maybe a sentence in perldiag.pod could be added.

Regards\,   Slaven

-- Slaven Rezic - slaven \ rezic \ de

  Berlin Perl Mongers - http​://berlin.pm.org

p5pRT commented 6 years ago

From gm@qwurx.de

From the keyboard of Zefram [23.12.17\,07​:56]​:

Danijel Tasov wrote​:

use overload 'eq' => \&equals_to\, ... sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to its operands\, which invokes the "eq" overload in an infinite recursion.

That's right; on my machine that segfaults at recursion 26186\, gdb bt shows 104775 stack frames.

The segv is just C stack overflow. You probably want to write the "eq" overload as

sub equals_to{ "$_[0]" eq $_[1] }

or just remove that overload and enable overload fallback\, so that Perl will synthesise that "eq" overload for you.

There is no Perl bug here.

You're probably right here\, but​: Why should a buffer overflow be a bug but a stack overflow should not?

I guess this could be handled more gracefully by determining the stack limit before runtime and die with an appropriate message\, instead of running into a SIGSEGV.

Is it feasible to implement an overloading short-circuit detection?

best regards\, 0--gg-

-- _($_=" "x(1\<\<5)."?\n".q路/)Oo. G掳\ /   /\_炉/(q / ---------------------------- \__(m.====路.(_("always off the crowd"))."路 ");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}

p5pRT commented 6 years ago

From zefram@fysh.org

shmem wrote​:

Why should a buffer overflow be a bug but a stack overflow should not?

For two reasons. Firstly\, buffer overflow is always an implementation mistake​: Perl doesn't expose fixed-size buffers that the user can overflow. Whereas stack overflow is a natural result of the Perl program​: it is the user that coded an infinite recursion. Secondly\, buffer overflow can cause all kinds of erroneous behaviour\, often exploitable to defeat security measures. Whereas stack overflow just causes a segv that terminates the program.

It would be nicer if deep recursion that currently occurs on the C stack were to be able to use more of the available memory\, and eventually fail in some slightly cleaner way that cites lack of memory. Simple cases of Perl recursion do avoid the C stack and get such behaviour. But it would be quite infeasible to avoid C stack recursion in all cases\, and very difficult to give C stack recursion nicer behaviour. So we have made a tacit design decision that we put up with the limited C stack size and segv as the result of overflowing it\, and thus we let that be the behaviour we offer to Perl programs.

-zefram

p5pRT commented 6 years ago

From @cpansprout

On Sat\, 23 Dec 2017 07​:42​:52 -0800\, zefram@​fysh.org wrote​:

So we have made a tacit design decision that we put up with the limited C stack size and segv as the result of overflowing it\, and thus we let that be the behaviour we offer to Perl programs.

This is the first I鈥檝e heard of it being a design decision\, rather than an unfortunate unfixable flaw.

--

Father Chrysostomos

p5pRT commented 6 years ago

From gm@qwurx.de

From the keyboard of Zefram [23.12.17\,15​:42]​:

shmem wrote​:

Why should a buffer overflow be a bug but a stack overflow should not?

For two reasons. Firstly\, buffer overflow is always an implementation mistake​: Perl doesn't expose fixed-size buffers that the user can overflow. Whereas stack overflow is a natural result of the Perl program​: it is the user that coded an infinite recursion. Secondly\, buffer overflow can cause all kinds of erroneous behaviour\, often exploitable to defeat security measures. Whereas stack overflow just causes a segv that terminates the program.

Arguably the buffer overflow is an implementation mistake of the C language implementation\, which is known and must carefully be avoided. C places the burden on the programmer here\, as it does with stack overflow. Perl has made sure buffer overflows don't occur\, whereas wrt stack overflows\, perl doesn't care & passes the burden to the perl user.

It would be nicer if deep recursion that currently occurs on the C stack were to be able to use more of the available memory\, and eventually fail in some slightly cleaner way that cites lack of memory. Simple cases of Perl recursion do avoid the C stack and get such behaviour. But it would be quite infeasible to avoid C stack recursion in all cases\, and very difficult to give C stack recursion nicer behaviour. So we have made a tacit design decision that we put up with the limited C stack size and segv as the result of overflowing it\, and thus we let that be the behaviour we offer to Perl programs.

It is not a design decision of perl\, because it is not perl which emits the SIGSEGV\, it is the kernel - at least on Linux\, and I bet it is no different with other unixish sytems. The C stack size depends on the current ulimit values which - again\, on Linux - can be retrieved with the getrlimit(2) syscall\, and the current stack size can be obtained via getrusage(2)\, if I am not mistaken altogether.

I know far too little of the perl internals to be able to tell at which conditions a stack check along the lines of the above would be nice.

An alternative would be to set up an alternate signal stack as described in getrlimit(2) and sigaltstack(2) to be able to mask SIGSEGV and make an educated guess about where the condition occured\, which could lead to better error messages whenever a SIGSEGV is triggered.

The current stack size limit is a perl runtime condition. If I run the short-circuited overload code in a root shell having set "ulimit -s unlimited"\, I can sit and wait until the C stack has gobbled up all memory (and swap) until the kernel finally emits an emergency kill (or not\, I didn't wait).

Running it with my current ulimit settings (-s being 8192 kbytes) it segfaults at recursion 26196; if I decrease the stack size limit to 64 kbytes\, it is blown off at recursion 171.

The simple recursion 'sub r{&r} r' instead seems to make the heap grow\, which is much harder to handle wrt limits.

Does that make sense?

best regards\, 0--gg-

-- _($_=" "x(1\<\<5)."?\n".q路/)Oo. G掳\ /   /\_炉/(q / ---------------------------- \__(m.====路.(_("always off the crowd"))."路 ");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}