Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subject: List::Util qw(min) bug #15007

Closed
p5pRT opened this issue Oct 23, 2015 · 7 comments
Closed

Subject: List::Util qw(min) bug #15007

p5pRT opened this issue Oct 23, 2015 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 23, 2015

Migrated from rt.perl.org#126440 (status was 'rejected')

Searchable as RT126440$

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From @lonerr

Created by @lonerr

This is a bug report for perl from oleg@​mamontov.net,
generated with the help of perlbug 1.40 running under perl 5.22.0.

-----------------------------------------------------------------
The core List​::Util​::min works incorrectly with the snippet below​:
------------------------------
#!/usr/bin/env perl

use strict;
use warnings;

use List​::Util qw(min);

my $x = [qw( 1 1 1 1 1 )];

for ( 1..10 ) {
  push @​$x, 1;
  $#$x = min $#$x, 3;
  warn $#$x . "\n";
}
------------------------------
result​: 3 4 5 6 7 8 9 10 11 12

but simple adding zero to the first argument fixes 'min' behaviour​:
------------------------------
#!/usr/bin/env perl

use strict;
use warnings;

use List​::Util qw(min);

my $x = [qw( 1 1 1 1 1 )];

for ( 1..10 ) {
  push @​$x, 1;
  $#$x = min 0 + $#$x, 3;
  warn $#$x . "\n";
}
------------------------------
result​: 3 3 3 3 3 3 3 3 3 3

It looks like the 'min' was broken between 5.16 and 5.18.

Perl Info

Flags:
    category=library
    severity=medium
    module=List::Util

Site configuration information for perl 5.22.0:

Configured by lonerr at Thu Sep 10 22:53:13 MSK 2015.

Summary of my perl5 (revision 5 version 22 subversion 0) configuration:
   
  Platform:
    osname=darwin, osvers=14.4.0, archname=darwin-2level
    uname='darwin xenon.mamontov.net 14.4.0 darwin kernel version 14.4.0: thu may 28 11:35:04 pdt 2015; root:xnu-2782.30.5~1release_x86_64 x86_64 '
    config_args='-de -Dprefix=/Users/lonerr/perl5/perlbrew/perls/perl-5.22.0 -Aeval:scriptdir=/Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/bin'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector-strong -I/opt/local/include',
    optimize='-O3',
    cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector-strong -I/opt/local/include'
    ccversion='', gccversion='4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)', 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='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector-strong -L/opt/local/lib'
    libpth=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/6.1.0/lib /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib /usr/lib /opt/local/lib
    libs=-lpthread -lgdbm -ldbm -ldl -lm -lutil -lc
    perllibs=-lpthread -ldl -lm -lutil -lc
    libc=, so=dylib, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/opt/local/lib -fstack-protector-strong'



@INC for perl 5.22.0:
    /Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/lib/site_perl/5.22.0/darwin-2level
    /Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/lib/site_perl/5.22.0
    /Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/lib/5.22.0/darwin-2level
    /Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/lib/5.22.0
    .


Environment for perl 5.22.0:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/lonerr
    LANG (unset)
    LANGUAGE (unset)
    LC_COLLATE=ru_RU.UTF-8
    LC_CTYPE=ru_RU.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/lonerr/perl5/perlbrew/bin:/Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/bin::/Users/lonerr/bin:/opt/local/bin::/opt/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin
    PERLBREW_BASHRC_VERSION=0.73
    PERLBREW_HOME=/Users/lonerr/.perlbrew
    PERLBREW_MANPATH=/Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/man
    PERLBREW_PATH=/Users/lonerr/perl5/perlbrew/bin:/Users/lonerr/perl5/perlbrew/perls/perl-5.22.0/bin
    PERLBREW_PERL=perl-5.22.0
    PERLBREW_ROOT=/Users/lonerr/perl5/perlbrew
    PERLBREW_VERSION=0.73
    PERLDOC_PAGER=less -ix4 -R
    PERL_BADLANG (unset)
    PERL_CPANM_OPT=-n
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

From @shlomif

