From malch@malch.com

This is a bug report for perl from malch@​malch.com,

Operating System​: 64-bit Windows 7.

Perl Version​: ActivePerl version 5.12.1.1201 64-bit​:

Flags​:
  category=core
  severity=high

Code to reproduce this problem​:

#====================================================

@​Big1 = ();
$Big1[4] = 0;
$Big1[9] = 1;
$Big1[6] = 2;
my $len = 0; # This appears to be significant
foreach $key (@​Big1) {
  $len = length ($key);
  print "key = $key, len=$len\n";
}
exit;

#=====================================================

Using ActivePerl version 5.12.1.1201 64-bit I get​:

C​:\ZIP>c​:\perl512\bin\perl.exe test.pl
key = , len=0
key = , len=0
key = , len=0
key = , len=0
key = 0, len=1
key = , len=1 <==== ???
key = 2, len=1
key = , len=1 <==== ???
key = , len=1 <==== ???
key = 1, len=1

Using ActivePerl version 5.10.1.1007 64-bit I get​:

C​:\ZIP>C​:\perl510\bin\perl.exe test.pl
key = , len=0
key = , len=0
key = , len=0
key = , len=0
key = 0, len=1
key = , len=0
key = 2, len=1
key = , len=0
key = , len=0
key = 1, len=1

Some other Perl programmers have taken the position that this
is a programmer error because the code attempts to establish
the length() of an undef value. That, they say, is garbage
analogous to division by zero.

I disagree and cite the following in support of my position
that this constitutes a bug​:

1. Perl Version 5.10.1.1007 and every other major Perl version
  going back 10 plus years have consistently returned zero
  for the length of an undef value.

2. perlsyn clearly states​:

  "A variable holds the undefined value ("undef") until it has been
  assigned a defined value, which is anything other than "undef".
  When used as a number, "undef" is treated as 0; when used as a
  string, it is treated as the empty string, ""; and when used as
  a reference that isn't being assigned to, it is treated as an error."

  Thus "undef" should be treated as the empty string. And the
  length of the empty string is unambiguously zero. Hence the
  behavior of the 5.12.1.1201 is clearly a bug!

Summary of my perl5 (revision 5 version 12 subversion 1) configuration​:

  Platform​:
  osname=MSWin32, osvers=5.2, archname=MSWin32-x64-multi-thread
  uname=''
  config_args='undef'
  hint=recommended, useposix=true, d_sigaction=undef
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cl', ccflags ='-nologo -GF -W3 -MD -Zi -DNDEBUG -Ox -GL -Wp64
-fp​:precis
e -DWIN32 -D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DWIN64
-DCONSERVATIVE -DUSE_
SITECUSTOMIZE -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
-DPERL_MS
VCRT_READFIX',
  optimize='-MD -Zi -DNDEBUG -Ox -GL -Wp64 -fp​:precise',
  cppflags='-DWIN32'
  ccversion='14.00.40310.41', gccversion='', gccosandvers=''
  intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8
  ivtype='__int64', ivsize=8, nvtype='double', nvsize=8,
Off_t='__int64', lsee
ksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='link', ldflags ='-nologo -nodefaultlib -debug -opt​:ref,icf
-ltcg -libpa
th​:"C​:\Perl512\lib\CORE" -machine​:AMD64'
  libpth=\lib
  libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
  comdlg32
.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib
uuid.lib ws2_
32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib
comctl32.lib buffe
roverflowU.lib msvcrt.lib
  perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib comd
lg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib
uuid.lib
ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib
comctl32.lib b
ufferoverflowU.lib msvcrt.lib
  libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl512.lib
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug
-opt​:ref,icf -l
tcg -libpath​:"C​:\Perl512\lib\CORE" -machine​:AMD64'

Characteristics of this binary (from libperl)​:
  Compile-time options​: MULTIPLICITY PERL_DONT_CREATE_GVSV
  PERL_IMPLICIT_CONTEXT PERL_IMPLICIT_SYS
  PERL_MALLOC_WRAP PL_OP_SLAB_ALLOC USE_64_BIT_INT
  USE_ITHREADS USE_LARGE_FILES USE_PERLIO
USE_PERL_ATOF
  USE_SITECUSTOMIZE
  Locally applied patches​:
  ActivePerl Build 1201 [292674]
  d956618 Make Term​::ReadLine​::findConsole fall back to STDIN if
/dev/tty
can't be opened
  321e50c Escape patch strings before embedding them in patchlevel.h
  Built under MSWin32
  Compiled at May 14 2010 00​:22​:26
  @​INC​:
  c​:/Perl512/site/lib
  c​:/Perl512/lib
  .

--
|~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~|
| Malcolm Hoar "The more I practice, the luckier I get". |
| malch@​malch.com Gary Player. |
| http​://www.malch.com/ Shpx gur PQN. |

