b9eeeef8c0 breaks Log::Trace and other modules

[jkeenan]

Carlos Guevara has sent me a list of CPAN distributions which are failing tests against Perl 5 blead. I am trying to bisect these failures now. Since I suspect that more than one distribution will be broken by certain particular commits, I'm opening a separate ticket for each bad commit.

Log::Trace

Bisecting with this invocation:

$ perl Porting/bisect.pl \
--start=v5.41.8 \
--end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=Log::Trace

... I got:

b9eeeef8c043fb0238a07e64636815bf327a6562 is the first bad commit
commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author: Lukas Mai <[email protected]>
Date:   Fri Feb 14 21:49:03 2025 +0100
Commit:     Lukas Mai <[email protected]>
CommitDate: Sun Feb 16 01:42:36 2025 +0100

    op.c: re-enable coderef-in-stash optimization
    
    When seeing 'sub foo { ... }', perl does not need to allocate a full
    typeglob (with the subroutine being stored in the CODE slot). Instead,
    it can just store a coderef directly in the stash.

@mauke, please take a look.

For reference: The breaking commit was:

$ git describe
v5.41.8-98-gb9eeeef8c0

As I write this blead is at:

$ git describe
v5.41.8-119-g8d462f6a9e

Note: Right now the CPANtesters database is having difficulty receiving reports. So responding to BBC tickets is going to be more tedious than usual. See: https://www.nntp.perl.org/group/perl.cpan.testers.discuss/2025/02/msg4654.html.

[jkeenan]

foundation:

perl Porting/bisect.pl \
--start=7fdc8f34fd20e8d4bec8de19f64c869b152f1249 \
end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=foundation

b9eeeef8c043fb0238a07e64636815bf327a6562 is the first bad commit
commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author: Lukas Mai <[email protected]>
Date:   Fri Feb 14 21:49:03 2025 +0100

    op.c: re-enable coderef-in-stash optimization
    

[jkeenan]

POE::Component::Basement:

perl Porting/bisect.pl \
--start=7fdc8f34fd20e8d4bec8de19f64c869b152f1249 \
end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=POE::Component::Basement

b9eeeef8c043fb0238a07e64636815bf327a6562 is the first bad commit
commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author: Lukas Mai <[email protected]>
Date:   Fri Feb 14 21:49:03 2025 +0100

    op.c: re-enable coderef-in-stash optimization

[jkeenan]

Prima:

http://www.cpantesters.org/cpan/report/758df122-eeb8-11ef-881f-a6c9b3d6a9e5

perl Porting/bisect.pl \
--start=7fdc8f34fd20e8d4bec8de19f64c869b152f1249 \
end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=Prima

b9eeeef is the first bad commit
commit b9eeeef
Author: Lukas Mai [email protected]
Date: Fri Feb 14 21:49:03 2025 +0100

op.c: re-enable coderef-in-stash optimization

[jkeenan]

Symbol::Get:

http://www.cpantesters.org/cpan/report/7c7a6254-eeb8-11ef-b0f7-f3945e623728

perl Porting/bisect.pl \
--start=7fdc8f34fd20e8d4bec8de19f64c869b152f1249 \
end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=Symbol::Get

b9eeeef8c043fb0238a07e64636815bf327a6562 is the first bad commit
commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author: Lukas Mai <[email protected]>
Date:   Fri Feb 14 21:49:03 2025 +0100

    op.c: re-enable coderef-in-stash optimization

@mauke, please take a look. Thanks.

[jkeenan]

Test::Sims:

http://www.cpantesters.org/cpan/report/85edc448-eeb8-11ef-a7c3-9ed74f0d3477

$ perl Porting/bisect.pl \
--start=7fdc8f34fd20e8d4bec8de19f64c869b152f1249 \
end=8d462f6a9e96adf5dac36c7e778468b78a47be96 \
--module=Test::Sims

b9eeeef8c043fb0238a07e64636815bf327a6562 is the first bad commit
commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author: Lukas Mai <[email protected]>
Date:   Fri Feb 14 21:49:03 2025 +0100

    op.c: re-enable coderef-in-stash optimization

[mauke]

Log::Trace has been broken for a long time; its tests just didn't notice. For example:

$ perl -we 'sub foo { 42 } use Log::Trace warn => {AllSubs => 1};'
Not a GLOB reference at /home/mauke/perl5/perlbrew/perls/perl-5.40.1/lib/site_perl/5.40.1/Log/Trace.pm line 273.
BEGIN failed--compilation aborted at -e line 1.
$ perl -we 'package Bar; use constant FOO => 42;  use Log::Trace warn => {AllSubs => 1};'
Not a GLOB reference at /home/mauke/perl5/perlbrew/perls/perl-5.40.1/lib/site_perl/5.40.1/Log/Trace.pm line 273.
BEGIN failed--compilation aborted at -e line 1.

A patch for these issues as well as the recent blead failures has been available in https://rt.cpan.org/Ticket/Display.html?id=49409 since 2017, courtesy of @cpansprout.

[mauke]

I'm not sure what to do about foundation. It looks unmaintained, the code is alpha quality, the POD contains errors, the name clashes with a system module on Mac OS (Foundation.pm), and the module is already broken:

$ perl -we '{ package Foo; use constant C => 42; } { package Bar; use foundation; foundation "Foo"; }'
Not a subroutine reference at /home/mauke/perl5/perlbrew/perls/perl-5.40.1/lib/site_perl/5.40.1/foundation.pm line 86.

I've submitted a bug report + patch at https://rt.cpan.org/Ticket/Display.html?id=159752.

[mauke]

POE::Component::Basement: bug report + patch at https://rt.cpan.org/Ticket/Display.html?id=159755

[mauke]

Test::Sims was theoretically fixed in 2018: schwern/Test-Sims#3
But the update was never released to CPAN.

[mauke]

I've reported the issue to Prima: dk/Prima#144 (including a patch that makes make test pass for me, but it needs review by someone more familiar with Perl or Prima internals)

[mauke]

I've reported the issue to Symbol::Get: FGasper/p5-Symbol-Get#4 (plus more general breakage that was not covered by tests)

[dk]

I've reported the issue to Prima: dk/Prima#144 (including a patch that makes make test pass for me, but it needs review by someone more familiar with Perl or Prima internals)

yes, but there's more: Prima uses GvCV(gv_fetchmeth) to get down to a CV, and with this new code, how one is supposed to get it? could there be some flags in the class that says build it the old way, through a GV?

[tonycoz]

yes, but there's more: Prima uses GvCV(gv_fetchmeth) to get down to a CV, and with this new code, how one is supposed to get it? could there be some flags in the class that says build it the old way, through a GV?

gv_fetchmeth() should always return a GV or NULL. Under what circumstances is it returning something else?

[haarg]

@mauke Although many of the issues related to this have been fixed, we aren't comfortable with the level of breakage this late in the release cycle.

Could you put together a PR to revert this?

[bulk88]

I've reported the issue to Symbol::Get: FGasper/p5-Symbol-Get#4 (plus more general breakage that was not covered by tests)
Symbol::Get is PP.

Didn't run any code or single stepping, but this line looks suspicious.

perl5/pp.c

Line 637 in 8456d3d

GV * const gv = MUTABLE_GV(PL_stack_sp[-1]);

Why does it assume it got a GV* with a GP* and not a GV* no GP*, or a LV*, SV* POK const folder or a RV* to CV*, and i forgot what else can be inside HeVAL() from a key from stash HV*.

    const char * const elem = SvPV_const(sv, len);
    if (elem) {

What? Worst case retval is a const C HW RO char * "" no segv and just a undef warning.

[bulk88]

yes, but there's more: Prima uses GvCV(gv_fetchmeth) to get down to a CV, and with this new code, how one is supposed to get it? could there be some flags in the class that says build it the old way, through a GV?

gv_fetchmeth() should always return a GV or NULL. Under what circumstances is it returning something else?

13c59d4
and #14410

Everyone on CPAN wants lval assignment and & op to work, on what are getters/access, instead of CPAN code migrating to GvSV_set(gv,sv); style macros. C doesn't have array index or assignment operator overloading.

perl5/pp_hot.c

Line 6874 in 8456d3d

#define METHOD_CHECK_CACHE(stash,cache,meth) \

what happens if and XS mod doesn't check GvCVGEN(gv) and sees an out of date wrong inheritance layer (@ISA was written to) stale GV*/CV* from a diff package?

[tonycoz]

Why does it assume it got a GV* with a GP* and not a GV* no GP*, or a LV*, SV* POK const folder or a RV* to CV*, and i forgot what else can be inside HeVAL() from a key from stash HV*.

gelem is only called on the results of rv2gv.

what happens if and XS mod doesn't check GvCVGEN(gv) and sees an out of date wrong inheritance layer (@ISA was written to) stale GV*/CV* from a diff package?

@ISA has magic on it, one of the side effects of that is to bump HvMROMETA(stash)->cache_gen, see mro_isa_changed_in() for the gory details.