search

Perl / perl5

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

Segmentation Fault provoqued by infinite recursion in double quote / stringification overloading #19559

Open florian-pe opened 2 years ago

florian-pe commented 2 years ago

Hi,

I just found out about a Segmentation Fault. This is my golfed version of the bug : 

perl -E 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'

The infinite recursion call happens when trying to print the arguments on the anonymous subroutine used to overload the double quotes operator.

In the module I am writing, I also got this error message Deep recursion on anonymous subroutine but it doesn't appear in the golfed replicate.

And this is my perl build : 

Summary of my perl5 (revision 5 version 34 subversion 0) configuration:

  Platform:
    osname=linux
    osvers=5.12.15-arch1-1
    archname=x86_64-linux-thread-multi
    uname='archlinux'
    config_args='-des -Dusethreads -Duseshrplib -Doptimize=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -Dprefix=/usr -Dvendorprefix=/usr -Dprivlib=/usr/share/perl5/core_perl -Darchlib=/usr/lib/perl5/5.34/core_perl -Dsitelib=/usr/share/perl5/site_perl -Dsitearch=/usr/lib/perl5/5.34/site_perl -Dvendorlib=/usr/share/perl5/vendor_perl -Dvendorarch=/usr/lib/perl5/5.34/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,-z,now -Dldflags=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  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 -D_FORTIFY_SOURCE=2'
    optimize='-march=x86-64 -mtune=generic -O2 -pipe -fno-plt'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.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='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,-z,now -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib
    libs=-lpthread -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.33.so
    so=so
    useshrplib=true
    libperl=libperl.so
    gnulibc_version='2.33'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib/perl5/5.34/core_perl/CORE'
    cccdlflags='-fPIC'
    lddlflags='-shared -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl): 
  Compile-time options:
    HAS_TIMES
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_IMPLICIT_CONTEXT
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_REENTRANT_API
    USE_THREAD_SAFE_LOCALE
  Built under linux
  Compiled at Nov 13 2021 20:22:42
  %ENV:
    PERL5LIB="/home/london/.my_configurations/perl_modules"
  @INC:
    /home/london/.my_configurations/perl_modules
    /usr/lib/perl5/5.34/site_perl
    /usr/share/perl5/site_perl
    /usr/lib/perl5/5.34/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib/perl5/5.34/core_perl
    /usr/share/perl5/core_perl
jkeenan commented 2 years ago

Hi,

I just found out about a Segmentation Fault. This is my golfed version of the bug :

perl -E 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'

Not a new problem, though I don't know whether it's been reported previously or not.

$ perl -E 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'
Segmentation fault (core dumped)

$ perlbrew use perl-5.20.3
$ perl -E 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'
Segmentation fault (core dumped)

$ perlbrew use perl-5.10.1
$ perl -E 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'
Segmentation fault (core dumped)

$ perlbrew use perl-5.6.2
$ perl -e 'package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { print join ("|", @_), "\n" }; print "$var\n"'
Segmentation fault (core dumped)
Leont commented 2 years ago

Yeah, I'm pretty sure this is both known and unfixable. Ultimately infinite recursion isn't finite.

florian-pe commented 2 years ago

@Leont Is it impossible, even in theory, to put a special conditional in the interpreter to deal with this ?

My first idea is that, infinite recursion is a special case of recursion. And recursive call should be identifiable. And in this particular case of infinite recursion, the difference with legitimate recursion would be that some of the arguments are guaranteed to be passed unchanged.

hvds commented 2 years ago

@Leont Is it impossible, even in theory, to put a special conditional in the interpreter to deal with this ?

My first idea is that, infinite recursion is a special case of recursion. And recursive call should be identifiable. And in this particular case of infinite recursion, the difference with legitimate recursion would be that some of the arguments are guaranteed to be passed unchanged.

I don't think we'd want to put a special-case in for this - it would not be appropriate to suppress the overriding function for the duration of the call, the way we do for things like $SIG{__DIE__}. And there's nothing that requires a stringification overload to return the same value for the same input, so even if we could detect that I'm not convinced we should:

perl -E 'use overload q{""} => sub { return $count++ & 1 ? "double the fun" : "$_[0]" };
  say "" . bless {}'

As @Leont implies, it is the responsibility of the programmer not to recurse without providing termination conditions. The "deep recursion" warning is there precisely so you have a clue why you got a segfault.

If you don't get the "deep recursion" warning that might be a bug - but make sure you actually have warnings turned on. (I find that when writing one-liners I always forget that -E doesn't turn on warnings.)

