BBC: Blead (cdbed2a) Breaks RDF::Trine (and others)

[cjg-cguevara]

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.8.

---

BBC: Blead Breaks RDF::Trine

Please see http://fast-matrix.cpantesters.org/?dist=RDF::Trine

---

Flags

• category=core
• severity=low

---

Perl configuration

Site configuration information for perl 5.41.8:

Configured by cpan at Tue Jan 14 00:11:14 EST 2025.

Summary of my perl5 (revision 5 version 41 subversion 8) configuration:
  Commit id: 61978476912ee303cc78e7bf09602a4b38f3d75e
  Platform:
    osname=linux
    osvers=5.14.0-503.21.1.el9_5.x86_64
    archname=x86_64-linux
    uname='linux cjg-rhel9 5.14.0-503.21.1.el9_5.x86_64 #1 smp preempt_dynamic thu dec 19 09:37:00 est 2024 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/bin -Dusedevel -Duse64bitall'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-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='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.5.0 20240719 (Red Hat 11.5.0-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-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.8:
    /home/cpan/bin/perl/lib/site_perl/5.41.8/x86_64-linux
    /home/cpan/bin/perl/lib/site_perl/5.41.8
    /home/cpan/bin/perl/lib/5.41.8/x86_64-linux
    /home/cpan/bin/perl/lib/5.41.8

---
Environment for perl 5.41.8:
    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

[jkeenan]

RDF-Trine has so many prereqs that bisecting is taking a long time. Based on the failure reports and examination of https://metacpan.org/release/GWILLIAMS/RDF-Trine-1.019/source/lib/RDF/Trine/Parser/RDFPatch.pm#L194, I'm going to guess that the breaking commit is cdbed2a.

commit cdbed2a40eb1292d5be2fd59c091cf78f6a4be69
Author:     Richard Leach <[email protected]>
AuthorDate: Tue Nov 19 18:02:37 2024
Commit:     Richard Leach <[email protected]>
CommitDate: Mon Jan 13 17:11:16 2025

    OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant
    
    This commit adds OP_SUBSTR_LEFT and associated machinery for fast
    handling of the constructions:
    
            substr EXPR,0,LENGTH,''
    and
            substr EXPR,0,LENGTH
    
    Where EXPR is a scalar lexical, the OFFSET is zero, and either there
    is no REPLACEMENT or it is the empty string. LENGTH can be anything
    that OP_SUBSTR supports. These constraints allow for a very stripped
    back and optimised version of pp_substr.
...

@richardleach, can you examine further and perhaps discuss with upstream author? Thanks.

[mauke]

Shorter reproducer:

$ ./perl -Ilib -wE 'my $x = "hello"; if ($x =~ /^../) { substr $x, 0, $+[0], ""; } say $x'
Argument "" isn't numeric in substr left at -e line 1.
Modification of a read-only value attempted at -e line 1.

It's trying to modify $+[0] somehow.

[jkeenan]

Shorter reproducer:

$ ./perl -Ilib -wE 'my $x = "hello"; if ($x =~ /^../) { substr $x, 0, $+[0], ""; } say $x'
Argument "" isn't numeric in substr left at -e line 1.
Modification of a read-only value attempted at -e line 1.

It's trying to modify $+[0] somehow.

Confirmed. Bisecting that also points to commit cdbed2a.

[jkeenan]

Also likely affected: IO-Async
http://www.cpantesters.org/cpan/report/e21668de-d265-11ef-b171-a1011627361f

Spreadsheet-WriteExcel
#22915

JSON-Validator
#22921

Template-Simple
#22923

[mauke]

I have a patch that seems to fix this issue:

diff --git peep.c peep.c
index e9de45eb0a..797c13113f 100644
--- peep.c
+++ peep.c
@@ -3869,7 +3869,7 @@ Perl_rpeep(pTHX_ OP *o)
             break;
 
         case OP_SUBSTR: {
-            OP *expr, *offs, *len;
+            OP *expr, *offs, *len, *repl = NULL;
             /* Specialize substr($x, 0, $y) and substr($x,0,$y,"") */
             /* Does this substr have 3-4 args and amiable flags? */
             if (
@@ -3897,7 +3897,7 @@ Perl_rpeep(pTHX_ OP *o)
 
                 if (cMAXARG3x(o) == 4) {/* replacement */
                     /* Is the replacement string CONST ""? */
-                    OP *repl = OpSIBLING(len);
+                    repl = OpSIBLING(len);
                     if (repl->op_type != OP_CONST)
                         break;
                     SV *repl_sv = cSVOPx_sv(repl);
@@ -3912,8 +3912,15 @@ Perl_rpeep(pTHX_ OP *o)
             /* (The finalizer does not seem to change op_next here) */
             expr->op_next = offs->op_next;
             o->op_private = cMAXARG3x(o);
-            if (cMAXARG3x(o) == 4)
+            if (repl) {
+                while (
+                    len->op_type == OP_NULL
+                    && (len->op_flags & OPf_KIDS)
+                )
+                    len = cUNOPx(len)->op_first;
+                assert(len->op_next == repl);
                 len->op_next = o;
+            }
 
             /* We have a problem if padrange pushes the expr OP for us,
              * then jumps straight to the offs CONST OP. For example:

But I wrote it by trying to fix the symptoms of the issue, so I don't know how complete (or correct) it really is.

[mauke]

Scratch that attempt; ./perl -Ilib -wE 'my $x = "hello"; substr $x, 0, @ARGV ? 2 : 3, ""; say $x' breaks it. I think this means it is fruitless trying to find a single place in len to patch. There can be multiple continuation pointers to repl and they all have to be fixed up ... unless repl itself can be nulled out. Hmm ...

[mauke]

Revised patch based on that idea:

diff --git peep.c peep.c
index e9de45eb0a..d311d2bdf4 100644
--- peep.c
+++ peep.c
@@ -3869,7 +3869,7 @@ Perl_rpeep(pTHX_ OP *o)
             break;
 
         case OP_SUBSTR: {
-            OP *expr, *offs, *len;
+            OP *expr, *offs, *len, *repl = NULL;
             /* Specialize substr($x, 0, $y) and substr($x,0,$y,"") */
             /* Does this substr have 3-4 args and amiable flags? */
             if (
@@ -3897,7 +3897,7 @@ Perl_rpeep(pTHX_ OP *o)
 
                 if (cMAXARG3x(o) == 4) {/* replacement */
                     /* Is the replacement string CONST ""? */
-                    OP *repl = OpSIBLING(len);
+                    repl = OpSIBLING(len);
                     if (repl->op_type != OP_CONST)
                         break;
                     SV *repl_sv = cSVOPx_sv(repl);
@@ -3912,8 +3912,6 @@ Perl_rpeep(pTHX_ OP *o)
             /* (The finalizer does not seem to change op_next here) */
             expr->op_next = offs->op_next;
             o->op_private = cMAXARG3x(o);
-            if (cMAXARG3x(o) == 4)
-                len->op_next = o;
 
             /* We have a problem if padrange pushes the expr OP for us,
              * then jumps straight to the offs CONST OP. For example:
@@ -3924,7 +3922,13 @@ Perl_rpeep(pTHX_ OP *o)
               * B::Deparse. :/  */
             op_null(offs);
 
-            /* repl status unchanged because it makes Deparsing easier. */
+            /* There can be multiple pointers to repl, too:
+             *    substr $x, 0, $y ? 2 : 3, "";
+             * So instead of rewriting all of len, null out repl.
+             */
+            if (repl) {
+                op_null(repl);
+            }
 
             /* Upgrade the SUBSTR to a SUBSTR_LEFT */
             OpTYPE_set(o, OP_SUBSTR_LEFT);

[richardleach]

@mauke many thanks for doing the legwork before I could look at it. 🎆 🏆
Unless you have already started, I'll open a PR tomorrow (or soon after).

[mauke]

No, what I've posted here is pretty much all I have (and perl -MO=Concise to examine optrees). My latest patch is probably morally correct, but it breaks at least one test and I feel a little out of my depth. Please go ahead.

[richardleach]

Local testing suggests this should (unsurprisingly) be fixed by b1397c4