On Fri Oct 23 15​:07​:47 2015, oleg@​mamontov.net wrote​:

This is a bug report for perl from oleg@​mamontov.net,
generated with the help of perlbug 1.40 running under perl 5.22.0.

-----------------------------------------------------------------
The core List​::Util​::min works incorrectly with the snippet below​:
------------------------------
#!/usr/bin/env perl

use strict;
use warnings;

use List​::Util qw(min);

my $x = [qw( 1 1 1 1 1 )];

for ( 1..10 ) {
push @​$x, 1;
$#$x = min $#$x, 3;
warn $#$x . "\n";
}
------------------------------
result​: 3 4 5 6 7 8 9 10 11 12

I can reproduce this problem on bleadperl on Mageia Linux x86-64 v6. Seems like it has to do with the handling of "magic" values. I should also note that assigning $#$x to a "my $t = $#$x;" first and passing that to min also fixes the problem.

Regards,

-- Shlomi Fish

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

From @shlomif

On Sun Oct 25 00​:29​:12 2015, shlomif wrote​:

On Fri Oct 23 15​:07​:47 2015, oleg@​mamontov.net wrote​:

This is a bug report for perl from oleg@​mamontov.net,
generated with the help of perlbug 1.40 running under perl 5.22.0.

-----------------------------------------------------------------
The core List​::Util​::min works incorrectly with the snippet below​:
------------------------------
#!/usr/bin/env perl

use strict;
use warnings;

use List​::Util qw(min);

my $x = [qw( 1 1 1 1 1 )];

for ( 1..10 ) {
push @​$x, 1;
$#$x = min $#$x, 3;
warn $#$x . "\n";
}
------------------------------
result​: 3 4 5 6 7 8 9 10 11 12

I can reproduce this problem on bleadperl on Mageia Linux x86-64 v6.
Seems like it has to do with the handling of "magic" values. I should
also note that assigning $#$x to a "my $t = $#$x;" first and passing
that to min also fixes the problem.

«git bisect start v5.18.0 v5.16.0» eventually gives me this​:

< QUOTE >

shlomif@​telaviv1​:~/Download/unpack/perl/p5/git/perl$ git bisect good
4bac9ae is the first bad commit
commit 4bac9ae
Author​: Chip Salzenberg <chip@​pobox.com>
Date​: Fri Jun 22 15​:18​:18 2012 -0700

  Magic flags harmonization.
 
  In restore_magic(), which is called after any magic processing, all of
  the public OK flags have been shifted into the private OK flags. Thus
  the lack of an appropriate public OK flags was used to trigger both get
  magic and required conversions. This scheme did not cover ROK, however,
  so all properly written code had to make sure mg_get was called the right
  number of times anyway. Meanwhile the private OK flags gained a second
  purpose of marking converted but non-authoritative values (e.g. the IV
  conversion of an NV), and the inadequate flag shift mechanic broke this
  in some cases.
 
  This patch removes the shift mechanic for magic flags, thus exposing (and
  fixing) some improper usage of magic SVs in which mg_get() was not called
  correctly. It also has the side effect of making magic get functions
  specifically set their SVs to undef if that is desired, as the new behavior
  of empty get functions is to leave the value unchanged. This is a feature,
  as now get magic that does not modify its value, e.g. tainting, does not
  have to be special cased.
 
  The changes to cpan/ here are only temporary, for development only, to
  keep blead working until upstream applies them (or something like them).
 
  Thanks to Rik and Father C for review input.