tonycoz commented 1 year ago

You do get the "Deep recursion" warning with warnings on:

$ ./perl -Ilib -E 'use warnings; package mypack; our $var = bless {}, "mypack"; use overload q{""} => sub { say join "|", @_ }; say $var'
Deep recursion on anonymous subroutine at -e line 1.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1257458==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe25a87e28 (pc 0x55739cede582 bp 0x7ffe25a88370 sp 0x7ffe25a87e10 T0)
    #0 0x55739cede582 in Perl_Gv_AMupdate /home/tony/dev/perl/git/perl6/gv.c:3141
    #1 0x55739cee98ce in Perl_amagic_call /home/tony/dev/perl/git/perl6/gv.c:3743
    #2 0x55739d3cbca7 in Perl_sv_2pv_flags /home/tony/dev/perl/git/perl6/sv.c:2880
    #3 0x55739d3a5d35 in Perl_SvPV_helper /home/tony/dev/perl/git/perl6/sv_inline.h:957
...
    #243 0x55739d636b46 in Perl_do_join /home/tony/dev/perl/git/perl6/doop.c:699
    #244 0x55739d35c7eb in Perl_pp_join /home/tony/dev/perl/git/perl6/pp_hot.c:1760
    #245 0x55739d266e2a in Perl_runops_debug /home/tony/dev/perl/git/perl6/dump.c:2861
    #246 0x55739cef1f7d in Perl_amagic_call /home/tony/dev/perl/git/perl6/gv.c:4146
    #247 0x55739d3cbca7 in Perl_sv_2pv_flags /home/tony/dev/perl/git/perl6/sv.c:2880
    #248 0x55739d3a5d35 in Perl_SvPV_helper /home/tony/dev/perl/git/perl6/sv_inline.h:957

SUMMARY: AddressSanitizer: stack-overflow /home/tony/dev/perl/git/perl6/gv.c:3141 in Perl_Gv_AMupdate
==1257458==ABORTING

I don't think there's a bug here.

demerphq commented 1 year ago

@tonycoz id say we should keep this bug report as we shouldn't segfault, which we do only because these overloads involve the C stack.

tonycoz commented 1 year ago

We've closed similar issues in the past.

In this case I don't think there's a practical fix for it and it should be WONTFIX, keeping it open would just be noise.

I can see a couple of approaches, neither really seems really practical:

OPs handling of overloads require a callback approach

Let's say I have an op that takes two string parameters, for an overloaded parameter the pp func would need to return to the event loop with another OP that behaves like pp_entersub, calling any overload implementation method, and use some mechanism to continue where it left off.

There would need to be some mechanism to store any in progress state of the OP (like the current state of the sort in pp_sort, which is a hairy case)

This is all really hairy.

OP arguments are pre-overloaded

Each argument that wants a particular type (PV, IV etc) has an OP generated that casts the argument to that type, so instead of:

pushmark padsv $x padsv $y print

we'd have:

pushmark padsv $x castpv padsv $y castpv print

This won't work for some OPs, like printf() or pack(), since the argument types depend on their format arguments.

Neither approach works for XSUBs. Both approaches will reduce performance, even for non-overloaded arguments.

iabyn commented 1 year ago

On Thu, Jun 15, 2023 at 05:00:27PM -0700, Tony Cook wrote:

We've closed similar issues in the past.

In this case I don't think there's a practical fix for it and it should be WONTFIX, keeping it open would just be noise.

I can see a couple of approaches, neither really seems really practical:

Also, neither of them would work for the similar recursive tie method bug, as tied FETCH()s etc can be called from just about anywhere where the value of an SV is retrieved.

A hacky "fix" would be to record recursion depth (i.e. how many times call_sv() or similar has been called) and croak on some arbitrary limit, on the grounds that a croak with a meaningful error message is better than a SEGV.

Choosing a limit is of course hard - should it be user-settable, and should the default depend on the platform, and whether the code is being run within a thread, and so on.

-- Spock (or Data) is fired from his high-ranking position for not being able to understand the most basic nuances of about one in three sentences that anyone says to him. -- Things That Never Happen in "Star Trek" #19

demerphq commented 1 year ago

A hacky "fix" would be to record recursion depth (i.e. how many times call_sv() or similar has been called) and croak on some arbitrary limit,

This is what i was thinking. My intent wasn't that this shouldn't throw an error, it was that it shouldn't segv when that error occurs.