call_argv fails to call SAVETMP/FREETMP when creating mortal argument

[p120ph37]

Module: perl core

Description

When calling call_argv in the way shown in the perlembed documentation, the C string argument(s) are converted to mortal SVs and pushed onto the stack, however this operation is not wrapped in SAVETMP / FREETMP markers, so the mortal SVs are not garbage-collected. If this is done in an embedded perl interpreter, even with G_DISCARD enabled, the uncollected garbage will accrue indefinitely.

Steps to Reproduce

Compile and run this example and observe the differences in allocated memory for the scoped and raw call_argv loops for various iteration-counts.

#include <EXTERN.h>
#include <perl.h>
#include <malloc.h>
#include <stdio.h>

static PerlInterpreter *my_perl;

int main(int argc, char **argv, char **env) {
    char *perlargs[] = { "", "-e", "sub foo { 1; };", NULL };
    PERL_SYS_INIT3(&argc, &argv, &env);
    my_perl = perl_alloc();
    perl_construct(my_perl);
    PL_exit_flags |= PERL_EXIT_DESTRUCT_END;
    perl_parse(my_perl, NULL, 3, perlargs, NULL);
    perl_run(my_perl);

    struct mallinfo mem;
    long max = argc > 1 ? strtol(argv[1], NULL, 10) : 1000;
    char *fooargs[] = {"someval", NULL};

    mem = mallinfo();
    printf("Mem usage before any invocations of call_argv: %zu\n", mem.hblkhd + mem.uordblks);

    for(long i = 0; i < max; i++) {
        ENTER;
        SAVETMPS;
        call_argv("foo", G_DISCARD, fooargs);
        FREETMPS;
        LEAVE;
    }
    mem = mallinfo();
    printf("Mem usage after %lu scoped invocations of call_argv: %zu\n", max, mem.hblkhd + mem.uordblks);

    for(long i = 0; i < max; i++) {
        call_argv("foo", G_DISCARD, fooargs);
    }
    mem = mallinfo();
    printf("Mem usage after %lu bare invocations of call_argv: %zu\n", max, mem.hblkhd + mem.uordblks);

    perl_destruct(my_perl);
    perl_free(my_perl);
    PERL_SYS_TERM();
    exit(EXIT_SUCCESS);
}

Compiling, running, and output:

$ cc -o call_argv_mem call_argv_mem.c `perl -MExtUtils::Embed -e ccopts -e ldopts`
$ ./call_argv_mem 10000
Mem usage before any invocations of call_argv: 180848
Mem usage after 10000 scoped invocations of call_argv: 182384
Mem usage after 10000 bare invocations of call_argv: 986000
$ ./call_argv_mem 20000
Mem usage before any invocations of call_argv: 180848
Mem usage after 20000 scoped invocations of call_argv: 182384
Mem usage after 20000 bare invocations of call_argv: 1793360

Expected behavior

I expect garbage-collection to recover the mortal SVs so that the program can continue to invoke call_argv indefinitely without increasing memory usage.

Perl configuration

perl -V
Summary of my perl5 (revision 5 version 28 subversion 1) configuration:

  Platform:
    osname=linux
    osvers=4.9.0
    archname=x86_64-linux-gnu-thread-multi
    uname='linux localhost 4.9.0 #1 smp debian 4.9.0 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dcc=x86_64-linux-gnu-gcc -Dcpp=x86_64-linux-gnu-cpp -Dld=x86_64-linux-gnu-gcc -Dccflags=-DDEBIAN -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/perl-voFw8F/perl-5.28.1=. -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.28 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.28 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.28 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.28.1 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.28.1 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Ui_xlocale -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.28.1'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='x86_64-linux-gnu-gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2 -g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion=''
    gccversion='8.3.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='x86_64-linux-gnu-gcc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/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
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=libc-2.28.so
    so=so
    useshrplib=true
    libperl=libperl.so.5.28
    gnulibc_version='2.28'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -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
  Locally applied patches:
    DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
    DEBPKG:debian/db_file_ver - https://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
    DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
    DEBPKG:debian/enc2xs_inc - https://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories.
    DEBPKG:debian/errno_ver - https://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
    DEBPKG:debian/libperl_embed_doc - https://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
    DEBPKG:fixes/respect_umask - Respect umask during installation
    DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories
    DEBPKG:debian/extutils_set_libperl_path - EU:MM: set location of libperl.a under /usr/lib
    DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
    DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
    DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
    DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
    DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
    DEBPKG:debian/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
    DEBPKG:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
    DEBPKG:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.28.1-6+deb10u1 in patchlevel.h
    DEBPKG:fixes/document_makemaker_ccflags - https://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
    DEBPKG:debian/find_html2text - https://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text
    DEBPKG:debian/perl5db-x-terminal-emulator.patch - https://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
    DEBPKG:debian/cpan-missing-site-dirs - https://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
    DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] https://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected
    DEBPKG:debian/makemaker-pasthru - https://bugs.debian.org/758471 Pass LD settings through to subdirectories
    DEBPKG:debian/makemaker-manext - https://bugs.debian.org/247370 Make EU::MakeMaker honour MANnEXT settings in generated manpage headers
    DEBPKG:debian/kfreebsd-softupdates - https://bugs.debian.org/796798 Work around Debian Bug#796798
    DEBPKG:fixes/autodie-scope - https://bugs.debian.org/798096 Fix a scoping issue with "no autodie" and the "system" sub
    DEBPKG:fixes/memoize-pod - [rt.cpan.org #89441] Fix POD errors in Memoize
    DEBPKG:debian/hurd-softupdates - https://bugs.debian.org/822735 Fix t/op/stat.t failures on hurd
    DEBPKG:fixes/math_complex_doc_great_circle - https://bugs.debian.org/697567 [rt.cpan.org #114104] Math::Trig: clarify definition of great_circle_midpoint
    DEBPKG:fixes/math_complex_doc_see_also - https://bugs.debian.org/697568 [rt.cpan.org #114105] Math::Trig: add missing SEE ALSO
    DEBPKG:fixes/math_complex_doc_angle_units - https://bugs.debian.org/731505 [rt.cpan.org #114106] Math::Trig: document angle units
    DEBPKG:fixes/cpan_web_link - https://bugs.debian.org/367291 CPAN: Add link to main CPAN web site
    DEBPKG:debian/hppa_op_optimize_workaround - https://bugs.debian.org/838613 Temporarily lower the optimization of op.c on hppa due to gcc-6 problems
    DEBPKG:debian/installman-utf8 - https://bugs.debian.org/840211 Generate man pages with UTF-8 characters
    DEBPKG:fixes/getopt-long-4 - https://bugs.debian.org/864544 [rt.cpan.org #122068] Fix issue #122068.
    DEBPKG:debian/hppa_opmini_optimize_workaround - https://bugs.debian.org/869122 Lower the optimization level of opmini.c on hppa
    DEBPKG:debian/sh4_op_optimize_workaround - https://bugs.debian.org/869373 Also lower the optimization level of op.c and opmini.c on sh4
    DEBPKG:debian/perldoc-pager - https://bugs.debian.org/870340 [rt.cpan.org #120229] Fix perldoc terminal escapes when sensible-pager is less
    DEBPKG:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
    DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian
    DEBPKG:debian/configure-regen - https://bugs.debian.org/762638 Regenerate Configure et al. after probe unit changes
    DEBPKG:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
    DEBPKG:debian/disable-stack-check - https://bugs.debian.org/902779 [perl #133327] Disable debugperl stack extension checks for binary compatibility with perl
    DEBPKG:debian/gdbm-fatal - [perl #133295] https://bugs.debian.org/904005 Temporarily skip GDBM_File fatal.t for gdbm >= 1.15 compatibility
    DEBPKG:fixes/storable-recursion - https://bugs.debian.org/912900 [perl #133326] [120060c] (perl #133326) fix and clarify handling of recurs_sv.
    DEBPKG:fixes/caretx-fallback - https://bugs.debian.org/913347 [perl #133573] [03b94aa] RT#133573: $^X fallback when platform-specific technique fails
    DEBPKG:fixes/eumm-usrmerge - https://bugs.debian.org/913637 Avoid mangling /bin non-perl shebangs on merged-/usr systems
    DEBPKG:fixes/errno-include-path - [6c5080f] [perl #133662] https://bugs.debian.org/875921 Make Errno_pm.PL compatible with /usr/include/<ARCH>/errno.h
    DEBPKG:fixes/kfreebsd-renameat - [a3c63a9] https://bugs.debian.org/912521 [perl #133668] Also work around renameat() kernel bug on GNU/kFreeBSD
    DEBPKG:fixes/time-local-2020 - https://bugs.debian.org/915209 [rt.cpan.org #124787] Fix Time::Local tests
    DEBPKG:fixes/inplace-editing-bugfix/part1 - https://bugs.debian.org/914651 (perl #133659) move argvout cleanup to a new function
    DEBPKG:fixes/inplace-editing-bugfix/part2 - https://bugs.debian.org/914651 (perl #133659) tests for global destruction handling of inplace editing
    DEBPKG:fixes/inplace-editing-bugfix/part3 - https://bugs.debian.org/914651 (perl #133659) make an in-place edit successful if the exit status is zero
    DEBPKG:fixes/fix-manifest-failures - https://bugs.debian.org/914962 Fix t/porting/manifest.t failures when run in a foreign git checkout
    DEBPKG:fixes/pipe-open-bugfix/part1 - [perl #133726] https://bugs.debian.org/916313 Always mark pipe in pipe-open as inherit-on-exec
    DEBPKG:fixes/pipe-open-bugfix/part2 - [perl #133726] https://bugs.debian.org/916313 Always mark pipe in list pipe-open as inherit-on-exec
    DEBPKG:fixes/storable-probing/prereq1 - [3f4cad1] Storable: fix for strawberry build failures:
    DEBPKG:fixes/storable-probing/prereq2 - [perl #133411] [edf639f] (perl #133411) don't try to load Storable with -Dusecrosscompile
    DEBPKG:fixes/storable-probing/disable-probing - https://bugs.debian.org/914133 [perl #133708] [2a0bbd3] (perl #133708) remove build-time probing for stack limits for Storable
    DEBPKG:debian/perlbug-editor - https://bugs.debian.org/922609 Use "editor" as the default perlbug editor, as per Debian policy
    DEBPKG:fixes/posix-mbrlen - [25d7b7a] https://bugs.debian.org/924517 [perl #133928] Fix POSIX::mblen mbstate_t initialization on threaded perls with glibc
    DEBPKG:fixes/CVE-2020-10543 - https://bugs.debian.org/962005 regcomp.c: Prevent integer overflow from nested regex quantifiers.
    DEBPKG:fixes/CVE-2020-10878 - https://bugs.debian.org/962005 study_chunk: extract rck_elide_nothing
    DEBPKG:fixes/CVE-2020-12723 - https://bugs.debian.org/962005 study_chunk: avoid mutating regexp program within GOSUB
    DEBPKG:fixes/io-socket-ip-nov4 - https://bugs.debian.org/962019 Fix test failures in IO::Socket::IP with an IPv6-only host
  Built under linux
  Compiled at Jul 21 2020 19:27:00
  @INC:
    /etc/perl
    /usr/local/lib/x86_64-linux-gnu/perl/5.28.1
    /usr/local/share/perl/5.28.1
    /usr/lib/x86_64-linux-gnu/perl5/5.28
    /usr/share/perl5
    /usr/lib/x86_64-linux-gnu/perl/5.28
    /usr/share/perl/5.28
    /usr/local/lib/site_perl
    /usr/lib/x86_64-linux-gnu/perl-base

[Leont]

Yeah that should honor G_DISCARD. It currently doesn't because G_DISCARD is only implemented for call_sv, which is called by call_argv. It shouldn't be hard to fix this, just a bit ugly. Using a refcounted stack (new in 5.40) should also fix this.

[p120ph37]

True, a reference-counted stack should solve the issue in the future, but there is a lot of existing third-party code out there that isn't going to be rc-aware any time soon, so this should either be addressed for non-rc stacks too, or (less-desirably) the docs should be updated to explicitly say that G_DISCARD alone is insufficient for call_argv cleanup if there are any input arguments present...