:100644 100644 05516688ceca341ac1f63a7f72f63f38c03c18c6 ce7af44486749f954ef01ca9947a30929cd521ea M av.c
:040000 040000 e8cce61b676e554a1a21aec8aada38b7cc26616c 979996a498feb6c0af0468e3b2e2509338a04d97 M cpan
:100644 100644 fd6683da2622596820f89d44f5cdcbe845cca4a5 e5098b700200db8ea297c685b56dabd24046125a M doio.c
:100644 100644 dd6add2bc5d91feeaeac408126c514ebeb0af1fa 1593d1942ba889cc8d6a5e3545d709ee4ea107cb M doop.c
:100644 100644 eb81d9c168b2d74c6df878f943c3e89ba69e8c6d 0f19f7c58ee84793ca0d6f0d7c6c893ea1356e4b M embed.fnc
:100644 100644 5dca8e33df73a8ea47b4d3739b21f56deefbae09 a010f2dc02d224219f343159afb4f61e642dc599 M embed.h
:040000 040000 0550166d7e2deaa7b3a584c1c41d3903d148420f b3a8da6c82519b0e1b8368a90ae1eba06c0d9039 M ext
:100644 100644 e0fdc630e913b217b5a02bc6626f3eabc9c464b8 c1652852622992628a77e66ea1edd1b6271a3a77 M gv.c
:100644 100644 1c01152496b21d0d22098b5d8567a1ed04e08b76 dd8003e4c2c1023afae2fbd031b2924b48492d94 M mg.c
:100644 100644 661c055ffa8462600b4b0fdac2cdd35ff80b9130 0def4ac35f2693bd07e5a45869cfd63d15aae5d0 M pp.c
:100644 100644 826a7729384c80b4d4d06d9b8dfbcca962a6eb28 f2119a77d1e273128ba0598e0a66d0e55ccf7851 M pp_ctl.c
:100644 100644 1c0acb92b048ca5ebee12876df4ca4e38f45bbff e04d5ca7ed17719696f050da5e2edd0760079c5f M pp_hot.c
:100644 100644 f0a799e238b1aa38338d5069190c7a6ec026a06f 76bd1acbf2390cb68a5b06d26c4c21c030201caa M pp_sys.c
:100644 100644 946e9fe76eae1a1e9a39d42604bb2d8e4607b612 db957ac4561e4b639425806f2d38ed0acb64dab0 M proto.h
:100644 100644 9caaa4da54d5af559c21bad9dd07bd747eabdcb2 25de4ac77f552d59257923960c04ae9039558247 M sv.c
:100644 100644 1eeda1cc5d9568221e1f9fbb1499116b549b9c77 c841c3e246bef6a2e0883c6cde920db8a72c477a M sv.h
:040000 040000 348277f03b386dfc5c78b85d03f84da724e64f6a b3a087f58519c7355d399e014750df546351abee M t

< / QUOTE >

Regards,

-- Shlomi

Regards,

-- Shlomi Fish

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

From @ikegami

Workaround​:

splice(@​$x, 4); # Warns in older versions (<5.12?) of Perl
  -or-
splice(@​$x, 4) if @​$x > 4;

Should be faster too. (It avoids creating two magic variables.)

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

From @tonycoz

On Fri Oct 23 15​:07​:47 2015, oleg@​mamontov.net wrote​:

The core List​::Util​::min works incorrectly with the snippet below​:

Scalar-List-Utils is maintained upstream, on CPAN.

This is a bug in the slu_sv_value() macro in ListUtil.xs​:

# define slu_sv_value(sv) (SvIOK(sv)) ? (SvIOK_UV(sv)) ? (NV)(SvUVX(sv)) : (NV)(SvIVX(sv)) : (SvNV(sv))

This macro is called without checking for GMAGIC on the SV, so the flags are meaningless, changing the macro to​:

# define slu_sv_value(sv) (SvGETMAGIC(sv), SvIOK(sv)) ? (SvIOK_UV(sv)) ? (NV)(SvUVX(sv)) : (NV)(SvIVX(sv)) : (SvNV(sv))

fixes the reported case, but will result in duplicate magic calls. This can probably be fixed by replacing SvNV() with SvNV_nomg(), but I haven't done any extensive testing.

The bisect to 4bac9ae is happening because prior to that commit, IOK would not be set on a magic SV, so SvNV() would always be called in the macro, properly calling get magic.

I've opened a ticket upstream​:

https://rt.cpan.org/Ticket/Display.html?id=107970

Any further discussion belongs there.

Rejecting this ticket.

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 25, 2015

@tonycoz - Status changed from 'open' to 'rejected'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant