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

segfault with s/(.)\G//g #9864

Closed
p5pRT opened this issue Sep 8, 2009 · 16 comments
Closed

segfault with s/(.)\G//g #9864

p5pRT opened this issue Sep 8, 2009 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 8, 2009

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

Searchable as RT69056$

@p5pRT
Copy link
Author

p5pRT commented Sep 8, 2009

From @ntyni

This is a bug report for perl from Niko Tyni <ntyni@​debian.org>,
generated with the help of perlbug 1.39 running under perl 5.11.0.


As reported by Raphael Geissert in <http​://bugs.debian.org/545234>​:

$ echo foo | perl -pe "s/(.)\G//g"

crashes on 5.10.0, 5.10.1 and current blead.

Backtrace​:

Core was generated by `./miniperl -pe s/(.)\G//g'.
Program terminated with signal 11, Segmentation fault.
[New process 30959]
#0 0x00007f970786761e in _wordcopy_bwd_aligned () from /lib/libc.so.6
(gdb) bt
#0 0x00007f970786761e in _wordcopy_bwd_aligned () from /lib/libc.so.6
#1 0x00007f9707866422 in memmove () from /lib/libc.so.6
#2 0x0000000000540cc0 in Perl_sv_catpvn_flags (my_perl=0x12c1010, dsv=0x12e3688, sstr=0x12dcde8 "",
  slen=18446744073709551615, flags=2) at sv.c​:4732
#3 0x000000000050d08f in Perl_pp_subst (my_perl=0x12c1010) at pp_hot.c​:2319
#4 0x00000000004b3b64 in Perl_runops_debug (my_perl=0x12c1010) at dump.c​:2044
#5 0x00000000006f4964 in S_run_body (my_perl=0x12c1010, oldscope=1) at perlmini.c​:2286
#6 0x00000000006f3c80 in perl_run (my_perl=0x12c1010) at perlmini.c​:2211
#7 0x00000000006c5c52 in main (argc=3, argv=0x7fff10631fb8, env=0x7fff10631fd8) at miniperlmain.c​:117



Flags​:
  category=core
  severity=low


Site configuration information for perl 5.11.0​:

Configured by Debian Project at Tue Sep 8 09​:40​:07 EEST 2009.

Summary of my perl5 (revision 5 version 11 subversion 0) configuration​:
  Commit id​: d1ce36d
  Platform​:
  osname=linux, osvers=2.6.26-2-openvz-amd64, archname=x86_64-linux-gnu-thread-multi
  uname='linux minerva 2.6.26-2-openvz-amd64 #1 smp wed aug 19 23​:15​:49 utc 2009 x86_64 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.11 -Darchlib=/usr/lib/perl/5.11 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.11.0 -Dsitearch=/usr/local/lib/perl/5.11.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=both -Doptimize=-O2 -O0 -Dusedevel -Uuseshrplib -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -O0 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.3.2', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lgdbm -ldb -ldl -lm -lcrypt -lpthread -lc -lgdbm_compat
  perllibs=-ldl -lm -lcrypt -lpthread -lc
  libc=/lib/libc-2.7.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.7'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -O0 -g -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.11.0​:
  lib
  /usr/local/lib/perl/5.11.0
  /usr/local/share/perl/5.11.0
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.11
  /usr/share/perl/5.11
  .


Environment for perl 5.11.0​:
  HOME=/home/niko
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LC_CTYPE=fi_FI.UTF-8
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/niko/bin​:/home/niko/bin​:/home/niko/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/games​:/sbin​:/usr/sbin
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Sep 9, 2009

From @demerphq

Hi, thanks for the report.

I have resolved this in bleadperl with commit​:

http​://perl5.git.perl.org/perl.git/commitdiff/c584a96ef5d

commit c584a96
Author​: Yves Orton <demerphq@​gmail.com>
Date​: Wed Sep 9 14​:20​:10 2009 +0200

  Fix RT69056 - postive GPOS leads to segv on first match
 
  http​://rt.perl.org/rt3/Ticket/Display.html?id=69056
 
  In perl 5.8 we get this​:
 
  $ perl -Mre=debug -le '$_="foo"; s/(.)\G//g; print'
  Freeing REx​: `","'
  Compiling REx `(.)\G'
  size 7 Got 60 bytes for offset annotations.
  first at 3
  1​: OPEN1(3)
  3​: REG_ANY(4)
  4​: CLOSE1(6)
  6​: GPOS(7)
  7​: END(0)
  GPOS minlen 1
  Offsets​: [7]
  1[1] 0[0] 2[1] 3[1] 0[0] 4[2] 6[0]
  Matching REx `(.)\G' against `foo'
  Setting an EVAL scope, savestack=3
  0 <> <foo> | 1​: OPEN1
  0 <> <foo> | 3​: REG_ANY
  1 <f> <oo> | 4​: CLOSE1
  1 <f> <oo> | 6​: GPOS
  failed...
  Setting an EVAL scope, savestack=3
  1 <f> <oo> | 1​: OPEN1
  1 <f> <oo> | 3​: REG_ANY
  2 <fo> <o> | 4​: CLOSE1
  2 <fo> <o> | 6​: GPOS
  failed...
  Setting an EVAL scope, savestack=3
  2 <fo> <o> | 1​: OPEN1
  2 <fo> <o> | 3​: REG_ANY
  3 <foo> <> | 4​: CLOSE1
  3 <foo> <> | 6​: GPOS
  failed...
  Setting an EVAL scope, savestack=3
  3 <foo> <> | 1​: OPEN1
  3 <foo> <> | 3​: REG_ANY
  failed...
  Match failed
  foo
  Freeing REx​: `"(.)\\G"'
 
  In perl 5.10 we get this​:
 
  $ perl -Mre=debug -le '$_="foo"; s/(.)\G//g; print'
  Compiling REx "(.)\G"
  Final program​:
  1​: OPEN1 (3)
  3​: REG_ANY (4)
  4​: CLOSE1 (6)
  6​: GPOS (7)
  7​: END (0)
  anchored(GPOS) GPOS​:1 minlen 1
  Matching REx "(.)\G" against "foo"
  -1 <> <%0foo> | 1​:OPEN1(3)
  -1 <> <%0foo> | 3​:REG_ANY(4)
  0 <> <foo> | 4​:CLOSE1(6)
  0 <> <foo> | 6​:GPOS(7)
  0 <> <foo> | 7​:END(0)
  Match successful!
  Segmentation fault
 
  With this patch we get​:
 
  $ ./perl -Ilib -Mre=debug -le '$_="foo"; s/(.)\G//g; print'
  Compiling REx "(.)\G"
  Final program​:
  1​: OPEN1 (3)
  3​: REG_ANY (4)
  4​: CLOSE1 (6)
  6​: GPOS (7)
  7​: END (0)
  anchored(GPOS) GPOS​:1 minlen 1
  Matching REx "(.)\G" against "foo"
  Match failed
  foo
  Freeing REx​: "(.)\G"
 
  Which seems to me to be a net improvement.

@p5pRT
Copy link
Author

p5pRT commented Sep 9, 2009

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

@p5pRT
Copy link
Author

p5pRT commented Sep 9, 2009

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

@p5pRT p5pRT closed this as completed Sep 9, 2009
@p5pRT
Copy link
Author

p5pRT commented Sep 9, 2009

From @Corion

Hello,

I can reproduce the bug on Win32 - attached is a patch with a test file.
I'm not sure where it should go, so I put the test into t/op where
Schwern already put another test where he wasn't sure where it should've
gone instead.

-max

@p5pRT
Copy link
Author

p5pRT commented Sep 9, 2009

From @Corion

0001-Add-test-for-perl-69056.patch
From f65553dbe4d6038b46a3e6d510e2d8becf9d903f Mon Sep 17 00:00:00 2001
From: Max Maischein <corion@corion.net>
Date: Wed, 9 Sep 2009 23:48:23 +0200
Subject: [PATCH] Add test for [perl #69056]

---
 t/op/rt69056.t |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
 create mode 100644 t/op/rt69056.t

diff --git a/t/op/rt69056.t b/t/op/rt69056.t
new file mode 100644
index 0000000..23c4f08
--- /dev/null
+++ b/t/op/rt69056.t
@@ -0,0 +1,22 @@
+#!perl -w
+
+=head1 DESCRIPTION
+
+This test tests against a regular expression bug
+that leads to a segfault
+
+The bug was reported in [perl #69056] by Niko Tyni
+
+=cut
+
+use strict;
+use Test::More tests => 1;
+
+TODO: {
+    local $TODO = 'Fix regex engine segfault';
+
+    $_='foo';
+    s/(.)\G//g;
+
+    ok(1, 'Did not segfault');
+};
\ No newline at end of file
-- 
1.6.0.2.1172.ga5ed0

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @demerphq

2009/9/9 Max Maischein <corion@​corion.net>​:

Hello,

I can reproduce the bug on Win32 - attached is a patch with a test file. I'm
not sure where it should go, so I put the test into t/op where Schwern
already put another test where he wasn't sure where it should've gone
instead.

Can you reproduce it after my patch?

Also the test, since it involves a segv perl, should probably be run
in its own process. There is some support for that in test.pl,
test_fresh_perl() or something like that i think?

Hrm, or maybe it doesnt matter as the script is standalone. Not sure
about this one. Anybody else have an opinion?

Schwern?

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @nwc10

On Thu, Sep 10, 2009 at 11​:15​:19AM +0200, demerphq wrote​:

Also the test, since it involves a segv perl, should probably be run
in its own process. There is some support for that in test.pl,
test_fresh_perl() or something like that i think?

Hrm, or maybe it doesnt matter as the script is standalone. Not sure
about this one. Anybody else have an opinion?

I'd prefer fresh_perl_is() or fresh_perl_like() over a proliferation of
small test scripts.

IIRC Schwern wrote them to remove t/op/misc.t, which had been the home for
might-SEGV tests, and thus allow tests to be moved back with their logical
counterparts in other test scripts.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @demerphq

2009/9/10 Nicholas Clark <nick@​ccl4.org>​:

On Thu, Sep 10, 2009 at 11​:15​:19AM +0200, demerphq wrote​:

Also the test, since it involves a segv perl, should probably be run
in its own process. There is some support for that in test.pl,
test_fresh_perl() or something like that i think?

Hrm, or maybe it doesnt matter as the script is standalone. Not sure
about this one. Anybody else have an opinion?

I'd prefer fresh_perl_is() or fresh_perl_like() over a proliferation of
small test scripts.

IIRC Schwern wrote them to remove t/op/misc.t, which had been the home for
might-SEGV tests, and thus allow tests to be moved back with their logical
counterparts in other test scripts.

Yes, this should probably go in a new test file called
t/op/reg_subst.t or something like that, if it doesn't exist.

Or possibly t/op/pat.t, but id like to see that split up in multiple
smaller test files anyway I think.

Can we maybe create t/op/regex/ and put anything intended to test
regex functionality in there? Would that cause too many maint issues?

cheers
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @nwc10

On Thu, Sep 10, 2009 at 11​:33​:54AM +0200, demerphq wrote​:

Yes, this should probably go in a new test file called
t/op/reg_subst.t or something like that, if it doesn't exist.

Or possibly t/op/pat.t, but id like to see that split up in multiple
smaller test files anyway I think.

Can we maybe create t/op/regex/ and put anything intended to test
regex functionality in there? Would that cause too many maint issues?

No, but probably t/regex/ would be better, as regexps are smeared across
several ops, and two C files.

IIRC new directories need to be added to t/TEST and t/harness, and there
seems to be some text in pod/perlhack.pod, but that's about it.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @demerphq

2009/9/10 Nicholas Clark <nick@​ccl4.org>​:

On Thu, Sep 10, 2009 at 11​:33​:54AM +0200, demerphq wrote​:

Yes, this should probably go in a new test file called
t/op/reg_subst.t or something like that, if it doesn't exist.

Or possibly t/op/pat.t, but id like to see that split up in multiple
smaller test files anyway I think.

Can we maybe create t/op/regex/ and put anything intended to test
regex functionality in there? Would that cause too many maint issues?

No, but probably t/regex/ would be better, as regexps are smeared across
several ops, and two C files.

Ah, good point, I had been think of "t/op" as "test internal perl
stuff" and not as "test ops", which is in hindsight a better
interpretation.

IIRC new directories need to be added to t/TEST and t/harness, and there
seems to be some text in pod/perlhack.pod, but that's about it.

Seems like, what was the term you used, "a small self contained task
that can be in done in a couple of a hours in an evening" :-)

So ill give it a crack.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @Corion

demerphq schrieb​:

2009/9/9 Max Maischein <corion@​corion.net>​:

Hello,

I can reproduce the bug on Win32 - attached is a patch with a test file.
Can you reproduce it after my patch?
No - the test runs OK at
  7fa8eabb7f2f64c581678a218bedcb3cd07de960 - Replace the uuencoded
perlexe.ico.packd with the genuine, binary, article.
which is five or six commits after
  c584a96 - Fix RT69056 - postive GPOS
leads to segv on first match

Also the test, since it involves a segv perl, should probably be run
in its own process. There is some support for that in test.pl,
test_fresh_perl() or something like that i think?
Ah - I hadn't seen that yet. Attached is a second version that runs
using fresh_perl_is(). It still goes into t/op.

-max

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @Corion

0001-Add-test-for-perl-69056-2nd-try.patch
From 908fda5755f791c36e08e69f397e4a24de0e1a28 Mon Sep 17 00:00:00 2001
From: Max Maischein <corion@corion.net>
Date: Thu, 10 Sep 2009 13:15:02 +0200
Subject: [PATCH] Add test for [perl #69056], 2nd try

---
 t/op/rt69056.t |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 t/op/rt69056.t

diff --git a/t/op/rt69056.t b/t/op/rt69056.t
new file mode 100644
index 0000000..bbbc1ed
--- /dev/null
+++ b/t/op/rt69056.t
@@ -0,0 +1,19 @@
+#!perl -w
+
+=head1 DESCRIPTION
+
+This test tests against a regular expression bug
+that leads to a segfault
+
+The bug was reported in [perl #69056] by Niko Tyni
+
+=cut
+
+use strict;
+require 't/test.pl';
+
+fresh_perl_is(
+    '$_=q(foo);s/(.)\G//g;print'
+        => 'foo', 
+    '[perl #69056] positive GPOS regex segfault'
+);
-- 
1.6.0.2.1172.ga5ed0

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From ams@toroid.org

At 2009-09-10 13​:15​:48 +0200, corion@​corion.net wrote​:

Attached is a second version that runs using fresh_perl_is(). It still
goes into t/op.

Thanks, applied. (#ac93a5c)

-- ams

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @demerphq

2009/9/10 Abhijit Menon-Sen <ams@​toroid.org>​:

At 2009-09-10 13​:15​:48 +0200, corion@​corion.net wrote​:

Attached is a second version that runs using fresh_perl_is(). It still
goes into t/op.

Thanks, applied. (#ac93a5c)

And reverted/changed with 831a7dd so
that the test runs from t/op/substr.t

I used your name for the commit Max.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Sep 10, 2009

From @demerphq

2009/9/10 Nicholas Clark <nick@​ccl4.org>​:

On Thu, Sep 10, 2009 at 11​:33​:54AM +0200, demerphq wrote​:

Yes, this should probably go in a new test file called
t/op/reg_subst.t or something like that, if it doesn't exist.

Or possibly t/op/pat.t, but id like to see that split up in multiple
smaller test files anyway I think.

Can we maybe create t/op/regex/ and put anything intended to test
regex functionality in there? Would that cause too many maint issues?

No, but probably t/regex/ would be better, as regexps are smeared across
several ops, and two C files.

FYI​: I created t/re/ and moved the regex tests that were in t/op/ to
it with commit a449955.

http​://perl5.git.perl.org/perl.git/commitdiff/a4499558e

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

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