s///g fails when using English & study in 5.8.0 #5979
Comments
From chrisd@apache.orgHi -- I first noticed this behaviour in Perl 5.6.1 in the I just downloaded and compiled the current stable Perl, Specifically, here's my test program: #!/usr/home/fooperl/bin/perl This produces the incorrect output: this X matches, this x doesn't However, it produces the correct output if any of three Here's my installation information: Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration: Characteristics of this binary (from libperl): Chris. |
From @eserteChris Darroch (via RT) <perlbug@perl.org> writes:
Here's a simple-minded fix. It just disables study if $& is used in Regards, Inline Patch--- pp.c.orig Fri Oct 4 21:20:04 2002
+++ pp.c Fri Oct 4 21:20:34 2002
@@ -613,6 +613,9 @@ PP(pp_study)
register I32 *snext;
STRLEN len;
+ if (PL_sawampersand)
+ RETPUSHNO;
+
if (sv == PL_lastscream) {
if (SvSCREAM(sv))
RETPUSHYES;
--- t/op/study.t.orig Fri Oct 4 21:24:41 2002
+++ t/op/study.t Fri Oct 4 21:33:01 2002
@@ -47,7 +47,7 @@ sub alarm_ok (&) {
}
-print "1..26\n";
+print "1..27\n";
$x = "abc\ndef\n";
study($x);
@@ -119,3 +119,12 @@ if ($^O eq 'os390' or $^O eq 'posix-bc'
alarm_ok { /[F]F$/ };
}
+eval q{
+ # test whether $& and study work together
+ my($f) = "this x matches, this x doesn't\n";
+ study $f;
+ $f =~ s/x/X/g;
+ $&;
+ ok($f eq "this X matches, this X doesn't\n");
+};
+die $@ if $@;
-- Slaven Rezic - slaven.rezic@berlin.de |
From @eserteNicholas Clark <nick@unfortu.net> writes:
It's q{ ... }, not { ... }, so it's only seen at eval time. Regards, -- |
From @nwc10On Fri, Oct 04, 2002 at 11:06:07PM +0200, Slaven Rezic wrote:
Oh yes. D'oh But it does mean that any tests that get added after it might be affected. Nicholas Clark |
From @nwc10On Fri, Oct 04, 2002 at 09:59:21PM +0200, Slaven Rezic wrote:
Are you sure you want to do the regression test like this? Nicholas Clark |
From @schwernOn Fri, Oct 04, 2002 at 11:51:09PM +0100, Nicholas Clark wrote:
Use the fresh_perl_is() and fresh_perl_like() stuff from test.pl to run the -- Michael G. Schwern <schwern@pobox.com> http://www.pobox.com/~schwern/ |
From @eserteMichael G Schwern <schwern@pobox.com> writes:
OK, a new patch. I included a comment in pp.c (suggested by Tels) and Regards, Inline Patch--- pp.c.orig Fri Oct 4 21:20:04 2002
+++ pp.c Sat Oct 5 09:49:52 2002
@@ -613,6 +613,10 @@ PP(pp_study)
register I32 *snext;
STRLEN len;
+ /* workaround for [perl #17757] */
+ if (PL_sawampersand)
+ RETPUSHNO;
+
if (sv == PL_lastscream) {
if (SvSCREAM(sv))
RETPUSHYES;
--- t/op/study.t.orig Fri Oct 4 21:24:41 2002
+++ t/op/study.t Sat Oct 5 10:06:12 2002
@@ -3,28 +3,12 @@
BEGIN {
chdir 't' if -d 't';
@INC = '../lib';
+ require './test.pl';
}
-$Ok_Level = 0;
-my $test = 1;
-sub ok ($;$) {
- my($ok, $name) = @_;
-
- local $_;
-
- # You have to do it this way or VMS will get confused.
- printf "%s $test%s\n", $ok ? 'ok' : 'not ok',
- $name ? " - $name" : '';
-
- printf "# Failed test at line %d\n", (caller($Ok_Level))[2] unless $ok;
-
- $test++;
- return $ok;
-}
-
+use strict;
sub nok ($;$) {
my($nok, $name) = @_;
- local $Ok_Level = 1;
ok( !$nok, $name );
}
@@ -42,14 +26,13 @@ sub alarm_ok (&) {
alarm(0) if $have_alarm;
};
- local $Ok_Level = 1;
ok( !$match && !$@, 'testing studys that used to hang' );
}
-print "1..26\n";
+print "1..27\n";
-$x = "abc\ndef\n";
+my $x = "abc\ndef\n";
study($x);
ok($x =~ /^abc/);
@@ -108,8 +91,9 @@ ok("ab\ncd\n" =~ /^cd/);
if ($^O eq 'os390' or $^O eq 'posix-bc' or $^O eq 'MacOS') {
# Even with the alarm() OS/390 and BS2000 can't manage these tests
# (Perl just goes into a busy loop, luckily an interruptable one)
- for (25..26) { print "not ok $_ # TODO compiler bug?\n" }
- $test += 2;
+ SKIP: {
+ skip("TODO compiler bug?", 2);
+ }
} else {
# [ID 20010618.006] tests 25..26 may loop
@@ -119,3 +103,11 @@ if ($^O eq 'os390' or $^O eq 'posix-bc'
alarm_ok { /[F]F$/ };
}
+fresh_perl_is(q{
+ # test whether $& and study work together
+ my($f) = "this x matches, this x doesn't";
+ study $f;
+ $f =~ s/x/X/g;
+ $&;
+ print $f;
+}, "this X matches, this X doesn't", {}, "study and ampersand");
-- Slaven Rezic - slaven.rezic@berlin.de |
From @hvdsSlaven Rezic <slaven.rezic@berlin.de> wrote: I don't think that is the right approach - I don't understand why the Hugo |
From @hvdsChris Darroch (via RT) <perlbug@perl.org> wrote: First, note that when perl is compiling code, if it ever sees Now, each time we enter the regexp matching engine, we pass in Most of the time we don't actually look into sv at all, but in The patch takes a simplistic approach to getting it right, by Hugo Inline Patch |
From @hvdshv@crypt.org wrote: Now applied to the development sources as change #18533. Hugo |
From @jhiSince the patch got applied I'm marking the problem ticket as resolved. |
|
@jhi - Status changed from 'new' to 'resolved' |
From david.dyck@fluke.comOn Sun, 2 Mar 2003 at 14:32 +0100, Andreas J. Koenig <andreas.koenig@anima....:
If I apply patch 18533 in reverse to regexec.c, then the infinite loop Change 18533 by hv@hv-crypt.org on 2003/01/21 02:15:29 Subject: Re: [perl #17757] s///g fails when using English & study in 5.8.0 Affected files ... ... //depot/perl/regexec.c#297 edit Differences ... ==== //depot/perl/regexec.c#297 (text) ==== Inline Patch--- perl/regexec.c#296~18529~ Mon Jan 20 16:44:20 2003
+++ perl/regexec.c Mon Jan 20 18:15:29 2003
@@ -541,6 +541,9 @@
start_shift + (s - strbeg), end_shift, pp, 0);
else
goto fail_finish;
+ /* we may be pointing at the wrong string */
+ if (s && RX_MATCH_COPIED(prog))
+ s = prog->subbeg + (s - SvPVX(sv));
if (data)
*data->scream_olds = s;
}
@@ -1858,6 +1861,9 @@
: (s = fbm_instr((unsigned char*)HOP3(s, back_min, strend),
(unsigned char*)strend, must,
PL_multiline ? FBMrf_MULTILINE : 0))) ) {
+ /* we may be pointing at the wrong string */
+ if ((flags & REXEC_SCREAM) && RX_MATCH_COPIED(prog))
+ s = prog->subbeg + (s - SvPVX(sv));
DEBUG_r( did_match = 1 );
if (HOPc(s, -back_max) > last1) {
last1 = HOPc(s, -back_min);
@@ -1944,6 +1950,9 @@
end_shift, &scream_pos, 1); /* last one */
if (!last)
last = scream_olds; /* Only one occurrence. */
+ /* we may be pointing at the wrong string */
+ else if (RX_MATCH_COPIED(prog))
+ s = prog->subbeg + (s - SvPVX(sv));
}
else {
STRLEN len; |
From @hvdsDavid Dyck <david.dyck@fluke.com> wrote: I spent some time on this tonight without getting very far. It would help a lot to remove some of the module dependencies in the The simplest I had with Inline was: .. and I'm guessing that it is Inline itself that is requiring the I probably won't have much time to look further into this for the next Hugo |
From david.dyck@fluke.comOn Mon, 3 Mar 2003 at 06:02 -0000, hv@crypt.org wrote:
Thanks, I've been using this trying different ways to simplify it, |
From david.dyck@fluke.comOn Fri, 7 Mar 2003 at 21:32 -0800, David Dyck <david.dyck@fluke.com> wrote:
When I apply this patch (that I think is an optimization to Inline::C::ParseRecDescent) I'm still hoping that this information will trigger some ideas Any takers? Inline Patch--- Inline-0.44/C/lib/Inline/C/ParseRecDescent.pm Mon Nov 4 13:39:09 2002
+++ Inline-0.44-3/C/lib/Inline/C/ParseRecDescent.pm Sat Mar 8 20:13:22 2003
@@ -9,6 +9,7 @@
}
}
+our $parser;
sub get_parser {
my $o = shift;
eval { require Parse::RecDescent };
@@ -17,7 +18,11 @@
$@
END
$main::RD_HINT++;
- Parse::RecDescent->new(grammar())
+ if ($parser) {
+ delete $parser->{data} if exists $parser->{data};
+ return $parser;
+ }
+ $parser = Parse::RecDescent->new(grammar());
}
sub grammar { |
From david.dyck@fluke.comOn Sat, 8 Mar 2003 at 20:19 -0800, David Dyck <david.dyck@fluke.com> wrote:
The following code using Parse::RecDescent does I think that this shows that the problem is not in What has changed since 5.8.0 that causes this problem? David use strict; # derived from Inline::C::ParseRecDescent sub get_parser { sub grammar { code: part(s) part: comment comment: function_definition: function_declaration: rtype: rtype1 | rtype2 rtype1: modifier(s?) TYPE star(s?) rtype2: modifier(s) star(s?) arg: type IDENTIFIER {[@item[1,2]]} arg_decl: type: type1 | type2 type1: modifier(s?) TYPE star(s?) type2: modifier(s) star(s?) modifier: star: '*' IDENTIFIER: TYPE: /\w+/ anything_else: END BEGIN { $::RD_TRACE = 1; } get_parser()->code('void true() { }'); |
From @hvdsDavid Dyck <david.dyck@fluke.com> wrote: Here's another step of simplification: #!/usr/bin/perl -w sub get_parser { sub grammar { BEGIN { $::RD_TRACE = 1; } get_parser()->code('x'); If I copy the Parse::RecDescent module into a new file and require The construction of the string also seems important: if I replace the Hugo |
From enache@rdslink.roOn Mon, Mar 10, 2003 at 05:56:17AM +0000, hv@crypt.org wrote:
The infinite loop happens in re_intuit_start. It always 'goto restart' s = fbm_instr( ... It's caused by this: Removing those lines stops cures that infinite loop; Subject: Re: [perl #17757] s///g fails when using English & study in 5.8.0 Hoping this helps, Regards |
From david.dyck@fluke.comOn Wed, 19 Mar 2003 at 02:11 +0200, Enache Adrian <enache@rdslink.ro> wrote:
Thank you for looking into this further. I removed the code you mentioned and it causes some failures in t/op/subst...........................# Failed at op/subst.t line 517 and t/op/subst_wamp......................# Failed at ./op/subst.t line 517 I guess this will get worked out before 5.8.1 gets released, right? |
From @rgsDavid Dyck <david.dyck@fluke.com> wrote:
I think Hugo reached the same conclusion.
If it isn't, the patch should be withdrawn from 5.8.1. I'd call this bug |
From @jhi
Withdrawn for now (change #19031). -- |
From david.dyck@fluke.comOn Wed, 19 Mar 2003 at 23:07 +0200, Jarkko Hietaniemi <jhi@iki.fi> wrote:
What about change 18533 in perl 5.9.0 ? Should 5.8.1 and 5.9.0 be consistent in their treatment of David |
From @jhiOn Thu, Mar 20, 2003 at 06:14:28AM -0800, David Dyck wrote:
There's much more time until 5.9.0 than time until 5.8.1.
-- |
From @RandalSchwartz
Jarkko> There's much more time until 5.9.0 than time until 5.8.1. I missed the full analysis. Is it that P::RD depends on a bug in the That is, I'm unsure why "moving forward" breaks things. Is it just a Then again, this is the maze-that-is-regex-code, eh? -- |
From @hvdsmerlyn@stonehenge.com (Randal L. Schwartz) wrote: I think it has been established only that a problem occurs in P::RD It hasn't been established yet precisely what is going wrong, but it Hugo |
From enache@rdslink.roOn Fri, Mar 21, 2003 at 12:29:18AM +0000, hv@crypt.org wrote:
The faulty regexp in RecDescent.pm is this, at line 1856 The second time the _generate method runs, fbm_instr() will be called If that pointer happens to be behind 't', an infinite loop happens. P::RD could be easily 'fixed' <:( by removing the 'study $grammar' Regards |
From inaba@st.rim.or.jpEnache Adrian wrote:
I think attached patch will adjust the #18533.
I failed to make small test for the problem. |
From inaba@st.rim.or.jpInline Patch--- regexec.c.org 2003-03-16 16:05:31.000000000 +0900
+++ regexec.c 2003-03-21 16:47:43.000000000 +0900
@@ -544,7 +544,7 @@
goto fail_finish;
/* we may be pointing at the wrong string */
if (s && RX_MATCH_COPIED(prog))
- s = prog->subbeg + (s - SvPVX(sv));
+ s = strbeg + (s - SvPVX(sv));
if (data)
*data->scream_olds = s;
}
@@ -1862,7 +1862,7 @@
PL_multiline ? FBMrf_MULTILINE : 0))) ) {
/* we may be pointing at the wrong string */
if ((flags & REXEC_SCREAM) && RX_MATCH_COPIED(prog))
- s = prog->subbeg + (s - SvPVX(sv));
+ s = strbeg + (s - SvPVX(sv));
DEBUG_r( did_match = 1 );
if (HOPc(s, -back_max) > last1) {
last1 = HOPc(s, -back_min);
@@ -1951,7 +1951,7 @@
last = scream_olds; /* Only one occurrence. */
/* we may be pointing at the wrong string */
else if (RX_MATCH_COPIED(prog))
- s = prog->subbeg + (s - SvPVX(sv));
+ s = strbeg + (s - SvPVX(sv));
}
else {
STRLEN len; |
From inaba@st.rim.or.jpInaba Hiroto wrote:
I think I made it. |
From inaba@st.rim.or.jpInline Patch--- t/op/pat.t.org 2003-03-16 16:05:32.000000000 +0900
+++ t/op/pat.t 2003-03-22 11:13:33.000000000 +0900
@@ -6,7 +6,7 @@
$| = 1;
-print "1..996\n";
+print "1..997\n";
BEGIN {
chdir 't' if -d 't';
@@ -3159,4 +3159,11 @@
ok(join(":", /\b(.)\x{100}/g) eq "a:/", "re_intuit_start and PL_bostr");
}
-# last test 996
+{
+ $_ = "code: 'x' { '...' }\n"; study;
+ my @x; push @x, $& while m/'[^\']*'/gx;
+ ok(join(":", @x) eq "'x':'...'",
+ "[perl #17757] Parse::RecDescent triggers infinete loop");
+}
+
+# last test 997 |
From @rgsInaba Hiroto wrote:
Thanks, applied (with your test) as #19210. -- |
|
@cwest - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#17757 (status was 'resolved')
Searchable as RT17757$
The text was updated successfully, but these errors were encountered: