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

Bleadperl v5.23.4-97-gedba15b breaks ZEFRAM/Data-Integer-0.005.tar.gz #15049

Closed
p5pRT opened this issue Nov 13, 2015 · 12 comments
Closed

Bleadperl v5.23.4-97-gedba15b breaks ZEFRAM/Data-Integer-0.005.tar.gz #15049

p5pRT opened this issue Nov 13, 2015 · 12 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 13, 2015

Migrated from rt.perl.org#126635 (status was 'resolved')

Searchable as RT126635$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 13, 2015

From @andk

bisect


commit edba15b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Thu Oct 22 16​:43​:49 2015 +0100

  make SETi/u/n, (X)PUSHi/u/n more efficient

cpantesters


http​://www.cpantesters.org/cpan/report/e49fea62-884b-11e5-b601-b72fa9884ca8

perl -V


Summary of my perl5 (revision 5 version 23 subversion 5) configuration​:
  Commit id​: 067cec4
  Platform​:
  osname=linux, osvers=4.2.0-1-amd64, archname=x86_64-linux-ld
  uname='linux k83 4.2.0-1-amd64 #1 smp debian 4.2.5-1 (2015-10-27) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.23.4-101-g067cec4/127e -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Uuseithreads -Duselongdouble -DDEBUGGING=-g'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=define, use64bitall=define, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion='', gccversion='5.2.1 20151028', 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='long double', nvsize=16, Off_t='off_t', lseeksize=8
  alignbytes=16, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
  PERL_PRESERVE_IVUV PERL_USE_DEVEL 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_LONG_DOUBLE
  USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Nov 10 2015 15​:47​:39
  %ENV​:
  PERL="/tmp/basesmoker-reloperl-hTKI/bin/perl"
  PERL5LIB=""
  PERL5OPT=""
  PERL5_CPANPLUS_IS_RUNNING="8719"
  PERL5_CPAN_IS_RUNNING="8719"
  PERL_MM_USE_DEFAULT="1"
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.23.4-101-g067cec4/127e/lib/site_perl/5.23.5/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.23.4-101-g067cec4/127e/lib/site_perl/5.23.5
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.23.4-101-g067cec4/127e/lib/5.23.5/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.23.4-101-g067cec4/127e/lib/5.23.5
  .
--
andreas

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

From @tonycoz

On Fri Nov 13 15​:34​:29 2015, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

bisect
------
commit edba15b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Thu Oct 22 16​:43​:49 2015 +0100

make SETi/u/n, (X)PUSHi/u/n more efficient

The new macros aren't clearing SVf_IVisUV, so when a targ that was used for a UV (setting the flag) was then used for a negative IV range number, it would be treated as a large UV rather than a small IV.

Patch attached.

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

From @tonycoz

0001-perl-126635-clear-SVf_IVisUV-when-saving-a-plain-IV.patch
From 9ebffc03436ce7849d637c54ab143e5d4668764e Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 18 Nov 2015 17:22:50 +1100
Subject: [perl 126635] clear SVf_IVisUV when saving a plain IV

---
 pp.h       |  2 ++
 t/op/int.t | 14 +++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/pp.h b/pp.h
index 945d1e5..985b04b 100644
--- a/pp.h
+++ b/pp.h
@@ -385,6 +385,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
              * clear can't be set, because we're SVt_IV */              \
             assert(!(SvFLAGS(TARG) &                                    \
                 (SVf_OOK|SVf_UTF8|(SVf_OK & ~(SVf_IOK|SVp_IOK)))));     \
+            SvFLAGS(TARG) &= ~SVf_IVisUV;                               \
             SvFLAGS(TARG) |= (SVf_IOK|SVp_IOK);                         \
             /* SvIV_set() where sv_any points to head */                \
             TARG->sv_u.svu_iv = TARGi_iv;                               \
@@ -408,6 +409,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
              * clear can't be set, because we're SVt_IV */              \
             assert(!(SvFLAGS(TARG) &                                    \
                 (SVf_OOK|SVf_UTF8|(SVf_OK & ~(SVf_IOK|SVp_IOK)))));     \
+            SvFLAGS(TARG) &= ~SVf_IVisUV;                               \
             SvFLAGS(TARG) |= (SVf_IOK|SVp_IOK);                         \
             /* SvIV_set() where sv_any points to head */                \
             TARG->sv_u.svu_iv = TARGu_uv;                               \
diff --git a/t/op/int.t b/t/op/int.t
index 9aad020..dda4908 100644
--- a/t/op/int.t
+++ b/t/op/int.t
@@ -4,9 +4,10 @@ BEGIN {
     chdir 't' if -d 't';
     @INC = '../lib';
     require './test.pl';
+    require Config;
 }
 
-plan 15;
+plan 17;
 
 # compile time evaluation
 
