search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.99k stars 559 forks source link

defined( \@foo = $obj->method) corrupts $obj when it is a loop variable in loop on Hash::Ordered objects #22710

Open djerius opened 1 month ago

djerius commented 1 month ago

Description

This is an odd one. I create an ordered hash whose values are objects via Hash::Ordered, then loop over the values, and incorrectly call

defined(\@tags = $value->method)

which, under 5.40 and not 5.38.3, corrupts $value. If I remove the call to defined, $value is not corrupted.

Steps to Reproduce

use v5.38;
use experimental 'declared_refs';

use Hash::Ordered;

package foo {
    sub new { my $class = shift; bless {}, $class }
    sub method { [] };
}

eval {
    my $hash = Hash::Ordered->new( map { $_ => foo->new } 0..1 );
    for my $value ( $hash->values ) {
        next if defined( my \@foo = $value->method );
    }
};

say "call defined: $@";

eval {
    my $hash = Hash::Ordered->new( map { $_ => foo->new } 0..1 );
    for my $value ( $hash->values ) {
        my \@foo = $value->method;
    }
};

say "don't call defined: $@";

This results in:

# Perl v5.38.2
$ perl bug.pl
call defined: 
don't call defined: 

# Perl v5.40
$ perl bug.pl
call defined: Can't locate object method "method" via package "1" (perhaps you forgot to load "1"?) at bug.pl line 15.

don't call defined:

This is perhaps something related to tied objects (which Hash::Ordered is based on)? I've experimented with looping over a simple array of objects, and the corruption does not occur.

Perl configuration

Summary of my perl5 (revision 5 version 40 subversion 0) configuration:

  Platform:
    osname=linux
    osvers=6.9.7+bpo-amd64
    archname=x86_64-linux
    uname='linux leafhopper 6.9.7+bpo-amd64 #1 smp preempt_dynamic debian 6.9.7-1~bpo12+1 (2024-07-03) x86_64 gnulinux '
    config_args='-Dprefix=/home/dj/.plenv/versions/5.40 -de -Dversiononly -A'eval:scriptdir=/home/dj/.plenv/versions/5.40/bin''
    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='12.2.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='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/x86_64-linux-gnu/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.36'
  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'

Characteristics of this binary (from libperl): 
  Compile-time options:
    HAS_LONG_DOUBLE
    HAS_STRTOLD
    HAS_TIMES
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
  Built under linux
  Compiled at Aug  5 2024 00:46:42
  %ENV:
    PERL_CPANM_OPT="--cascade-search --mirror-only  --mirror /home/dj/Work/darkpan --mirror http://www.cpan.org"
    PERL_CPAN_MIRROR_TINY_BASE="/home/dj/Work/darkpan"
    PERL_DARKPAN="/home/dj/Work/darkpan"
  @INC:
    /home/dj/.plenv/versions/5.40/lib/perl5/site_perl/5.40.0/x86_64-linux
    /home/dj/.plenv/versions/5.40/lib/perl5/site_perl/5.40.0
    /home/dj/.plenv/versions/5.40/lib/perl5/5.40.0/x86_64-linux
    /home/dj/.plenv/versions/5.40/lib/perl5/5.40.0
jkeenan commented 1 month ago

Bisecting with the following invocation on unthreaded Linux builds ...

perl Porting/bisect.pl \
--start=v5.38.0 \
--end=v5.40.0 \
--with-module Hash::Ordered \
-- ./perl -Ilib $P5P_DIR/gh-22710-hash-ordered.pl

... where I modified @djerius's program to read:

$ cat gh-22710-hash-ordered.pl 
use v5.38;
use experimental 'declared_refs';
use Hash::Ordered;

package foo {
    sub new { my $class = shift; bless {}, $class }
    sub method { [] };
}

eval {
    my $hash = Hash::Ordered->new( map { $_ => foo->new } 0..1 );
    for my $value ( $hash->values ) {
        next if defined( my \@foo = $value->method );
    }
};

if ($@) {
    print "XXX: $@\n";
    exit 1;
}
else {
    exit 0;
}

... and that pointed to a change of behavior at 604c0355d2 last December

604c0355d21cd2bdfa4adbc4cc6e2014acb8760a is the first bad commit
commit 604c0355d21cd2bdfa4adbc4cc6e2014acb8760a
Author: David Mitchell <davem@iabyn.com>
Date:   Sun Sep 24 22:11:21 2023 +0100
Commit:     David Mitchell <davem@iabyn.nospamdeletethisbit.com>
CommitDate: Mon Dec 4 16:42:35 2023 +0000

    make RC-stack-aware: unwrap pp_refassign()

    Remove the temporary wrapper from pp_refassign()

    Also move a couple of variable declarations into the smallest scope
    possible: mg and stash are used only by (and implicitly by)
    SvCANEXISTDELETE().

I say "change of behavior" because I don't know enough about declared_refs or Hash::Ordered to say what good behavior should be.

@iabyn, can you take a look? Thanks.