[PATCH] Regression in 5.12: labelled string eval

[p5pRT]

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

Searchable as RT74290$

[p5pRT]

From @cpansprout

http​://www.nntp.perl.org/group/perl.perl5.porters/2010/04/msg158760.html

This was broken by change 33656 (http​://perl5.git.perl.org/perl.git/commitdiff/dca6062a863d0
) moves labels to the hints hash.

Here’s another example of the same bug, which shows that that it has
nothing to do with $_​:

perl5.12.0 -le 'F1​:++$x and eval q"print qq/x is $x/; exit if ++$y ==
10; goto q/F1/;"'
x is 1
x is 1
x is 1
x is 1
x is 1
x is 1
x is 1
x is 1
x is 1
x is 1

In short,
  Foo​: eval "goto Foo"
doesn’t go to Foo, but simply repeats the eval.

I tried making S_dofindlabel skip any op that == PL_eval_start, but
that prevents eval "Foo​: print q/foo/; goto Foo" from working.

In other words, the goto inside the eval cannot tell the difference
between
  Foo​: eval " ... "
and
  eval "Foo​: ... "

It seems to have something to do with %^H not being synchronised with
the equivalent internal structure, because putting BEGIN{ delete $^H
{anything} } at the beginning of the evalled code fixes it.

The attached patch is perhaps not the best fix, but works around the
bug in a way that can’t cause any negative side-effects.

Use of uninitialized value $category in concatenation (.) or string
at /usr/local/bin/perlbug5.12.0 line 645.
Use of uninitialized value $severity in concatenation (.) or string
at /usr/local/bin/perlbug5.12.0 line 645.

---

Flags​:
  category=core
  severity=medium

---

Site configuration information for perl 5.12.0​:

Configured by sprout at Tue Apr 6 15​:41​:56 PDT 2010.

Summary of my perl5 (revision 5 version 12 subversion 0 patch v5.12.0-
RC3-6-g8c57606) configuration​:
  Snapshot of​: 431be7b
  Platform​:
  osname=darwin, osvers=10.0.0, archname=darwin-2level
  uname='darwin pint.local 10.0.0 darwin kernel version 10.0.0​: fri
jul 31 22​:47​:34 pdt 2009; root​:xnu-1456.1.25~1release_i386 i386 '
  config_args='-de -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define,
usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-
strict-aliasing -pipe -fstack-protector -I/usr/local/include',
  optimize='-O3',
  cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-
precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/
include'
  ccversion='', gccversion='4.2.1 (Apple Inc. build 5646)',
gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=4, 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 -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-ldbm -ldl -lm -lutil -lc
  perllibs=-ldl -lm -lutil -lc
  libc=/usr/lib/libc.dylib, 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/
usr/local/lib -fstack-protector'

Locally applied patches​:
  RC5

---

@​INC for perl 5.12.0​:
  /usr/local/lib/perl5/site_perl/5.12.0/darwin-2level
  /usr/local/lib/perl5/site_perl/5.12.0
  /usr/local/lib/perl5/5.12.0/darwin-2level
  /usr/local/lib/perl5/5.12.0
  /usr/local/lib/perl5/site_perl
  .

---

Environment for perl 5.12.0​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/sprout
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/
usr/local/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

[p5pRT]

From @cpansprout

Inline Patch

diff -Nurp blead/pp_ctl.c blead-labelled-eval/pp_ctl.c
--- blead/pp_ctl.c	2010-04-10 14:03:15.000000000 -0700
+++ blead-labelled-eval/pp_ctl.c	2010-04-11 14:27:16.000000000 -0700
@@ -3763,6 +3763,8 @@ PP(pp_entereval)
 	HINTS_REFCNT_LOCK;
 	PL_compiling.cop_hints_hash->refcounted_he_refcnt++;
 	HINTS_REFCNT_UNLOCK;