@@ -71,3 +72,14 @@ cmp_ok($y, '==', 4745162525730, 'compile time division, result of about 42 bits'
 $y = 279964589018079;
 $y = int($y/59);
 cmp_ok($y, '==', 4745162525730, 'run time divison, result of about 42 bits');
+
+SKIP:
+{   # see #126635
+    my $large;
+    $large = eval "0xffff_ffff" if $Config::Config{ivsize} == 4;
+    $large = eval "0xffff_ffff_ffff_ffff" if $Config::Config{ivsize} == 8;
+    $large or skip "Unusual ivsize", 1;
+    for my $x ($large, -1) {
+        cmp_ok($x, "==", int($x), "check $x == int($x)");
+    }
+}
-- 
2.1.4

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

From @iabyn

On Tue, Nov 17, 2015 at 10​:25​:48PM -0800, Tony Cook via RT wrote​:

On Fri Nov 13 15​:34​:29 2015, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

bisect
------
commit edba15b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Thu Oct 22 16​:43​:49 2015 +0100

make SETi/u/n, (X)PUSHi/u/n more efficient

The new macros aren't clearing SVf_IVisUV, so when a targ that was used
for a UV (setting the flag) was then used for a negative IV range
number, it would be treated as a large UV rather than a small IV.

Patch attached.

Looks good to me; thanks.

--
Monto Blanco... scorchio!

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

From @tonycoz

On Wed Nov 18 08​:56​:10 2015, davem wrote​:

On Tue, Nov 17, 2015 at 10​:25​:48PM -0800, Tony Cook via RT wrote​:

The new macros aren't clearing SVf_IVisUV, so when a targ that was
used
for a UV (setting the flag) was then used for a negative IV range
number, it would be treated as a large UV rather than a small IV.

Patch attached.

Looks good to me; thanks.

I was thinking about it afterwards, and wondered if the attached was closer to the spirit of the original change.

I'm curious as to why the TARG[iun] macros use & instead of && for the conditional?

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 18, 2015

From @tonycoz

0001-perl-126635-don-t-shortcut-when-SVf_IVisUV-is-set.patch
From cd4c2c9944c5927af983c32cd6e78099348e58d3 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Thu, 19 Nov 2015 10:04:25 +1100
Subject: [perl #126635] don't shortcut when SVf_IVisUV is set

Most integers are small, so in most cases it won't be set.

The other option would be to always clear it, but that increases the
amount of inline code for a rare case.
---
 pp.h       |  4 ++--
 t/op/int.t | 14 +++++++++++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/pp.h b/pp.h
index 945d1e5..b92230a 100644
--- a/pp.h
+++ b/pp.h
@@ -377,7 +377,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
     STMT_START {                                                        \
         IV TARGi_iv = i;                                                \
         if (LIKELY(                                                     \
-              ((SvFLAGS(TARG) & (SVTYPEMASK|SVf_THINKFIRST)) == SVt_IV) \
+              ((SvFLAGS(TARG) & (SVTYPEMASK|SVf_THINKFIRST|SVf_IVisUV)) == SVt_IV) \
             & (do_taint ? !TAINT_get : 1)))                             \
         {                                                               \
             /* Cheap SvIOK_only().                                      \
@@ -399,7 +399,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
     STMT_START {                                                        \
         UV TARGu_uv = u;                                                \
         if (LIKELY(                                                     \
-              ((SvFLAGS(TARG) & (SVTYPEMASK|SVf_THINKFIRST)) == SVt_IV) \
+              ((SvFLAGS(TARG) & (SVTYPEMASK|SVf_THINKFIRST|SVf_IVisUV)) == SVt_IV) \
             & (do_taint ? !TAINT_get : 1)                               \
             & (TARGu_uv <= (UV)IV_MAX)))                                \
         {                                                               \
diff --git a/t/op/int.t b/t/op/int.t
index 9aad020..dda4908 100644
--- a/t/op/int.t
+++ b/t/op/int.t
@@ -4,9 +4,10 @@ BEGIN {
     chdir 't' if -d 't';
     @INC = '../lib';
     require './test.pl';
+    require Config;
 }
 
-plan 15;
+plan 17;
 
 # compile time evaluation
 
@@ -71,3 +72,14 @@ cmp_ok($y, '==', 4745162525730, 'compile time division, result of about 42 bits'
 $y = 279964589018079;
 $y = int($y/59);
 cmp_ok($y, '==', 4745162525730, 'run time divison, result of about 42 bits');
+
+SKIP:
+{   # see #126635
+    my $large;
+    $large = eval "0xffff_ffff" if $Config::Config{ivsize} == 4;
+    $large = eval "0xffff_ffff_ffff_ffff" if $Config::Config{ivsize} == 8;
+    $large or skip "Unusual ivsize", 1;
+    for my $x ($large, -1) {
+        cmp_ok($x, "==", int($x), "check $x == int($x)");
+    }
+}
-- 
2.1.4

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 19, 2015

From @iabyn

On Wed, Nov 18, 2015 at 03​:08​:15PM -0800, Tony Cook via RT wrote​:

I was thinking about it afterwards, and wondered if the attached was
closer to the spirit of the original change.

Yes, that looks better.

I'm curious as to why the TARG[iun] macros use & instead of && for the
conditional?

Its to avoid doing an extra conditionals with the risk of branch
prediction misses.

--
I don't want to achieve immortality through my work...
I want to achieve it through not dying.
  -- Woody Allen

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 24, 2015

From @tonycoz

On Thu Nov 19 01​:42​:24 2015, davem wrote​:

On Wed, Nov 18, 2015 at 03​:08​:15PM -0800, Tony Cook via RT wrote​:

I was thinking about it afterwards, and wondered if the attached was
closer to the spirit of the original change.

Yes, that looks better.

Applied as 2efdfb1.

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 24, 2015

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented May 13, 2016

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.
 
With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented May 13, 2016

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT closed this May 13, 2016
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.