BBC: Blead Breaks B::Utils

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

---

BBC: Blead Breaks B::Utils

Please see http://fast-matrix.cpantesters.org/?dist=B::Utils

---

Flags

• category=core
• severity=low

---

Perl configuration

Site configuration information for perl 5.41.7:

Configured by cpan at Thu Dec 19 00:23:29 EST 2024.

Summary of my perl5 (revision 5 version 41 subversion 7) configuration:
  Commit id: 7496ff12fc1a5176641e424139fc7ad8e8ca08e5
  Platform:
    osname=linux
    osvers=5.14.0-503.16.1.el9_5.x86_64
    archname=x86_64-linux
    uname='linux cjg-rhel9 5.14.0-503.16.1.el9_5.x86_64 #1 smp preempt_dynamic thu nov 21 07:26:21 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.7:
    /home/cpan/bin/perl/lib/site_perl/5.41.7/x86_64-linux
    /home/cpan/bin/perl/lib/site_perl/5.41.7
    /home/cpan/bin/perl/lib/5.41.7/x86_64-linux
    /home/cpan/bin/perl/lib/5.41.7

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

Bisecting with the following invocation:

perl Porting/bisect.pl \
--start=342b57d68213add262954103c711be7d6ed0f128 \
--end=7496ff12fc1a5176641e424139fc7ad8e8ca08e5 \
--module=B::Utils

... pointed to this as the breaking point:

7015de19863573066cc75ff79721681082895466 is the first bad commit
commit 7015de19863573066cc75ff79721681082895466
Author: Richard Leach <[email protected]>
Date:   Wed Nov 13 21:47:35 2024 +0000
Commit:     Richard Leach <[email protected]>
CommitDate: Mon Dec 16 21:32:16 2024 +0000


    op_scope/newCONDOP: Optimise away empty else blocks in OP_COND_EXPR
    
    This commit comprises two main changes:
    * Not wrapping a bare OP_STUB in an enter/leave pair (Perl_op_scope)
    * Checking for a bare OP_STUB in the `falseop` argument when building
      an OP_COND_EXPR and then freeing it, rather than adding it as a
      sibling to the `first` and `trueop` branches.
    
    The main benefits of this are:
    * lower memory usage due to unnecessary OP removal
    * faster execution due to unnecessary OPs not being executed
    
    There are also some small changes to Deparse.pm and an additional
    Deparse.t test.

 lib/B/Deparse.pm | 23 ++++++++++++++++-------
 lib/B/Deparse.t  |  4 ++++
 op.c             | 23 +++++++++++++++++++----
 3 files changed, 39 insertions(+), 11 deletions(-)
bisect found first bad commit
That took 953 seconds.

@richardleach, please take a look. Thanks.

[jkeenan]

The commit identified in this ticket was one of three code-containing commits in #22725. One other of those commits has also been associated with CPAN breakage (#22868). Given that we're due for a dev release tomorrow, should we revert the commits in that p.r. to give us time to diagnose the problem more fully?

[richardleach]

I can have an initial look this evening and do a revert if it looks like anything other than "update to know about this new OP" PRs are needed. Or if someone can get there first and wants to, feel free to revert away.

[richardleach]

The commit identified in this ticket was one of three code-containing commits in #22725. One other of those commits has also been associated with CPAN breakage (#22868). Given that we're due for a dev release tomorrow, should we revert the commits in that p.r. to give us time to diagnose the problem more fully?

Different PR. The bisected commit for this issue was in #22745.

[richardleach]

Without DEBUGGING, the test (t/utils/30parent.t) seg faults:

Program received signal SIGSEGV, Segmentation fault.
0x000055555563e379 in Perl_pp_or ()
(gdb) bt
#0  0x000055555563e379 in Perl_pp_or ()
#1  0x00005555556c6e43 in Perl_runops_standard ()
#2  0x00005555555c15a8 in perl_run ()
#3  0x000055555559a3b2 in main ()

With DDEBUGGING, we get an assert:

perl5.41.7: inline.h:3424: Perl_cx_pushblock: Assertion `cxstack_ix <= 0 || CxTYPE(cx-1) == CXt_SUBST || cx->blk_oldsp >= (cx-1)->blk_oldsp' failed.

[jkeenan]

Without DEBUGGING, the test (t/utils/30parent.t) seg faults:

Program received signal SIGSEGV, Segmentation fault.
0x000055555563e379 in Perl_pp_or ()
(gdb) bt
#0  0x000055555563e379 in Perl_pp_or ()
#1  0x00005555556c6e43 in Perl_runops_standard ()
#2  0x00005555555c15a8 in perl_run ()
#3  0x000055555559a3b2 in main ()

With DDEBUGGING, we get an assert:

perl5.41.7: inline.h:3424: Perl_cx_pushblock: Assertion `cxstack_ix <= 0 || CxTYPE(cx-1) == CXt_SUBST || cx->blk_oldsp >= (cx-1)->blk_oldsp' failed.

On a -DDEBUGGING threaded build on FreeBSD-13, I am similarly getting:

[perlmonger: B-Utils-0.27-2] $ bleadprove -vb t/utils/30parent.t 
t/utils/30parent.t .. 
1..181
Assertion failed: (cxstack_ix <= 0 || CxTYPE(cx-1) == CXt_SUBST || cx->blk_oldsp >= (cx-1)->blk_oldsp), function Perl_cx_pushblock, file ./inline.h, line 3426.
Failed 181/181 subtests 

Test Summary Report
-------------------
t/utils/30parent.t (Wstat: 134 (Signal: ABRT, dumped core) Tests: 0 Failed: 0)
  Non-zero wait status: 134
  Parse errors: Bad plan.  You planned 181 tests but ran 0.
Files=1, Tests=0,  1 wallclock secs ( 0.05 usr  0.01 sys +  0.08 cusr  0.03 csys =  0.17 CPU)
Result: FAIL

[richardleach]

Things seem to go awry in t/utils/30parent.t at:

        if ($] >= 5.021002 and exists &B::OP::parent) {
            $parent = $op->_parent;

Within B::Utils.pm, that seems to call sub B::OP::_parent_impl, where we get to the return statement, which has a mixture of ternaries with empty false branches in them:

    return (
        $op->sibling->_parent_impl( $target, "$cx$$op S " )
            || (
              $cx =~ /^(?:\d+ S )*(?:\d+ N )*$/
            ? $op->next->_parent_impl( $target, "$cx$$op N " )
            : ()
            )
            || (
              $op->can('first')
            ? $op->first->_parent_impl( $target, "$cx$$op F " )
            : ()
            )
    );

$op->first->_parent_impl( $target, "$cx$$op F " ) seems key, if you swap it for a placeholder (having commented out much of 30parent.t), things run to completion.

[richardleach]

I'm out of time tonight. Will pick this up again tomorrow evening.

I don't understand the context system well enough to know what the failing assert in Perl_cx_pushblock is telling us / what normal looks like:

assert(cxstack_ix <= 0
            || CxTYPE(cx-1) == CXt_SUBST
            || cx->blk_oldsp >= (cx-1)->blk_oldsp);

[richardleach]

Within B::Utils.pm, that seems to call sub B::OP::_parent_impl, where we get to the return statement, which has a mixture of ternaries with empty false branches in them:

If the return statement is broken up to remove the ternaries as follows, then all tests pass:

my $result;

$result = $op->sibling->_parent_impl($target, "$cx$$op S ");
if ($result) {
    return $result;
}

if ($cx =~ /^(?:\d+ S )*(?:\d+ N )*$/) {
    $result = $op->next->_parent_impl($target, "$cx$$op N ");
    if ($result) {
        return $result;
    }
}

if ($op->can('first')) {
    $result = $op->first->_parent_impl($target, "$cx$$op F ");
    if ($result) {
        return $result;
    }
}

return $result;

So the working assumption is that there is a bug in 7015de19863573066cc75ff79721681082895466. It could be reverted for this month's point release or kept in while I debug it.

[richardleach]

I've opened #22870 to revert 7015de1 for the time being.

[richardleach]

7015de1 will probably come back as the same change to Perl_op_scope, but modifying OP_COND_EXPR kids via a peephole change rather than in Perl_newCONDOP.

[richardleach]

7015de1 reverted by 321290a. The need exists for a ternary to meaningfully push PL_sv_undef to the stack, whereas for an else {} block it doesn't matter.

[iabyn]

On Thu, Dec 19, 2024 at 03:43:42PM -0800, Richard Leach wrote: I don't understand the context system well enough to know what the failing assert in `Perl_cx_pushblock` is telling us / what normal looks like: ``` assert(cxstack_ix <= 0 || CxTYPE(cx-1) == CXt_SUBST || cx->blk_oldsp >= (cx-1)->blk_oldsp); ```

Each pushed context block should have a not-lower argument stack position than the previous pushed block, if any. Except that the weird CXt_SUBST context type (used only for running code in s/XXX/YYY/e) doesn't include a saved stack position.

…

-- "Emacs isn't a bad OS once you get used to it. It just lacks a decent editor."