+	PL_compiling.cop_hints_hash
+	    = Perl_store_cop_label(aTHX_ PL_compiling.cop_hints_hash, "");
     }
     /* special case: an eval '' executed within the DB package gets lexically
      * placed in the first non-DB CV rather than the current CV - this
diff -Nurp blead/t/op/eval.t blead-labelled-eval/t/op/eval.t
--- blead/t/op/eval.t	2010-04-02 12:23:09.000000000 -0700
+++ blead-labelled-eval/t/op/eval.t	2010-04-11 14:30:00.000000000 -0700
@@ -6,7 +6,7 @@ BEGIN {
     require './test.pl';
 }
 
-print "1..106\n";
+print "1..107\n";
 
 eval 'print "ok 1\n";';
 
@@ -602,3 +602,18 @@ eval q{ eval { + } };
 print "ok\n";
 EOP
 
+fresh_perl_is(<<'EOP', <<'expected', undef, 'labelled string eval');
+#!perl -l
+F1:++$x and eval q"print qq/x is $x/; exit if ++$y == 10; goto q/F1/;"
+EOP
+x is 1
+x is 2
+x is 3
+x is 4
+x is 5
+x is 6
+x is 7
+x is 8
+x is 9
+x is 10
+expected

[p5pRT]

From @nwc10

On Sun, Apr 11, 2010 at 05​:03​:41PM -0700, Father Chrysostomos wrote​:

diff -Nurp blead/pp_ctl.c blead-labelled-eval/pp_ctl.c
--- blead/pp_ctl.c 2010-04-10 14​:03​:15.000000000 -0700
+++ blead-labelled-eval/pp_ctl.c 2010-04-11 14​:27​:16.000000000 -0700
@​@​ -3763,6 +3763,8 @​@​ PP(pp_entereval)
HINTS_REFCNT_LOCK;
PL_compiling.cop_hints_hash->refcounted_he_refcnt++;
HINTS_REFCNT_UNLOCK;
+ PL_compiling.cop_hints_hash
+ = Perl_store_cop_label(aTHX_ PL_compiling.cop_hints_hash, "");
}
/* special case​: an eval '' executed within the DB package gets lexically
* placed in the first non-DB CV rather than the current CV - this

As is, that's going to leak, as the reference to the (then current)
PL_compiling.cop_hints_hash is going to be bumped once by
PL_compiling.cop_hints_hash->refcounted_he_refcnt++ and then bumped again
within Perl_store_cop_label().

The simplest solution would be to remove the preceding 3 lines.

A more efficient solution (avoiding any allocation) would be to walk back
to the parent if the topmost item on PL_compiling.cop_hints_hash is a "label".

Nicholas Clark

[p5pRT]

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

[p5pRT]

From @nwc10

On Tue, Apr 13, 2010 at 11​:31​:51AM +0100, Nicholas Clark wrote​:

On Sun, Apr 11, 2010 at 05​:03​:41PM -0700, Father Chrysostomos wrote​:

diff -Nurp blead/pp_ctl.c blead-labelled-eval/pp_ctl.c
--- blead/pp_ctl.c 2010-04-10 14​:03​:15.000000000 -0700
+++ blead-labelled-eval/pp_ctl.c 2010-04-11 14​:27​:16.000000000 -0700
@​@​ -3763,6 +3763,8 @​@​ PP(pp_entereval)
HINTS_REFCNT_LOCK;
PL_compiling.cop_hints_hash->refcounted_he_refcnt++;
HINTS_REFCNT_UNLOCK;
+ PL_compiling.cop_hints_hash
+ = Perl_store_cop_label(aTHX_ PL_compiling.cop_hints_hash, "");
}
/* special case​: an eval '' executed within the DB package gets lexically
* placed in the first non-DB CV rather than the current CV - this

As is, that's going to leak, as the reference to the (then current)
PL_compiling.cop_hints_hash is going to be bumped once by
PL_compiling.cop_hints_hash->refcounted_he_refcnt++ and then bumped again
within Perl_store_cop_label().

The simplest solution would be to remove the preceding 3 lines.

I'm wrong. Your patch as is is correct.

Thanks for correctly identifying a solution, and for the simpler test code.

A more efficient solution (avoiding any allocation) would be to walk back
to the parent if the topmost item on PL_compiling.cop_hints_hash is a "label".

However, I preferred this approach, and implemented it as 4755081.

Nicholas Clark

[p5pRT]

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