From @ikegami

On Fri, Aug 20, 2010 at 11​:00 AM, Malcolm Hoar <perlbug-followup@​perl.org>wrote​:

# New Ticket Created by Malcolm Hoar
# Please include the string​: [perl #77336]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=77336 >

Fixed in 5.12.1 by commit 656266f

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

From ben@morrow.me.uk

Quoth Malcolm Hoar​:

@​Big1 = ();
$Big1[4] = 0;
$Big1[9] = 1;
$Big1[6] = 2;
my $len = 0; # This appears to be significant
foreach $key (@​Big1) {
$len = length ($key);
print "key = $key, len=$len\n";
}
[...]

C​:\ZIP>c​:\perl512\bin\perl.exe test.pl
key = , len=0
key = , len=0
key = , len=0
key = , len=0
key = 0, len=1
key = , len=1 <==== ???
key = 2, len=1
key = , len=1 <==== ???
key = , len=1 <==== ???
key = 1, len=1

Subsequent discussion on clpmisc has reduced the testcase to

  my $x = 3;
  $x = length(undef);
  is($x, undef);

The bug was introduced by commit 9f621b (p4 rev 32969) which made
length(undef) return undef instead of 0. That change failed to take into
account the fact that in the code above pp_length is optimised to take
the variable to assign to in TARG. Since previously pp_length only used
SETi, which sets TARG as a side-effect, this was safe; but the changed
version uses SETs(&PL_sv_undef) for the undef case which doesn't set
TARG.

Patch attached.

Ben

From ben@morrow.me.uk

From 55f8b2ade4a0143d9a1b434fd83f187c81691352 Mon Sep 17 00:00:00 2001
From: Ben Morrow <ben@morrow.me.uk>
Date: Fri, 20 Aug 2010 18:35:58 +0100
Subject: [PATCH] Fix my $x = 3; $x = length(undef);.

Assignment of length() to a lexical is optimized by passing the
assigned-to variable as TARG, avoiding a pp_padsv and pp_sassign.
9f621b which changed length(undef) to return undef didn't take this into
account, and used SETs (which doesn't set TARG), so the code above left
$x == 3.
---
 pp.c          |    9 ++++++---
 t/op/length.t |   17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pp.c b/pp.c
index 0da8bba..fcb7ff2 100644
--- a/pp.c
+++ b/pp.c
@@ -3105,8 +3105,10 @@ PP(pp_length)
 	    = sv_2pv_flags(sv, &len,
 			   SV_UNDEF_RETURNS_NULL|SV_CONST_RETURN|SV_GMAGIC);
 
-	if (!p)
-	    SETs(&PL_sv_undef);
+	if (!p) {
+	    sv_setsv(TARG, &PL_sv_undef);
+	    SETTARG;
+	}
 	else if (DO_UTF8(sv)) {
 	    SETi(utf8_length((U8*)p, (U8*)p + len));
 	}
@@ -3119,7 +3121,8 @@ PP(pp_length)
 	else
 	    SETi(sv_len(sv));
     } else {
-	SETs(&PL_sv_undef);
+	sv_setsv_nomg(TARG, &PL_sv_undef);
+	SETTARG;
     }
     RETURN;
 }
diff --git a/t/op/length.t b/t/op/length.t
index c73d4c5..705b9d5 100644
--- a/t/op/length.t
+++ b/t/op/length.t
@@ -6,7 +6,7 @@ BEGIN {
     @INC = '../lib';
 }
 
-plan (tests => 30);
+plan (tests => 36);
 
 print "not " unless length("")    == 0;
 print "ok 1\n";
@@ -193,6 +193,21 @@ my $uo = bless [], 'U';
 
 is(length($uo), undef, "Length of overloaded reference");
 
+my $ul = 3;
+is(($ul = length(undef)), undef, 
+                    "Returned length of undef with result in TARG");
+is($ul, undef, "Assigned length of undef with result in TARG");
+
+$ul = 3;
+is(($ul = length($u)), undef,
+                "Returned length of tied undef with result in TARG");
+is($ul, undef, "Assigned length of tied undef with result in TARG");
+
+$ul = 3;
+is(($ul = length($uo)), undef,
+                "Returned length of overloaded undef with result in TARG");
+is($ul, undef, "Assigned length of overloaded undef with result in TARG");
+
 # ok(!defined $uo); Turns you can't test this. FIXME for pp_defined?
 
 is($warnings, 0, "There were no warnings");
-- 
1.7.1.1

From ben@morrow.me.uk

Quoth ikegami@​adaelis.com (Eric Brine)​:

On Fri, Aug 20, 2010 at 11​:00 AM, Malcolm Hoar <perlbug-followup@​perl.org>wrote​:

