Collection of issues related to new procedure for disabling 'taint', including 5.36 blockers.

[demerphq]

Description
The new procedure for disabling taint support has several issues, some of which are merely annoying technical debt, some of which are show-stoppers.

Background: The logic to support taint mode, even when taint mode is not enabled, impose a non-trivial performance burden on all perl code executed. Conservative estimates for disabling it at compile time are a 3%-5% speedup, but some workloads may see significantly more performance win. In Perl 5.18 options were provided to disable taint support at compile time, the 'NO_TAINT_SUPPORT' define and the 'SILENT_NO_TAINT_SUPPORT' define. Enabling either removes taint related logic from the compile process, NO_TAINT_SUPPORT results in a perl which will throw a fatal exception when the -t option is used to enable taint, and SILENT_NO_TAINT_SUPPORT builds a perl which silently ignores the -t switch, so scripts previously using it will continue to run, albeit without taint protections.

In PR #19541 a new procedure was added to control these features. This involved adding support for a 'taint_support' fake define which can be used with Configure via -Utaint_support to "undefine" this fake define. This then defines 'SILENT_NO_TAINT_SUPPORT'. There does not seem to be a way to enable the safer 'NO_TAINT_SUPPORT' mode other than the "old way".

1. Using the old way of -Accflags='-DNO_TAINT_SUPPORT' or -Accglags='-DSILENT_NO_TAINT_SUPPORT' results in a perl whose %Config is incorrect and which states that $Config{taint_support}='define', even though that is not true. This is definitely a show-stopper.
2. There is no documentation of 'taint_support' in INSTALL. If this is an "official" setting then INSTALL should include documentation. It is minimally documented in perldelta. This is not a show-stopper but is unfortunate, arguably the new feature is incomplete without it.
3. The new method does not provide way to enable the safer "NO_TAINT_SUPPORT". I think this is a show stopper.
4. perl -V does not show this setting. This is not a show stopper as the NO_TAINT_SUPPORT and SILENT_NO_TAINT_SUPPORT defines will be shown in the define setting, but I think it is unfortunate and unhelpful.
5. The new settings is true when taint is enabled. This is problematic as in older Perl's the setting did not exist. Thus $Config{taint_support} has two false modes which means the opposite of each other. If $Config{taint_support} is undefined (because it does not exist) it means the same thing as when taint support is true. This is not a show stopper, but it is very awkward, and I consider it basically to be technical debt.
6. Because this setting uses 'true' to specify that taint_support is enabled it provides no natural way to indicate the difference between SILENT_NO_TAINT_SUPPORT and NO_TAINT_SUPPORT. This is not a show stopper, but it is very awkward, and I consider it basically to be technical debt
7. The documentation for NO_TAINT_SUPPORT and SILENT_NO_TAINT_SUPPORT in perl.h says

 * DANGER! Using NO_TAINT_SUPPORT or SILENT_NO_TAINT_SUPPORT
 *         voids your nonexistent warranty!

if we are offering this as a Configure option this language is unfortunate. This is not a show stopper but indicates the most recent change is incomplete.

I believe that ultimately it was a mistake that we will come to regret that we created this "fake" reverse polarity define "taint_support" for this feature. It produces several awkward outcomes that I believe we will come to regret. I believe very strongly that the new define should be "no_taint_support" and it should have two values, "silent" or "define". That would be the most backwards compatible option, and introduce the least confusion. We will pay the price of this reverse polarity fake define for the rest of the history of perl if we release as is.

I know this is frustrating to hear after it has been merged, and I am truly sorry for that, but for what it is worth I actually tried to raise some of the issues mentioned in this ticket in response to the original PR before it was merged, but I did so via email reply and unbeknownst to me it was not added to the ticket (github "ate" the mail) so no-one else actually saw it. I though my objections were actually being ignored, and was growing frustrated about it, but did not want to kick up a stink as I do think that in principal this new configure functionality is a good thing. So it was merged even though I had raised serious misgivings about the implementation choice of "taint_support".

I think this ticket raises larger issues frankly. Configure thinks it is in charge of the configuration process, but it really isn't. The sole and only keeper of the truth about our defines is the C code itself. I believe long term we need to get Configure about of the business of constructing the %Config hash and move it all into the build process itself, so that the C code generates it during the build process absolutely truthfully regardless of how a setting is set. I will raise a separate ticket for this discussion however, it is a digression from the immediate issues of this ticket.

Steps to Reproduce
The biggest issue is that $Config{taint_support} can end up being 'define' when in fact taint support has been disabled by using the "old way" of configuring it:

Configure perl with -Accflags='-DNO_TAINT_SUPPORT' or -Accflags='-DSILENT_NO_TAINT_SUPPORT'

Execute: ./perl -Ilib -MConfig -le'print $Config{taint_support}' you will see it print out 'define', this is wrong.
You can also do ./perl -Ilib -V:taint_support, you will see:

taint_support='define';

Grep ./perl -Ilib -V case-insensitively and you will not see anything about 'taint_support' but you will see NO_TAINT_SUPPORT or it and SILENT_NO_TAINT_SUPPORT depending on which you configured with:

./perl -Ilib -V | grep -i taint_support
    config_args='-Dusethreads -Doptimize=-g -d -Dusedevel -Dcc=ccache gcc -Dld=gcc -DDEBUGGING -Accflags=-DSILENT_NO_TAINT_SUPPORT'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -DSILENT_NO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DSILENT_NO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    NO_TAINT_SUPPORT
    SILENT_NO_TAINT_SUPPORT

Expected behavior

1. Assuming we keep 'taint_support' as a setting (see point 6 below) I expect that $Config{taint_support} to be false when perl was configured with either -Accflags='-DNO_TAINT_SUPPORT' or -Accflags='-DSILENT_NO_TAINT_SUPPORT'
2. I do not expect perl to lie to me about a setting that does not exist being defined.
3. I expect perl -V to show an important property like '$Config{taint_support}', yes it is visible in the list of defines under its true names, but that is not what users who use -Utaint_support will expect to be looking for.
4. I expect to be able to distinguish SILENT_NO_TAINT_SUPPORT from NO_TAINT_SUPPORT in %Config. The only way to determine if a perl is compiled with NO_TAINT_SUPPORT when it is not already running under -T is to execute a perl script with -T and see if it dies with a message about not supporting taint.
5. I expect Configure to give me an option to choose the less "surprising" mode of 'NO_TAINT_SUPPORT' versus the more convenient but potentially surprising 'SILENT_NO_TAINT_SUPPORT'.
6. I actually expect there to be a 'no_taint_support' entry in %Config, which when true tells me that taint is disabled, I expect that setting to be 'define' under 'NO_TAINT_SUPPORT' and 'silent' when under 'SILENT_NO_TAINT_SUPPORT', and false when taint is enabled.
7. I expect that the 'taint_support' option to be documented in INSTALL
8. I do not expect that the use of 'taint_support' is commented with terms like 'DANGER'. Is the feature safe to use or not?

Perl configuration

Note the locally applied patch is the one from PR #19656 to disable a compile warning associated with building with NO_TAINT_SUPPORT.

With -DNO_TAINT_SUPPORT:

$ ./perl -Ilib -V
Summary of my perl5 (revision 5 version 35 subversion 12) configuration:
  Derived from: 99db5f9692dfa6466693dce901a6e805243181fc
  Platform:
    osname=linux
    osvers=5.14.0-1032-oem
    archname=x86_64-linux-thread-multi
    uname='linux oncidium 5.14.0-1032-oem #35-ubuntu smp thu mar 31 12:49:29 utc 2022 x86_64 x86_64 x86_64 gnulinux '
    config_args='-Dusethreads -Doptimize=-g -d -Dusedevel -Dcc=ccache gcc -Dld=gcc -DDEBUGGING -Accflags=-DNO_TAINT_SUPPORT'
    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='gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -DNO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DNO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='9.4.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='gcc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.31.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.31'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong'


Characteristics of this binary (from libperl): 
  Compile-time options:
    DEBUGGING
    HAS_TIMES
    MULTIPLICITY
    NO_TAINT_SUPPORT
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_TRACK_MEMPOOL
    PERL_USE_DEVEL
    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
  Locally applied patches:
    uncommitted-changes
  Built under linux
  Compiled at Apr 22 2022 04:02:01
  %ENV:
    PERLBREW_CONFIGURE_FLAGS="-de -Dcc=ccache\ gcc -Dld=gcc"
    PERLBREW_HOME="/home/yorton/.perlbrew"
    PERLBREW_MANPATH="/home/yorton/perl5/perlbrew/perls/perl-5.34.1/man"
    PERLBREW_PATH="/home/yorton/perl5/perlbrew/bin:/home/yorton/perl5/perlbrew/perls/perl-5.34.1/bin"
    PERLBREW_PERL="perl-5.34.1"
    PERLBREW_ROOT="/home/yorton/perl5/perlbrew"
    PERLBREW_SHELLRC_VERSION="0.88"
    PERLBREW_VERSION="0.88"
  @INC:
    lib
    /usr/local/lib/perl5/site_perl/5.35.12/x86_64-linux-thread-multi
    /usr/local/lib/perl5/site_perl/5.35.12
    /usr/local/lib/perl5/5.35.12/x86_64-linux-thread-multi
    /usr/local/lib/perl5/5.35.12

With SILENT_NO_TAINT_SUPPORT:

$ ./perl -Ilib -V
Summary of my perl5 (revision 5 version 35 subversion 12) configuration:
  Local Commit: cf026252112f094e91b5e5674035127d6460661f
  Ancestor: 99db5f9692dfa6466693dce901a6e805243181fc
  Platform:
    osname=linux
    osvers=5.14.0-1032-oem
    archname=x86_64-linux-thread-multi
    uname='linux oncidium 5.14.0-1032-oem #35-ubuntu smp thu mar 31 12:49:29 utc 2022 x86_64 x86_64 x86_64 gnulinux '
    config_args='-Dusethreads -Doptimize=-g -d -Dusedevel -Dcc=ccache gcc -Dld=gcc -DDEBUGGING -Accflags=-DSILENT_NO_TAINT_SUPPORT'
    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='gcc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -DSILENT_NO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DSILENT_NO_TAINT_SUPPORT -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='9.4.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='gcc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.31.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.31'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong'


Characteristics of this binary (from libperl): 
  Compile-time options:
    DEBUGGING
    HAS_TIMES
    MULTIPLICITY
    NO_TAINT_SUPPORT
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_TRACK_MEMPOOL
    PERL_USE_DEVEL
    SILENT_NO_TAINT_SUPPORT
    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
  Locally applied patches:
    cf026252112f094e91b5e5674035127d6460661f
  Built under linux
  Compiled at Apr 22 2022 05:51:40
  %ENV:
    PERLBREW_CONFIGURE_FLAGS="-de -Dcc=ccache\ gcc -Dld=gcc"
    PERLBREW_HOME="/home/yorton/.perlbrew"
    PERLBREW_MANPATH="/home/yorton/perl5/perlbrew/perls/perl-5.34.1/man"
    PERLBREW_PATH="/home/yorton/perl5/perlbrew/bin:/home/yorton/perl5/perlbrew/perls/perl-5.34.1/bin"
    PERLBREW_PERL="perl-5.34.1"
    PERLBREW_ROOT="/home/yorton/perl5/perlbrew"
    PERLBREW_SHELLRC_VERSION="0.88"
    PERLBREW_VERSION="0.88"
  @INC:
    lib
    /usr/local/lib/perl5/site_perl/5.35.12/x86_64-linux-thread-multi
    /usr/local/lib/perl5/site_perl/5.35.12
    /usr/local/lib/perl5/5.35.12/x86_64-linux-thread-multi
    /usr/local/lib/perl5/5.35.12

[neilb]

Thanks Yves – some good points there. Some of them were discussed at the RFC stage, but some not.

We theoretically have at least three options:

1. Leave it in, as-is
2. Revert it. This would pretty much necessitate a quick 5.35.12, so that we’ve got most of the pre-RC1 month to test.
3. Leave it in, and change it down the road. This is a non-starter.

I’ve added this to the agenda for today’s PSC meeting, but at the moment I think #2 is the way to go.

[neilb]

The PR is going to be reverted, and I'll start the RFC process again.

[demerphq]

On Sat, 23 Apr 2022 at 12:26, Neil Bowers ***@***.***> wrote: The PR is going to be reverted, and I'll start the RFC process again.

I regret that outcome, especially as if I had realized that my email had been lost we might have had time to resolve this without a revert. Sorry mate, I know you put a lot of effort into this. I feel crappy being the messenger here. :-( Yves

…

-- perl -Mre=debug -e "/just|another|perl|hacker/"

[neilb]

Don't feel bad – the job here is to make the best Perl we can, and as a result of you raising this issue, we'll end up with a better version of "turn off taint". I'm glad you did send this, and that we reverted my PR before 5.36 went out. Sure we would have "patched it up", but the end result will be better this way.