perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

BBC: Blead Breaks EV

Open cjg-cguevara opened this issue 1 year ago • 13 comments

This is a bug report for perl from "Carlos Guevara" [email protected], generated with the help of perlbug 1.43 running under perl 5.41.1.


BBC: Blead Breaks EV

Please see http://fast-matrix.cpantesters.org/?dist=EV%204.34


Flags

  • category=core
  • severity=low

Perl configuration

Site configuration information for perl 5.41.1:

Configured by cpan at Fri Jun 28 23:41:57 EDT 2024.

Summary of my perl5 (revision 5 version 41 subversion 1) configuration:
  Commit id: 99e7291a160c07384076cfdcf5b3126143155452
  Platform:
    osname=linux
    osvers=5.14.0-427.22.1.el9_4.x86_64
    archname=x86_64-linux-thread-multi
    uname='linux cjg-rhel9 5.14.0-427.22.1.el9_4.x86_64 #1 smp preempt_dynamic mon jun 10 09:23:36 edt 2024 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/bin -Dusedevel -Duse64bitall -Duseithreads'
    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='-O2'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.4.1 20231218 (Red Hat 11.4.1-3)'
    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-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /usr/lib64 /usr/local/lib64
    libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/../lib64/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.34'
  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-strong'


---
@INC for perl 5.41.1:
    /home/cpan/bin/perl/lib/site_perl/5.41.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/site_perl/5.41.1
    /home/cpan/bin/perl/lib/5.41.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/5.41.1

---
Environment for perl 5.41.1:
    HOME=/home/cpan
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_ALL=C
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/cpan/bin/perl/bin:/home/cpan/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

cjg-cguevara avatar Jun 29 '24 04:06 cjg-cguevara

This is a typemap error. I haven't bisected it, but I strongly suspect commit 2f4409bf707ec51:

Use alert control character as quoting character in INPUT templates

This way, one no longer needs to escape double quotes inside an input template.

The typemap in EV contains this:

INPUT

T_WATCHER
	if (!(SvROK ($arg) && SvOBJECT (SvRV ($arg))
              && (SvSTASH (SvRV ($arg)) == stash_" . ($type =~ /ev_(\S+)/, "$1") . "
                  || sv_derived_from ($arg, \"EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\"))))
          croak (\"object is not of type EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\");
	$var = ($type)SvPVX (SvRV ($arg));

I.e. it deliberately uses unescaped " to break out of the quoting and inject Perl code.

Fixing it should be easy because the OUTPUT section also does it, but using ${...} instead:

T_WATCHER
	$arg = e_bless ((struct ev_watcher *)$var, stash_${ ($type =~ /ev_(\S+)/, \"$1") });

If I understand the situation correctly, using the ${...} trick in the INPUT section should work and be compatible with previous ExtUtils::ParseXS versions. @Leont ?

mauke avatar Jun 29 '24 06:06 mauke

This is a typemap error. I haven't bisected it, but I strongly suspect commit https://github.com/Perl/perl5/commit/2f4409bf707ec51ce63fd97c981a2c9e08b97241:

Yeah definitely.

If I understand the situation correctly, using the ${...} trick in the INPUT section should work and be compatible with previous ExtUtils::ParseXS versions. @Leont ?

And yes ${ ... } is the way that works consistently everywhere.

Leont avatar Jun 29 '24 10:06 Leont

This seems to fix it

diff --git typemap typemap
index f30badc..ea5c33b 100644
--- typemap
+++ typemap
@@ -37,9 +37,9 @@ T_LOOP
 
 T_WATCHER
    if (!(SvROK ($arg) && SvOBJECT (SvRV ($arg))
-              && (SvSTASH (SvRV ($arg)) == stash_" . ($type =~ /ev_(\S+)/, "$1") . "
-                  || sv_derived_from ($arg, \"EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\"))))
-          croak (\"object is not of type EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\");
+              && (SvSTASH (SvRV ($arg)) == stash_${ ($type =~ /ev_(\S+)/, \"$1") }
+                  || sv_derived_from ($arg, \"EV::${ ($type =~ /ev_(\S+)/, \ucfirst "$1") }\"))))
+          croak (\"object is not of type EV::${ ($type =~ /ev_(\S+)/, \ucfirst "$1") }\");
    $var = ($type)SvPVX (SvRV ($arg));
 
 OUTPUT

Leont avatar Jun 29 '24 16:06 Leont

Bisecting with this invocation:

perl Porting/bisect.pl \
--start=56d5b6ef820802cd4c57fc2cfd8301a3f5d7af04 \
--end=99e7291a160c07384076cfdcf5b3126143155452 \
--module=EV

... pointed to commit 2f4409bf70

commit 2f4409bf707ec51ce63fd97c981a2c9e08b97241
Author: Leon Timmermans <[email protected]>
Date:   Fri Jun 14 01:08:03 2024 +0200
Commit:     Leon Timmermans <[email protected]>
CommitDate: Mon Jun 24 05:30:37 2024 +0100

    Use alert control character as quoting character in INPUT templates

    This way, one no longer needs to escape double quotes inside an input
    template.

    Parsexs has done this for OUTPUT templates since the very beginning, I
    can't find any reason for why we weren't also doing it for INPUT
    templates.

jkeenan avatar Jun 29 '24 18:06 jkeenan

Based on some grepping there are two more modules that used this "feature", but both of them were already broken when ParseXS was made strict 12 years ago and never even had that fixed so they can be considered dead.

Leont avatar Jul 01 '24 17:07 Leont

On 7/1/24 13:08, Leon Timmermans wrote:

Based on some grepping there are two more modules that used this "feature", but both of them were already broken when ParseXS was made strict 12 years ago and never even had that fixed so they can be considered dead.

Just for the record, those two other modules are ... ?

jkeenan avatar Jul 01 '24 19:07 jkeenan

Just for the record, those two other modules are ... ?

Gtk-perl and Etk-perl

Leont avatar Jul 01 '24 22:07 Leont

Still failing as of v5.41.9-77-g28aeb4ecd4 (Mar 12 2025).

jkeenan avatar Mar 13 '25 10:03 jkeenan

Leon's patch can now be used by CPAN.pm users using this distroprefs file: https://github.com/eserte/srezic-cpan-distroprefs/blob/master/EV.yml

The patch is in https://cpan.metacpan.org/authors/id/S/SR/SREZIC/patches/EV-4.34-RT156069-LEONT.patch (note that the patched typemap does not work with older ExtUtils::ParseXS, so I increased the prereq version for that; also the META.* files are included in the patch file for this reason).

eserte avatar May 14 '25 17:05 eserte

(note that the patched typemap does not work with older ExtUtils::ParseXS, so I increased the prereq version for that; also the META.* files are included in the patch file for this reason).

It was meant to work on older versions of ParseXS. I thought I had double-checked that. I may have to fix that.

Leont avatar May 14 '25 18:05 Leont

This seems to work on older and newer versions of ParseXS, my apologies for the flawed earlier patch

diff --git typemap typemap
index f30badc..7f00476 100644
--- typemap
+++ typemap
@@ -37,9 +37,9 @@ T_LOOP
 
 T_WATCHER
    if (!(SvROK ($arg) && SvOBJECT (SvRV ($arg))
-              && (SvSTASH (SvRV ($arg)) == stash_" . ($type =~ /ev_(\S+)/, "$1") . "
-                  || sv_derived_from ($arg, \"EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\"))))
-          croak (\"object is not of type EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\");
+              && (SvSTASH (SvRV ($arg)) == stash_${ \($type =~ /ev_(\S+)/, \"$1\") }
+                  || sv_derived_from ($arg, \"EV::${ ($type =~ /ev_(\S+)/, \ucfirst \"$1\") }\"))))
+          croak (\"object is not of type EV::${ ($type =~ /ev_(\S+)/, \\ucfirst \"$1\") }\");
    $var = ($type)SvPVX (SvRV ($arg));
 
 OUTPUT

Leont avatar May 14 '25 18:05 Leont

diff --git typemap.old typemap
index f30badc..663f769 100644
--- typemap.old
+++ typemap
@@ -37,9 +37,9 @@ T_LOOP
 
 T_WATCHER
 	if (!(SvROK ($arg) && SvOBJECT (SvRV ($arg))
-              && (SvSTASH (SvRV ($arg)) == stash_" . ($type =~ /ev_(\S+)/, "$1") . "
-                  || sv_derived_from ($arg, \"EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\"))))
-          croak (\"object is not of type EV::" . ($type =~ /ev_(\S+)/, ucfirst "$1") . "\");
+              && (SvSTASH (SvRV ($arg)) == stash_${ ($type =~ /ev_(\S+)/, \qq{$1}) }
+                  || sv_derived_from ($arg, \"EV::${ ($type =~ /ev_(\S+)/, \ucfirst qq{$1}) }\"))))
+          croak (\"object is not of type EV::${ ($type =~ /ev_(\S+)/, \ucfirst qq{$1}) }\");
 	$var = ($type)SvPVX (SvRV ($arg));
 
 OUTPUT

This version should be more compatible, and actually gets the tabs right so it will apply cleanly.

Leont avatar May 25 '25 12:05 Leont

This version should be more compatible, and actually gets the tabs right so it will apply cleanly.

I tried @leont's latest patch on the following perls:

perl-5.16.3 perl-5.20.3 perl-5.36.3 perl-5.40.1 (v5.41.13 (v5.41.12-57-g072a106277))

EV built and tested OK on all of these.

jkeenan avatar May 27 '25 11:05 jkeenan