# New Ticket Created by Malcolm Hoar
# Please include the string​: [perl #77336]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=77336 >

Fixed in 5.12.1 by commit 656266f

No it's not. He was *using* 5.12.1. See my patch xthread.

Ben

@ikegami - Status changed from 'open' to 'resolved'

@ikegami - Status changed from 'resolved' to 'open'

From @ikegami

On Fri Aug 20 11​:01​:00 2010, ben@​morrow.me.uk wrote​:

Quoth ikegami@​adaelis.com (Eric Brine)​:

Fixed in 5.12.1 by commit 656266f
No it's not. He was *using* 5.12.1. See my patch xthread.

I couldn't reproduce with what I thought was blead, and the mixup
snowballed from there. My apologies.
- Eric

From @csjewell

I can also reproduce with my installations of Strawberry Perl, so it's
not just ActiveState, not just 64-bit, and probably not just Windows,
either. (5.12.0.1 32-bit in a VM I'm working in, 5.12.1.0 beta 2 64-bit
on the host)

From malch@malch.com

Curtis Jewell via RT wrote​:

I can also reproduce with my installations of Strawberry Perl, so it's
not just ActiveState, not just 64-bit, and probably not just Windows,
either. (5.12.0.1 32-bit in a VM I'm working in, 5.12.1.0 beta 2 64-bit
on the host)

Interesting. Thanks for letting me know!

--
|~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~|
| Malcolm Hoar "The more I practice, the luckier I get". |
| malch@​malch.com Gary Player. |
| http​://www.malch.com/ Shpx gur PQN. |

From @jandubois

On Fri, 20 Aug 2010, Ben Morrow wrote​:

Subsequent discussion on clpmisc has reduced the testcase to

my $x = 3;
$x = length\(undef\);
is\($x\, undef\);

The bug was introduced by commit 9f621b (p4 rev 32969) which made
length(undef) return undef instead of 0. That change failed to take into
account the fact that in the code above pp_length is optimised to take
the variable to assign to in TARG. Since previously pp_length only used
SETi, which sets TARG as a side-effect, this was safe; but the changed
version uses SETs(&PL_sv_undef) for the undef case which doesn't set
TARG.

Patch attached.

Thanks, applied as #d88e091f66.

Given that it is a regression against 5.10.1 I'll nominate this as
a candidate for 5.12.2, although it may be too later for that now.

Cheers,
-Jan

From @obra

Thanks, applied as #d88e091f66.

Given that it is a regression against 5.10.1 I'll nominate this as
a candidate for 5.12.2, although it may be too later for that now.

It's not, and I +1 it :)

Cheers,
-Jan

--

From @Tux

On Fri, 20 Aug 2010 19​:46​:39 -0400, Jesse Vincent <jesse@​fsck.com>
wrote​:

Thanks, applied as #d88e091f66.

Given that it is a regression against 5.10.1 I'll nominate this as
a candidate for 5.12.2, although it may be too later for that now.

It's not, and I +1 it :)

but not yet in cherrymaint. only jdb and me have touched it for now

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using 5.00307 through 5.12 and porting perl5.13.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, OpenSuSE 10.3, 11.0, and 11.1, AIX 5.2 and 5.3.
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

From @jandubois

On Sat, 21 Aug 2010, H.Merijn Brand wrote​:

On Fri, 20 Aug 2010 19​:46​:39 -0400, Jesse Vincent <jesse@​fsck.com>
wrote​:

Thanks, applied as #d88e091f66.

Given that it is a regression against 5.10.1 I'll nominate this as
a candidate for 5.12.2, although it may be too later for that now.

It's not, and I +1 it :)

but not yet in cherrymaint. only jdb and me have touched it for now

A vote on the mailing list is good enough for me; I've now applied the
change to maint-5.12.

Just for record keeping purposes it would be good to see Jesse's +1 on
cheerymaint eventually.

Cheers,
-Jan

From @obra

On Sun, Aug 22, 2010 at 11​:44​:48AM -0700, Jan Dubois wrote​:

On Sat, 21 Aug 2010, H.Merijn Brand wrote​:

On Fri, 20 Aug 2010 19​:46​:39 -0400, Jesse Vincent <jesse@​fsck.com>
wrote​:

Thanks, applied as #d88e091f66.

Given that it is a regression against 5.10.1 I'll nominate this as
a candidate for 5.12.2, although it may be too later for that now.

It's not, and I +1 it :)

but not yet in cherrymaint. only jdb and me have touched it for now

A vote on the mailing list is good enough for me; I've now applied the
change to maint-5.12.

Just for record keeping purposes it would be good to see Jesse's +1 on
cheerymaint eventually.

Done.

Cheers,
-Jan

--

@iabyn - Status changed from 'open' to 'resolved'