Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 555 forks source link

corrupt memory (since @8531, in 5.6.1-T2) #3499

Closed p5pRT closed 20 years ago

p5pRT commented 23 years ago

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

Searchable as RT5935$

p5pRT commented 23 years ago

From @jhi

Trying to add a couple of tests to op/pat unearthed a nasty bug that has been there at least since bleadperl@​8531 (the oldest snapshot I had around)\, and it is also in 5.6.1-T2 :-( It is not in 5.7.0.

Here's a script that tickles the bug in Alpha (thanks for Inaba Hiroto for helping me out in cutting down the pat.t)

reset; if (0) {   if ("" =~ //) {   } }

and here's a script that tickles the bug in x86 Linux

$ENV{BAR} = 0; reset; if (0) {   if ("" =~ //) {   } }

Want mystery? Change the if (0) to if (1) and the corruption seems to go away. This makes the bug smell like a compile time problem\, and Third Degree log (shown soon) says the same.

Since the bug seems to be some sort of a memory corruption (accessing already freed memory on)\, it is unlikely to show the same symptoms everywhere. If you use debugging\, the problem might go away. Trying to cut down the script proved to be a rather elusive sport. Just in case my Linux is different from your Linux\, here's the script that "worked" for Inaba Hiroto​:

$ENV{BAR} = 0; reset; if (0) {   if ("\x00" !~ /[0]/) {   #foobarbazfoobarbaz   } }

Here's the stack trace for Alpha Tru64 (x86 Linux shows the same stack trace)​:

GDB is free software and you are welcome to distribute copies of it under certain conditions; type "show copying" to see the conditions. There is absolutely no warranty for GDB; type "show warranty" for details. GDB 4.16 (alpha-dec-osf4.0)\, Copyright 1996 Free Software Foundation\, Inc... Core was generated by `perl'. Program terminated with signal 11\, Segmentation fault. Reading symbols from /tmp/jhi/perl/libperl.so...done. Reading symbols from /usr/shlib/libm.so...done. Reading symbols from /usr/shlib/libiconv.so...done. Reading symbols from /usr/shlib/libc.so...done. Reading symbols from /usr/lib/nls/loc//fi_FI.ISO8859-1...done. #0 Perl_sv_reset (s=0x3ffffff1d40 ""\, stash=0x1400024b0) at sv.c​:5797 5797 pm->op_pmdynflags &= ~PMdf_USED; (gdb) where #0 Perl_sv_reset (s=0x3ffffff1d40 ""\, stash=0x1400024b0) at sv.c​:5797 #1 0x3ffbffac378 in Perl_pp_reset () at pp_ctl.c​:1715 #2 0x3ffbff607c0 in Perl_runops_debug () at run.c​:53 #3 0x3ffbfeeb084 in S_run_body (oldscope=1) at perl.c​:1458 #4 0x3ffbfeead14 in perl_run (my_perl=0x140002280) at perl.c​:1380 #5 0x120001f44 in main (argc=2\, argv=0x11ffff448\, env=0x11ffff460)   at perlmain.c​:52 (gdb) p *pm Cannot access memory at address 0x2. (gdb) list 5792 if (!stash) 5793 return; 5794 5795 if (!*s) { /* reset ?? searches */ 5796 for (pm = HvPMROOT(stash); pm; pm = pm->op_pmnext) { 5797 pm->op_pmdynflags &= ~PMdf_USED; 5798 } 5799 return; 5800 } 5801 (gdb)

Here's are the suspicious looking Third Degree log entries from running the script show to "work" for Alpha. It seems that something frees the PMOP but sv_reset() expects ito be there\, KABOOM ensues.

---------------------------------------------------------------- rih -- 8 -- sv.c​: 5797​: reading invalid heap at byte 92 of 96-byte block   Perl_sv_reset libperl.so\, sv.c\, line 5797   Perl_pp_reset libperl.so\, pp_ctl.c\, line 1715   Perl_runops_debug libperl.so\, run.c\, line 53   S_run_body libperl.so\, perl.c\, line 1458   perl_run libperl.so\, perl.c\, line 1380   main perl\, perlmain.c\, line 52   __start perl

This block at address 0x1400279f0 was allocated at​:   malloc libc.so   Perl_safemalloc libperl.so\, util.c\, line 87   Perl_newPMOP libperl.so\, op.c\, line 2905   S_scan_pat libperl.so\, toke.c\, line 6160   Perl_yylex libperl.so\, toke.c\, line 3594   Perl_yyparse libperl.so\, perly.c\, line 1432   S_parse_body libperl.so\, perl.c\, line 1304   perl_parse libperl.so\, perl.c\, line 886   main perl\, perlmain.c\, line 50   __start perl

This block was freed at​:   free libc.so   Perl_safefree libperl.so\, util.c\, line 160   Perl_op_free libperl.so\, op.c\, line 758   Perl_op_free libperl.so\, op.c\, line 739   Perl_op_free libperl.so\, op.c\, line 739   Perl_op_free libperl.so\, op.c\, line 739   S_new_logop libperl.so\, op.c\, line 3673   Perl_newLOGOP libperl.so\, op.c\, line 3634   Perl_newCONDOP libperl.so\, op.c\, line 3756   Perl_yyparse libperl.so\, perly.c\, line 1563   S_parse_body libperl.so\, perl.c\, line 1304   perl_parse libperl.so\, perl.c\, line 886   main perl\, perlmain.c\, line 50   __start perl

---------------------------------------------------------------- rih -- 9 -- sv.c​: 5796​: reading invalid heap at byte 72 of 96-byte block   Perl_sv_reset libperl.so\, sv.c\, line 5796   Perl_pp_reset libperl.so\, pp_ctl.c\, line 1715   Perl_runops_debug libperl.so\, run.c\, line 53   S_run_body libperl.so\, perl.c\, line 1458   perl_run libperl.so\, perl.c\, line 1380   main perl\, perlmain.c\, line 52   __start perl

This block at address 0x1400279f0 was allocated at​:   malloc libc.so   Perl_safemalloc libperl.so\, util.c\, line 87   Perl_newPMOP libperl.so\, op.c\, line 2905   S_scan_pat libperl.so\, toke.c\, line 6160   Perl_yylex libperl.so\, toke.c\, line 3594   Perl_yyparse libperl.so\, perly.c\, line 1432   S_parse_body libperl.so\, perl.c\, line 1304   perl_parse libperl.so\, perl.c\, line 886   main perl\, perlmain.c\, line 50   __start perl

This block was freed at​:   free libc.so   Perl_safefree libperl.so\, util.c\, line 160   Perl_op_free libperl.so\, op.c\, line 758   Perl_op_free libperl.so\, op.c\, line 739   Perl_op_free libperl.so\, op.c\, line 739   Perl_op_free libperl.so\, op.c\, line 739   S_new_logop libperl.so\, op.c\, line 3673   Perl_newLOGOP libperl.so\, op.c\, line 3634   Perl_newCONDOP libperl.so\, op.c\, line 3756   Perl_yyparse libperl.so\, perly.c\, line 1563   S_parse_body libperl.so\, perl.c\, line 1304   perl_parse libperl.so\, perl.c\, line 886   main perl\, perlmain.c\, line 50   __start perl

Perl Info ``` Flags: category=core severity=high Site configuration information for perl v5.7.0: Configured by jhi at Thu Mar 1 18:57:21 EET 2001. Summary of my perl5 (revision 5.0 version 7 subversion 0) configuration: Platform: osname=dec_osf, osvers=4.0f, archname=alpha-dec_osf uname='osf1 kosh.hut.fi v4.0 1229 alpha ' config_args='-des -Dusedevel -Doptimize=-g -Uusemymalloc' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=define uselongdouble=undef Compiler: cc='cc', ccflags ='-std -DDEBUGGING -DLANGUAGE_C', optimize='-g', cppflags='-std -DDEBUGGING -DLANGUAGE_C' ccversion='V5.9-010', gccversion='', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, usemymalloc=n, prototype=define Linker and Libraries: ld='ld', ldflags ='' libpth=/usr/shlib /usr/ccs/lib /usr/lib/cmplrs/cc /usr/lib /var/shlib libs=-lgdbm -ldbm -ldb -lm -liconv -lutil perllibs=-lm -liconv -lutil libc=/usr/shlib/libc.so, so=so, useshrplib=true, libperl=libperl.so Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' -Wl,-rpath,/usr/local/lib/perl5/5.7.0/alpha-dec_osf/CORE' cccdlflags=' ', lddlflags='-shared -expect_unresolved "*" -g -msym -std' Locally applied patches: DEVEL8956 @INC for perl v5.7.0: lib /u/vieraat/vieraat/jhi/Perl/lib /usr/local/lib/perl5/5.7.0/alpha-dec_osf /usr/local/lib/perl5/5.7.0 /usr/local/lib/perl5/site_perl/5.7.0/alpha-dec_osf /usr/local/lib/perl5/site_perl/5.7.0 /usr/local/lib/perl5/site_perl . Environment for perl v5.7.0: HOME=/u/vieraat/vieraat/jhi LANG=C LANGUAGE (unset) LC_ALL=fi_FI.ISO8859-1 LC_CTYPE=fi_FI.ISO8859-1 LD_LIBRARY_PATH=/u/vieraat/vieraat/jhi/pp4/perl LOGDIR (unset) PATH=/u/vieraat/vieraat/jhi/Perl/bin:/u/vieraat/vieraat/jhi/.s:/u/vieraat/vieraat/jhi/.b/OSF1:/c/bin:/p/bin:/p/adm/bin:/usr/bin:/usr/sbin:/sbin:/bin:/usr/ccs/bin:/usr/lib:/etc:/lib:/p/X6/bin:/p/X5/bin:/usr/bin/X11:/usr/lbin:/usr/sbin/acct:/usr/tcb/bin:/tcb/bin:/usr/field:/u/vieraat/vieraat/jhi PERLLIB=/u/vieraat/vieraat/jhi/Perl/lib PERL_BADLANG (unset) SHELL=/bin/zsh ```
p5pRT commented 23 years ago

From @andk

On Thu\, 1 Mar 2001 19​:22​:10 +0200 (EET)\, jhi@​cc.hut.fi (Jarkko Hietaniemi) said​:

  > and here's a script that tickles the bug in x86 Linux

  > $ENV{BAR} = 0;   > reset;   > if (0) {   > if ("" =~ //) {   > }   > }

Works on my Linux (i.e. no output). Please tell us what kind of error it produces for you\, maybe I can find a variation that fails for me too. Only then I could do a binary search through all patches since 5.7.0.

p5pRT commented 23 years ago

From @jhi

On Sat\, Mar 03\, 2001 at 11​:12​:03AM +0100\, Andreas J. Koenig wrote​:

On Thu\, 1 Mar 2001 19​:22​:10 +0200 (EET)\, jhi@​cc.hut.fi (Jarkko Hietaniemi) said​:

and here's a script that tickles the bug in x86 Linux

$ENV{BAR} = 0; reset; if (0) { if ("" =~ //) { } }

Works on my Linux (i.e. no output). Please tell us what kind of error

A core dump\, stack trace showing sv_reset() accessing already freed memory of an PMOP.

it produces for you\, maybe I can find a variation that fails for me

Try

  $ENV{"BAR"} = 0;

  reset;

  if (0) {   if ("\x00" !~ /[0]/) {   #foobarbazfoobarbaz   }   }

with -Mwarnings.

too. Only then I could do a binary search through all patches since 5.7.0.

-- andreas

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 10​:18​:21AM -0600\, Jarkko Hietaniemi wrote​:

On Sat\, Mar 03\, 2001 at 11​:12​:03AM +0100\, Andreas J. Koenig wrote​:

On Thu\, 1 Mar 2001 19​:22​:10 +0200 (EET)\, jhi@​cc.hut.fi (Jarkko Hietaniemi) said​:

and here's a script that tickles the bug in x86 Linux

$ENV{BAR} = 0; reset; if (0) { if ("" =~ //) { } }

Works on my Linux (i.e. no output). Please tell us what kind of error

A core dump\, stack trace showing sv_reset() accessing already freed memory of an PMOP.

it produces for you\, maybe I can find a variation that fails for me

Try

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

(gdb) where #0 0x2070760 in Perl_sv_reset () #1 0x208bf38 in Perl_pp_reset () #2 0x205ec04 in Perl_runops_standard () #3 0x2016dc4 in S_run_body () #4 0x2016a38 in perl_run () #5 0x2013d20 in main () #6 0x40091e8c in __libc_start_main () from /lib/libc.so.6

actually\, doesn't SEGV with -Mwarnings\, and the original program doesn't SEGV with and without

Summary of my perl5 (revision 5.0 version 7 subversion 0) configuration​:   Platform​:   osname=linux\, osvers=2.4.0-rmk3\, archname=armv4l-linux   uname='linux bagpuss.unfortu.net 2.4.0-rmk3 #3 sat jan 20 21​:38​:55 gmt 2001 armv4l unknown '   config_args='-Dmksymlinks -Dusedevel -Ubincompat5005 -Uinstallusrbinperl -Dcf_email=nick@​talking.bollo.cx -Dperladmin=nick@​talking.bollo.cx -Dinc_version_list= -Dinc_version_list_init=0 -de'   hint=recommended\, useposix=true\, d_sigaction=define   usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef   useperlio=define d_sfio=undef uselargefiles=define usesocks=undef   use64bitint=undef use64bitall=undef uselongdouble=undef   Compiler​:   cc='cc'\, ccflags ='-fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\,   optimize='-O2'\,   cppflags='-fno-strict-aliasing -I/usr/local/include'   ccversion=''\, gccversion='2.95.2 20000220 (Debian GNU/Linux)'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=8   ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=4\, usemymalloc=n\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags =' -L/usr/local/lib'   libpth=/usr/local/lib /lib /usr/lib   libs=-lnsl -lndbm -ldb -ldl -lm -lc -lposix -lcrypt -lutil   perllibs=-lnsl -ldl -lm -lc -lposix -lcrypt -lutil   libc=/lib/libc-2.1.3.so\, so=so\, useshrplib=false\, libperl=libperl.a   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-rdynamic'   cccdlflags='-fpic'\, lddlflags='-shared -L/usr/local/lib'

Characteristics of this binary (from libperl)​:   Compile-time options​: USE_LARGE_FILES   Locally applied patches​:   DEVEL8890   Built under linux   Compiled at Feb 24 2001 15​:05​:00   @​INC​:   /usr/local/lib/perl5/5.7.0/armv4l-linux   /usr/local/lib/perl5/5.7.0   /usr/local/lib/perl5/site_perl/5.7.0/armv4l-linux   /usr/local/lib/perl5/site_perl/5.7.0   /usr/local/lib/perl5/site_perl   .

Nicholas Clark

p5pRT commented 23 years ago

From @jhi

Try

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

(gdb) where #0 0x2070760 in Perl_sv_reset () #1 0x208bf38 in Perl_pp_reset () #2 0x205ec04 in Perl_runops_standard () #3 0x2016dc4 in S_run_body () #4 0x2016a38 in perl_run () #5 0x2013d20 in main () #6 0x40091e8c in __libc_start_main () from /lib/libc.so.6

actually\, doesn't SEGV with -Mwarnings\, and the original program doesn't SEGV with and without

Good to know I'm not hallucinating :-) To be pedantic\, the *original* (long) thing is attached in this message\, what you call original (the shortest version) is what I cut down as much as I could and still core dumps in Alpha.

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

p5pRT commented 23 years ago

From @jhi

mystery.t.gz

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 11​:00​:10AM -0600\, Jarkko Hietaniemi wrote​:

Try

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

and coredumps on 5.6.0 with and without -Mwarnings I don't have a 5.6.1-trial-anything to hand.

Good to know I'm not hallucinating :-) To be pedantic\, the *original* (long) thing is attached in this message\, what you call original (the shortest version) is what I cut down as much as I could and still core dumps in Alpha.

Heisenbug.

./perl will pass /usr/local/bin/perl will SEGV they are byte for byte identical

As usual\, I don't trust my CPU - it's probably page faults hitting hardware bugs.

Nicholas Clark

p5pRT commented 23 years ago

From @jhi

and coredumps on 5.6.0 with and without -Mwarnings

Whoa! On 5.6.0? That would re-orient our bug hunt considerably...

Heisenbug.

./perl will pass /usr/local/bin/perl will SEGV they are byte for byte identical

As usual\, I don't trust my CPU - it's probably page faults hitting hardware bugs.

Rats. So the 5.6.0 coredump may be a heisencore?

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 11​:26​:15AM -0600\, Jarkko Hietaniemi wrote​:

and coredumps on 5.6.0 with and without -Mwarnings

Whoa! On 5.6.0? That would re-orient our bug hunt considerably...

Yes. See below. plum can coredump it on 5.6.0 5.005_03 doesn't

Heisenbug.

./perl will pass /usr/local/bin/perl will SEGV they are byte for byte identical

As usual\, I don't trust my CPU - it's probably page faults hitting hardware bugs.

Rats. So the 5.6.0 coredump may be a heisencore?

No. alwayscore. (or whatever a non-heisencore is) coredump with and without -Mwarnings on FreeBSD 4.2-STABLE on x86

Nicholas Clark

p5pRT commented 23 years ago

From @jhi

On Sat\, Mar 03\, 2001 at 11​:26​:15AM -0600\, Jarkko Hietaniemi wrote​:

and coredumps on 5.6.0 with and without -Mwarnings

Whoa! On 5.6.0? That would re-orient our bug hunt considerably...

Holy mackerel\, Batman! I do get a core dump at sv_reset() on vanilla 5.6.0 in (Sparc) Solaris (7) using the most minimal script

reset; if (0) {   if ("" =~ //) {   } }

5.6.0 in (x86) Linux and Alpha do not core on that (my usual platforms\, so I stupidly didn't test further.)

So it would seem that the bug is of older lineage that we though.

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 05​:34​:43PM +0000\, Nicholas Clark wrote​:

On Sat\, Mar 03\, 2001 at 11​:26​:15AM -0600\, Jarkko Hietaniemi wrote​:

and coredumps on 5.6.0 with and without -Mwarnings

Whoa! On 5.6.0? That would re-orient our bug hunt considerably...

Yes. See below. plum can coredump it on 5.6.0 5.005_03 doesn't   on arm

5.005_02 and 03 do on FreeBSD and 5.004 on armlinux (both mystery.t and the short coredump script)

Summary of my perl5 (5.0 patchlevel 5 subversion 3) configuration​:   Platform​:   osname=freebsd\, osvers=4.0-current\, archname=i386-freebsd   uname='FreeBSD freefall.FreeBSD.org 4.0-current FreeBSD 4.0-current #0​: $Date$'   hint=recommended\, useposix=true\, d_sigaction=define   usethreads=undef useperlio=undef d_sfio=undef   Compiler​:   cc='cc'\, optimize='undef'\, gccversion=2.95.2 19991024 (release)   cppflags=''   ccflags =''   stdchar='char'\, d_stdstdio=undef\, usevfork=true   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12   alignbytes=4\, usemymalloc=n\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags ='-Wl\,-E -lperl -lm '   libpth=/usr/lib   libs=-lm -lc -lcrypt   libc=\, so=so\, useshrplib=true\, libperl=libperl.so.3   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags=' -Wl\,-R/usr/lib'   cccdlflags='-DPIC -fpic'\, lddlflags='-Wl\,-E -shared -lperl -lm '

Characteristics of this binary (from libperl)​:   Built under freebsd   Compiled at Dec 5 2000 02​:24​:55   @​INC​:   /usr/libdata/perl/5.00503/mach   /usr/libdata/perl/5.00503   /usr/local/lib/perl5/site_perl/5.005/i386-freebsd   /usr/local/lib/perl5/site_perl/5.005   .

I don't have anything earlier than 5.004_05 to hand.

Nicholas Clark

p5pRT commented 23 years ago

From @jhi

5.005_03 doesn't on arm

5.005_02 and 03 do on FreeBSD and 5.004 on armlinux (both mystery.t and the short coredump script)

Wow. So we are talking a vintage bug here. Can you shave the short coredump-inducing script any shorter?

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 11​:51​:17AM -0600\, Jarkko Hietaniemi wrote​:

5.005_03 doesn't on arm

5.005_02 and 03 do on FreeBSD and 5.004 on armlinux (both mystery.t and the short coredump script)

Wow. So we are talking a vintage bug here. Can you shave the short coredump-inducing script any shorter?

$ENV{TERM} = 0; reset;
// if 0;

all 4 things are needed seemingly in this order

assignment to %ENV (new or existing entry) reset // or ?? false if\, so that the peephole optimiser can optimise it away.

cores on 5.004_05 on armlinux\, and 5.6 and 5.005_02 on FreeBSD x86

Nicholas Clark

p5pRT commented 23 years ago

From @jhi

On Sat\, Mar 03\, 2001 at 06​:06​:43PM +0000\, Nicholas Clark wrote​:

On Sat\, Mar 03\, 2001 at 11​:51​:17AM -0600\, Jarkko Hietaniemi wrote​:

5.005_03 doesn't on arm

5.005_02 and 03 do on FreeBSD and 5.004 on armlinux (both mystery.t and the short coredump script)

Wow. So we are talking a vintage bug here. Can you shave the short coredump-inducing script any shorter?

$ENV{TERM} = 0; reset;
// if 0;

all 4 things are needed seemingly in this order

assignment to %ENV (new or existing entry) reset // or ?? false if\, so that the peephole optimiser can optimise it away.

cores on 5.004_05 on armlinux\, and 5.6 and 5.005_02 on FreeBSD x86

Ack on sparc solaris 7 with 5.6.0 (don't have any older ones in there)\, and on x86 debian 2.2 with 5.6.0\, no core on tru64 with 5.6.0.

The %ENV may (or may not\, aren't heisenbugs fun) be a red herring\, since this

reset; if (0) {   if ("" =~ //) {   } }

did core on alpha with bleedperl and 5.6.1-to-be.

p5pRT commented 23 years ago

From @nwc10

On Sat\, Mar 03\, 2001 at 12​:11​:42PM -0600\, Jarkko Hietaniemi wrote​:

The %ENV may (or may not\, aren't heisenbugs fun) be a red herring\, since this

reset; if (0) { if ("" =~ //) { } }

did core on alpha with bleedperl and 5.6.1-to-be.

I can't make it core without %ENV. However\, it's nothing to do with which variables you match on\, or which you reset​:

$ENV{TERM} = 0; reset $a; $b =~ // if 0

[back to 5.004_05]

Nicholas Clark

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

and coredumps on 5.6.0 with and without -Mwarnings I don't have a 5.6.1-trial-anything to hand.

This dumps core in the very same way as in memory corruption that was discovered by me recently for "#define ENV_IS_CASELESS" systems. It was caused by freeing same chunk of memory twice - once by Safefree(var)\, and once much later via FREETMPS.

About current dump core - I've almost cathed it in debugger.

This makes me think that it will be usefull to implement a way to catch such cases in general - to track allocs and frees (in DEBUG mode) and catch cases when something is freed twice.

Best wishes\, \<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From @jhi

This dumps core in the very same way as in memory corruption that was discovered by me recently for "#define ENV_IS_CASELESS" systems. It was caused by freeing same chunk of memory twice - once by Safefree(var)\, and once much later via FREETMPS.

About current dump core - I've almost caught it in debugger.

Well\, if you take a look at my original bug report

http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2001-03/msg00021.html

and if one believes Third Degree\, I've got the stack trace of the at least one free() moment. In this case it *seems* to be the case of free()ing too early\, not a double free().

p5pRT commented 23 years ago

From @AlanBurlison

jhi@​cc.hut.fi wrote​:

reset; if (0) { if ("" =~ //) { } }

Here's are the suspicious looking Third Degree log entries from running the script show to "work" for Alpha. It seems that something frees the PMOP but sv_reset() expects ito be there\, KABOOM ensues.

Confirmed in pureperl with purify\, if it helps​:

FMR​: Free memory read​:   * This is occurring while in​:   Perl_sv_reset [sv.c​:5076]   Perl_pp_reset [pp_ctl.c​:1666]   Perl_runops_debug [run.c​:53]   S_run_body [perl.c​:1476]   perl_run [perl.c​:1398]   main [perlmain.c​:52]   _start [crt1.o]   * Reading 1 byte from 0x1fb3b4 in the heap.   * Address 0x1fb3b4 is 52 bytes into a freed block at 0x1fb380 of 56 bytes.   * This block was allocated from​:   malloc [malloc.c]   Perl_safesysmalloc [util.c​:85]   Perl_newPMOP [op.c​:2892]   S_scan_pat [toke.c​:6161]   Perl_yylex [toke.c​:3594]   Perl_yyparse [perly.c​:1432]   S_parse_body [perl.c​:1322]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]   * There have been 26 frees since this block was freed from​:   free [malloc.c]   Perl_safesysfree [util.c​:158]   Perl_op_free [op.c​:757]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   S_new_logop [op.c​:3656]   Perl_newLOGOP [op.c​:3617]   Perl_newCONDOP [op.c​:3739]   Perl_yyparse [perly.y​:212]   S_parse_body [perl.c​:1322]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]

Alan Burlison

p5pRT commented 23 years ago

From @AlanBurlison

Jarkko Hietaniemi wrote​:

$ENV{TERM} = 0; reset; // if 0;

all 4 things are needed seemingly in this order

assignment to %ENV (new or existing entry) reset // or ?? false if\, so that the peephole optimiser can optimise it away.

cores on 5.004_05 on armlinux\, and 5.6 and 5.005_02 on FreeBSD x86

Ack on sparc solaris 7 with 5.6.0 (don't have any older ones in there)\, and on x86 debian 2.2 with 5.6.0\, no core on tru64 with 5.6.0.

The %ENV may (or may not\, aren't heisenbugs fun) be a red herring\, since this

With a FMR it will be tickled differently by any sort of heap activity\, and the assignment to ENV will come into that category. I guess you'll get all sorts of random strangeness\, depending on exactly what has gone into the freed-but-still-referenced block\, and where it is in relation to other allocated blocks.

Alan Burlison

p5pRT commented 23 years ago

From @AlanBurlison

BTW\, it then goes on to write into the freed block. From that point onwards all bets are off - it could dump core anywhere.

Alan Burlison

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

and coredumps on 5.6.0 with and without -Mwarnings I don't have a 5.6.1-trial-anything to hand.

I found the reason of dump core\, but need more digging to be able to fix this.

Structures "struct pmop" in op.h ("pm" stands for pattern matching here) are linked via the field   PMOP * op_pmnext; /* list of all scanpats */ and perl sometimes go through this link starting from HvPMROOT(stash). Here is excerpt from function Perl_sv_reset(sv.c)​: if (!*s) { /* reset ?? searches */   for (pm = HvPMROOT(stash); pm; pm = pm->op_pmnext) {   pm->op_pmdynflags &= ~PMdf_USED;   }   return; }

The reason of memory corrupt is - that list is not re-built when that OP freed.

namely\, that op get freed in the function S_new_logop(op.c)\, lines​: [....]   if (first->op_type == OP_CONST) { if (ckWARN(WARN_BAREWORD) && (first->op_private & OPpCONST_BARE))   Perl_warner(aTHX_ WARN_BAREWORD\, "Bareword found in conditional"); if ((type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv))) {   op_free(first);   *firstp = Nullop;   return other; } else {   op_free(other); /*\<===here it's freed*/   *otherp = Nullop;   return first; }   }

As I see following transformations occured​:

if (0) { /[pattern-matching-op]/} ==> 0 OP_AND /[pattern-matching-op]/ ==>   0 \<== but here that PM-OP is freed but link became broken long after that "pp_reset" tries to walk and dumps core...

Function S_op_clear(in op.c) should have code for rebuilding link of that OP if OP is pattern-matching op.

Currently\, I have no courage to implement this. The courage and time for implementing and testing will appear only at Wednesday\, so anyone with more experience please check my explanations and fix the code.

Best wishes\, \<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

This dumps core in the very same way as in memory corruption that was discovered by me recently for "#define ENV_IS_CASELESS" systems. It was caused by freeing same chunk of memory twice - once by Safefree(var)\, and once much later via FREETMPS.

About current dump core - I've almost caught it in debugger.

Well\, if you take a look at my original bug report

http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2001-03/msg00021.html

and if one believes Third Degree\, I've got the stack trace of the at least one free() moment. In this case it *seems* to be the case of free()ing too early\, not a double free().

agreed -- my first feeling lied to me... But it seems to that I catched the real reason of problem\, please see my another letter with the same subject for explanations.

Best wishes\, \<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

$ENV\{"BAR"\} = 0;

reset;

if \(0\) \{
  if \("\\x00" \!~ /\[0\]/\) \{
    \#foobarbazfoobarbaz
  \}
\}

with -Mwarnings.

works on arm linux without needing -Mwarnings

and coredumps on 5.6.0 with and without -Mwarnings I don't have a 5.6.1-trial-anything to hand.

I found the reason of dump core\, but need more digging to be able to fix this.

Structures "struct pmop" in op.h ("pm" stands for pattern matching here) are linked via the field PMOP * op_pmnext; /* list of all scanpats */ and perl sometimes go through this link starting from HvPMROOT(stash). Here is excerpt from function Perl_sv_reset(sv.c)​: if (!*s) { /* reset ?? searches */ for (pm = HvPMROOT(stash); pm; pm = pm->op_pmnext) { pm->op_pmdynflags &= ~PMdf_USED; } return; }

The reason of memory corrupt is - that list is not re-built when that OP freed.

namely\, that op get freed in the function S_new_logop(op.c)\, lines​: [....] if (first->op_type == OP_CONST) { if (ckWARN(WARN_BAREWORD) && (first->op_private & OPpCONST_BARE)) Perl_warner(aTHX_ WARN_BAREWORD\, "Bareword found in conditional"); if ((type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv))) { op_free(first); *firstp = Nullop; return other; } else { op_free(other); /*\<===here it's freed*/ *otherp = Nullop; return first; } }

As I see following transformations occured​:

if (0) { /[pattern-matching-op]/} ==> 0 OP_AND /[pattern-matching-op]/ ==> 0 \<== but here that PM-OP is freed but link became broken long after that "pp_reset" tries to walk and dumps core...

Function S_op_clear(in op.c) should have code for rebuilding link of that OP if OP is pattern-matching op.

Currently\, I have no courage to implement this. The courage and time for implementing and testing will appear only at Wednesday\, so anyone with more experience please check my explanations and fix the code.

Finally I've found enough courage to produce a patch (for perl@​8958)\, as I understand the problem.

I am not sure for this patch\, so feel free to improve it. More specifically\, I do not know how to get right stash for that chain of OPs. The commented text should not exist\, as well\, as label "Done_this" - this is all debugging and "very bad" is printed if I uncomment it :(

But. That error gone (for me\, at least)\, and this patch does not seem to introduce test fails.

\<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

op.c.diff

p5pRT commented 23 years ago

From @AlanBurlison

Nicholas Clark wrote​:

Ah. But with all the "magic" tools any idea what's causing the free memory read? Presumably with the three line program you can take the lines out and see what removes the FMR

Or is that too obvious and you've already found neither rhyme nor reason in when one gets a FMR as a function of perl statements?

Ahah - Purify 101 time :-) Well\, let's look at the stack trace​:

FMR​: Free memory read​:   * This is occurring while in​:   Perl_sv_reset [sv.c​:5076]   Perl_pp_reset [pp_ctl.c​:1666]   Perl_runops_debug [run.c​:53]   S_run_body [perl.c​:1476]   perl_run [perl.c​:1398]   main [perlmain.c​:52]   _start [crt1.o]

So let's set a breakpoint on purify_stop_here so we stop the point of the FMR\, and then run perl with the -Dlts flags​:

EXECUTING...

  =>
(/tmp/z​:0) enter (/tmp/z​:0) ENTER scope 2 at pp_hot.c​:1535 Entering block 0\, type BLOCK   =>
(/tmp/z​:0) nextstate   =>
(/tmp/z​:2) reset

So it is obviously the 'reset' statement that is causing the FMR. Now let's take a peek at the code in Perl_sv_reset​:  
  if (!*s) { /* reset ?? searches */   for (pm = HvPMROOT(stash); pm; pm = pm->op_pmnext) { => pm->op_pmdynflags &= ~PMdf_USED;   }   return;   }

So it is the assignment to op_pmdynflags that is actually triggering the error. Ok\, now let's look at where the memory was freed up​:

  There have been 26 frees since this block was freed from​:   free [malloc.c]   Perl_safesysfree [util.c​:158]   Perl_op_free [op.c​:757]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   S_new_logop [op.c​:3656]   Perl_newLOGOP [op.c​:3617]   Perl_newCONDOP [op.c​:3739]   Perl_yyparse [perly.y​:212]   S_parse_body [perl.c​:1322]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]

Examination of Perl_op_free shows that it just does a recursive delete of the OP - which doesn't actually tell us *why* the op was freed. Scooting up the stack to S_new_logop\, we find this​:

  if (first->op_type == OP_CONST) {   if (ckWARN(WARN_BAREWORD) && (first->op_private & OPpCONST_BARE))   Perl_warner(aTHX_ WARN_BAREWORD\, "Bareword found in conditional");   if ((type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv))) {   op_free(first);   *firstp = Nullop;   return other;   }   else { => op_free(other);   *otherp = Nullop;   return first;   }   }

Ahah - so the logic here seems to be saying that if the first (LHS) parameter of the logical operator is a constant we can short-circuit the answer and avoid evaluating one or other of the two operands. In this particular case the logic says we can work out the answer in this case just by inspecting the LHS\, so the RHS (other) can just be deleted. Let's see if that tallies with the perl source code​:

reset; if (0) {   if ("" =~ //) {   } }

Hmm\, could be either one of those 2 'if' statements\, and that's bourne out if we look at the stack trace of where the block was freed​:

  There have been 26 frees since this block was freed from​:   free [malloc.c]   Perl_safesysfree [util.c​:158]   Perl_op_free [op.c​:757]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   Perl_op_free [op.c​:738]   S_new_logop [op.c​:3656]   Perl_newLOGOP [op.c​:3617]   Perl_newCONDOP [op.c​:3739]   Perl_yyparse [perly.y​:212]  
  cond : IF '(' remember mexpr ')' mblock else   { PL_copline = $1;   => $$ = block_end($3\,   newCONDOP(0\, $4\, scope($6)\, $7)); }   | UNLESS '(' remember miexpr ')' mblock else   { PL_copline = $1;   S_parse_body [perl.c​:1322]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]

Which 'if' statement actually triggers the free isn't important\, we know that it is freed\, and why. What we really need to know is why 'reset' is stomping on it after it has been freed. Let's revisit where the UMR takes place\, in Perl_sv_reset​:

  if (!*s) { /* reset ?? searches */   for (pm = HvPMROOT(stash); pm; pm = pm->op_pmnext) { => pm->op_pmdynflags &= ~PMdf_USED;   }   return;   }

Ahah! We are getting close now. Best guess​: the block is being pointed at by HvPMROOT(stash)\, or something dangling off it. I have no idea what HvPMROOT(stash) is\, but let's set a breakpoint in Perl_sv_reset and have a dig around​:

(dbx) print pm pm = 0x1fb380

And now let's compare that with the Purify UMR report​:

  Reading 1 byte from 0x1fb3b4 in the heap.   Address 0x1fb3b4 is 52 bytes into a freed block at 0x1fb380 of 56 bytes.

Bingo! Same block address. Now we are at the point just before the UMR\, let's ask Purify about the contents of the 'pm' variable just to make absolutely sure​:

(dbx) call purify_describe(pm)

  Address 0x1fb380 is in the heap.   Address 0x1fb380 is at the beginning of a freed block of 56 bytes.

So\, at some point HvPMROOT(stash) is made to point to the the miscreant block\, the block is freed and the reference from HvPMROOT(stash) is then doomed to cause trouble whenever it is used - in this case in Perl_sv_reset.

Ok\, now let's track that sucker down. Firstly\, let's figure out the address of HvPMROOT(stash) - as always with perl this involves a mad macro chase to find out what HvPMROOT(stash) *really* is.

(from hv.h) #define HvPMROOT(hv) ((XPVHV*) SvANY(hv))->xhv_pmroot

(dbx) print &stash->sv_any->xhv_pmroot
&stash->sv_any->xhv_pmroot = 0x1b36a8 (dbx) call purify_watch(&stash->sv_any->xhv_pmroot) (dbx) run

  Started pureperl ( 1 error\, 0 leaked bytes)   Purify instrumented /home1/homedir/alanbur/perlforce/pureperl/pureperl (pid 134658 at Sun Mar 4 01​:12​:41 2001)   Loading watchpoints from file ./pureperl.watchpoints.   Command-line​: /home1/homedir/alanbur/perlforce/pureperl/t/perl -Dlts \   WPM​: Watch point malloc   WPW​: Watch point write   WPW​: Watch point write   WPW​: Watch point write   This is occurring while in​:   Perl_newPMOP [op.c​:2907]   S_scan_pat [toke.c​:6161]   Perl_yylex [toke.c​:3594]   Perl_yyparse [perly.c​:1432]   S_parse_body [perl.c​:1322]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]   Watchpoint 1   Writing 4 bytes to 0x1b36a8 in the heap.   Value changing from 0 (0x00000000\, "\000\000\000\000")   to 2077568 (0x001fb380\, "\000\037\263\200")   Address 0x1b36a8 is 48 bytes into a malloc'd block at 0x1b3678 of 56 bytes.   This block was allocated from​:   malloc [malloc.c]   Perl_safesysmalloc [util.c​:85]   Perl_sv_upgrade [sv.c​:1114]   Perl_newHV [hv.c​:1006]   S_init_main_stash [perl.c​:2516]   S_parse_body [perl.c​:956]   perl_parse [perl.c​:900]   main [perlmain.c​:50]   _start [crt1.o]   FMR​: Free memory read

So Perl_newPMOP is the thing that points HvPMROOT(stash) to the block in question\, here is the code.

  /* link into pm list */   if (type != OP_TRANS && PL_curstash) {   pmop->op_pmnext = HvPMROOT(PL_curstash); => HvPMROOT(PL_curstash) = pmop;   }

So now we know what is causing the problem. As a quick hack\, let's knock out the 'op_free(other)' in S_new_logop and see what happens​:

  Finished pureperl ( 0 errors\, 835 leaked bytes)   Purify instrumented pureperl (pid 134860 at Sun Mar 4 01​:23​:06 2001)   Command-line​: /home1/homedir/alanbur/perlforce/pureperl/t/perl -Dlts \   Memory leaked​: 835 bytes (100%); potentially leaked​: 0 bytes (0%)   Program exited with status code 0.

Ok\, so we got a couple of leaks\, but at least we have got rid of the UMR\, which proves that we have identified the bug. The fix is left as an excercise for the reader :-)

Alan Burlison.

p5pRT commented 23 years ago

From @AlanBurlison

Vadim Konovalov wrote​:

I found the reason of dump core\, but need more digging to be able to fix this.

Your analysis is spot on.

The problem is that the head of the pmop chain is held in the xhv_pmroot field of the stash to which the regexp belongs\, and as far as I can tell S_new_logop doesn't have access to the relevant stash\, so it can't cleanly remove the OP it is trying to remove. Whatever\, randomly deleting something that is an element of a linked list without unlinking it first is bound to cause problems.

Alan Burlison

p5pRT commented 23 years ago

From @AlanBurlison

Vadim Konovalov wrote​:

But. That error gone (for me\, at least)\, and this patch does not seem to introduce test fails.

No\, but it does introduce memory leaks.

Alan Burlison

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

I found the reason of dump core\, but need more digging to be able to fix this.

Your analysis is spot on.

The problem is that the head of the pmop chain is held in the xhv_pmroot field of the stash to which the regexp belongs\, and as far as I can tell S_new_logop doesn't have access to the relevant stash\, so it can't cleanly remove the OP it is trying to remove. Whatever\, randomly deleting something that is an element of a linked list without unlinking it first is bound to cause problems.

Yes\, I realized that there is no direct access to an appropriate stash with a head of that chain\, and some kind of uncomfortable feeling came to me - what if those patterns do not belong to any of stashes? Does this mean that any anonymous sub with patterns and "reset" in it is a potential crash-maker?

That error gone (for me\, at least)\, and this patch does not seem to introduce test fails.

No\, but it does introduce memory leaks.

... that is why I felt very uncomfortable with my patch\, and did not even threw away raw debug printing\, no code cleaning. I tried patch to check things and to prove the concept\, but not to apply it directly.

BTW\, which leaks were introduced\, more specifically? I think those lines of code should not free any memory. And they did not requested any memory either.

Best wishes\, \<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From @AlanBurlison

Vadim Konovalov wrote​:

BTW\, which leaks were introduced\, more specifically? I think those lines of code should not free any memory. And they did not requested any memory either.

You seem to have removed this line​:

  ReREFCNT_dec(cPMOPo->op_pmregexp);

Alan Burlison

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

BTW\, which leaks were introduced\, more specifically? I think those lines of code should not free any memory. And they did not requested any memory either.

You seem to have removed this line​:

    ReREFCNT\_dec\(cPMOPo\->op\_pmregexp\);

My patch did NOT contained any removal lines! All my lines in patch have "+" (if any) at first position! And I do see that line in my patch as NOT changed.

BTW what do you think\, what should be done if start of that chain did not found (when line with "very bad!" printing is executed) ? Do you have any ideas?

\<!ENTITY Vadim REALLIFE "St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From @AlanBurlison

Vadim Konovalov wrote​:

My patch did NOT contained any removal lines! All my lines in patch have "+" (if any) at first position! And I do see that line in my patch as NOT changed.

Sorry\, my mistake\, you are right - I should read more carefully before I reply ;-)

BTW what do you think\, what should be done if start of that chain did not found (when line with "very bad!" printing is executed) ? Do you have any ideas?

I'm no expert\, but assuming that you can guess the head of the chain the OP might be in\, as your patch does\, seems very suspect to me. I'm not sure that S_op_clear is the right place to try to fix this - it seems to me that the suspect optimisation code in S_new_logop is where it should be fixed\, if at all possible.

Alan Burlison

p5pRT commented 23 years ago

From @gsar

On Sun\, 04 Mar 2001 12​:14​:48 GMT\, Alan Burlison wrote​:

Vadim Konovalov wrote​:

BTW what do you think\, what should be done if start of that chain did not found (when line with "very bad!" printing is executed) ? Do you have any ideas?

I'm no expert\, but assuming that you can guess the head of the chain the OP might be in\, as your patch does\, seems very suspect to me. I'm not sure that S_op_clear is the right place to try to fix this - it seems to me that the suspect optimisation code in S_new_logop is where it should be fixed\, if at all possible.

FWIW\, it seems to me that PMOPs either need some kind of refcount mechanism\, or they need to remember the package stash in which they were defined so that they can take themselves out of the linked list when they go away. I'd be inclined to do the latter\, because keeping PMOPs around when their outer context has been optimized away seems suspect to me\, and that's what the first approach will get you.

If somebody wants to tackle this further\, note that mutable data is not stored in the op tree under ithreads. You'll need to do something like what's done in the COP structure in cop.h.

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test case in question is a widely used idiom.

Sarathy gsar@​ActiveState.com

p5pRT commented 23 years ago

From @jhi

FWIW\, it seems to me that PMOPs either need some kind of refcount mechanism\, or they need to remember the package stash in which they were defined so that they can take themselves out of the linked list when they go away. I'd be inclined to do the latter\, because keeping PMOPs around when their outer context has been optimized away seems suspect to me\, and that's what the first approach will get you.

If somebody wants to tackle this further\, note that mutable data is not stored in the op tree under ithreads. You'll need to do something like what's done in the COP structure in cop.h.

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test

Deprecated as in will give a -w warning or deprecated as in gone?

case in question is a widely used idiom.

The test case is just a very cut down version of pat.t. G

Sarathy gsar@​ActiveState.com

p5pRT commented 23 years ago

From @gsar

On Sun\, 04 Mar 2001 22​:39​:12 CST\, Jarkko Hietaniemi wrote​:

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test

Deprecated as in will give a -w warning or deprecated as in gone?

Deprecated as in documented as being unsupported\, maybe.

case in question is a widely used idiom.

The test case is just a very cut down version of pat.t. G

That's easy to fix. ;-)

Sarathy gsar@​ActiveState.com

p5pRT commented 23 years ago

From @jhi

On Sun\, Mar 04\, 2001 at 08​:50​:02PM -0800\, Gurusamy Sarathy wrote​:

On Sun\, 04 Mar 2001 22​:39​:12 CST\, Jarkko Hietaniemi wrote​:

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test

Deprecated as in will give a -w warning or deprecated as in gone?

Deprecated as in documented as being unsupported\, maybe.

case in question is a widely used idiom.

The test case is just a very cut down version of pat.t.

That's easy to fix. ;-)

:-)

The bug is rather inconvenient\, though. It came as up as I was trying to add more test cases to pat.t... somewhat disconcerting\, add test number 240 or so and test 25 or so starts to core dump...

Sarathy gsar@​ActiveState.com

p5pRT commented 23 years ago

From @gsar

On Sun\, 04 Mar 2001 22​:53​:49 CST\, Jarkko Hietaniemi wrote​:

The bug is rather inconvenient\, though. It came as up as I was trying to add more test cases to pat.t... somewhat disconcerting\, add test number 240 or so and test 25 or so starts to core dump...

Try this (untested!) patch.

Sarathy gsar@​ActiveState.com

Inline Patch ```diff -----------------------------------8<----------------------------------- Index: perl/op.c --- perl/op.c.~1~ Sun Mar 4 22:30:19 2001 +++ perl/op.c Sun Mar 4 22:30:19 2001 @@ -842,6 +842,29 @@ case OP_MATCH: case OP_QR: clear_pmop: + { + HV *pmstash = PmopSTASH(cPMOPo); + if (pmstash) { + PMOP *pmop = HvPMROOT(pmstash); + PMOP *lastpmop = NULL; + while (pmop) { + if (cPMOPo == pmop) { + if (lastpmop) + lastpmop->op_pmnext = pmop->op_pmnext; + else + HvPMROOT(pmstash) = pmop->op_pmnext; + break; + } + lastpmop = pmop; + pmop = pmop->op_pmnext; + } +#ifdef USE_ITHREADS + Safefree(PmopSTASHPV(cPMOPo)); +#else + /* NOTE: PMOP.op_pmstash is not refcounted */ +#endif + } + } cPMOPo->op_pmreplroot = Nullop; ReREFCNT_dec(cPMOPo->op_pmregexp); cPMOPo->op_pmregexp = (REGEXP*)NULL; @@ -2914,6 +2937,7 @@ if (type != OP_TRANS && PL_curstash) { pmop->op_pmnext = HvPMROOT(PL_curstash); HvPMROOT(PL_curstash) = pmop; + PmopSTASH_set(pmop,PL_curstash); } return (OP*)pmop; Index: perl/op.h --- perl/op.h.~1~ Sun Mar 4 22:30:19 2001 +++ perl/op.h Sun Mar 4 22:30:19 2001 @@ -242,6 +242,11 @@ U16 op_pmflags; U16 op_pmpermflags; U8 op_pmdynflags; +#ifdef USE_ITHREADS + char * op_pmstashpv; +#else + HV * op_pmstash; +#endif }; #define PMdf_USED 0x01 /* pm has been used once already */ @@ -267,6 +272,20 @@ /* mask of bits stored in regexp->reganch */ #define PMf_COMPILETIME (PMf_MULTILINE|PMf_SINGLELINE|PMf_LOCALE|PMf_FOLD|PMf_EXTENDED) + +#ifdef USE_ITHREADS +# define PmopSTASHPV(o) ((o)->op_pmstashpv) +# define PmopSTASHPV_set(o,pv) ((o)->op_pmstashpv = ((pv) ? savepv(pv) : Nullch)) +# define PmopSTASH(o) (PmopSTASHPV(o) \ + ? gv_stashpv(PmopSTASHPV(o),GV_ADD) : Nullhv) +# define PmopSTASH_set(o,hv) PmopSTASHPV_set(o, (hv) ? HvNAME(hv) : Nullch) +#else +# define PmopSTASH(o) ((o)->op_pmstash) +# define PmopSTASH_set(o,hv) ((o)->op_pmstash = (hv)) +# define PmopSTASHPV(o) (PmopSTASH(o) ? HvNAME(PmopSTASH(o)) : Nullch) + /* op_pmstash is not refcounted */ +# define PmopSTASHPV_set(o,pv) PmopSTASH_set((o), gv_stashpv(pv,GV_ADD)) +#endif struct svop { BASEOP End of Patch. ```
p5pRT commented 23 years ago

From @tux

On Sun\, 04 Mar 2001 19​:26​:51 -0800 Gurusamy Sarathy \gsar@&#8203;ActiveState\.com wrote​:

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test case in question is a widely used idiom.

My talk to YAPC​::Europe-2001 will also address a usage of reset that I wanted to get rid of\, but by then was unable to\, because of a bug in format.

Since we *know* we havn't solved /all/ bugs in perl (yet)\, be *very* careful to remove features!

My vote is to document the ill-behaviour and maybe surprising and/or unwanted sideeffects of reset\, and leave it in as is (for now). Document that it might be dropped in a (near) future release.

Sarathy gsar@​ActiveState.com

p5pRT commented 23 years ago

From @AlanBurlison

"H.Merijn Brand" wrote​:

My vote is to document the ill-behaviour and maybe surprising and/or unwanted sideeffects of reset\, and leave it in as is (for now). Document that it might be dropped in a (near) future release.

I thought there was an unwritten rule that anything that caused coredumps should be fixed. This does.

Alan Burlison

p5pRT commented 23 years ago

From @tux

On Mon\, 05 Mar 2001 10​:22​:41 +0000\, Alan Burlison \Alan\.Burlison@&#8203;uk\.sun\.com wrote​:

"H.Merijn Brand" wrote​:

My vote is to document the ill-behaviour and maybe surprising and/or unwanted sideeffects of reset\, and leave it in as is (for now). Document that it might be dropped in a (near) future release.

I thought there was an unwritten rule that anything that caused coredumps should be fixed. This does.

My vote was *against* removing the reset function\, not against the cure against a core dump.

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

My vote is to document the ill-behaviour and maybe surprising and/or unwanted sideeffects of reset\, and leave it in as is (for now). Document that it might be dropped in a (near) future release.

I thought there was an unwritten rule that anything that caused coredumps should be fixed. This does.

IMHO this deprecated function "reset" takes too much overhead while it is deprecated. Because of it PMOPs are linked chain\, so it takes 4 extra bytes perl PMOP. Latest fix supposed by Gurusamy Sarathy takes even more overhead - it should remember stash when it was created\, 4 bytes more.

8 extra bytes for each PMOP to support rarely used deprecated feature!

OTOH there hardly exists an acceptable way to get rid of that "reset" anyway...

Best wishes\, \<!ENTITY Vadim REALLIFE "Vadim V.Konovalov\, St.Petersburg\, Russia"> &Vadim;

p5pRT commented 23 years ago

From @AlanBurlison

"H.Merijn Brand" wrote​:

I thought there was an unwritten rule that anything that caused coredumps should be fixed. This does.

My vote was *against* removing the reset function\, not against the cure against a core dump.

That *is* the proposed cure!

Sarathay is right - the correct fix for this is to refcount the PMOPs\, or remember the stash from which they came\, but either of those sounds like a major piece of work.

Alan Burlison

p5pRT commented 23 years ago

From @tux

On Mon\, 05 Mar 2001 11​:01​:39 +0000\, Alan Burlison \Alan\.Burlison@&#8203;uk\.sun\.com wrote​:

"H.Merijn Brand" wrote​:

I thought there was an unwritten rule that anything that caused coredumps should be fixed. This does.

My vote was *against* removing the reset function\, not against the cure against a core dump.

That *is* the proposed cure!

Sarathay is right - the correct fix for this is to refcount the PMOPs\, or remember the stash from which they came\, but either of those sounds like a major piece of work.

\ Since reset only works on global variables\, isn't it possible to get reset out of the core and implement it in pure perl\, autoloaded on demand? \

Would that ease the problem?

p5pRT commented 23 years ago

From @jhi

On Sun\, Mar 04\, 2001 at 10​:33​:15PM -0800\, Gurusamy Sarathy wrote​:

On Sun\, 04 Mar 2001 22​:53​:49 CST\, Jarkko Hietaniemi wrote​:

The bug is rather inconvenient\, though. It came as up as I was trying to add more test cases to pat.t... somewhat disconcerting\, add test number 240 or so and test 25 or so starts to core dump...

Try this (untested!) patch.

Seems to work for me with bleadperl in all the platforms that cored.

Sarathy gsar@​ActiveState.com -----------------------------------8\<----------------------------------- Index​: perl/op.c --- perl/op.c.~1~ Sun Mar 4 22​:30​:19 2001 +++ perl/op.c Sun Mar 4 22​:30​:19 2001 @​@​ -842\,6 +842\,29 @​@​ case OP_MATCH​: case OP_QR​: clear_pmop​: + { + HV *pmstash = PmopSTASH(cPMOPo); + if (pmstash) { + PMOP *pmop = HvPMROOT(pmstash); + PMOP *lastpmop = NULL; + while (pmop) { + if (cPMOPo == pmop) { + if (lastpmop) + lastpmop->op_pmnext = pmop->op_pmnext; + else + HvPMROOT(pmstash) = pmop->op_pmnext; + break; + } + lastpmop = pmop; + pmop = pmop->op_pmnext; + } +#ifdef USE_ITHREADS + Safefree(PmopSTASHPV(cPMOPo)); +#else + /* NOTE​: PMOP.op_pmstash is not refcounted */ +#endif + } + } cPMOPo->op_pmreplroot = Nullop; ReREFCNT_dec(cPMOPo->op_pmregexp); cPMOPo->op_pmregexp = (REGEXP*)NULL; @​@​ -2914\,6 +2937\,7 @​@​ if (type != OP_TRANS && PL_curstash) { pmop->op_pmnext = HvPMROOT(PL_curstash); HvPMROOT(PL_curstash) = pmop; + PmopSTASH_set(pmop\,PL_curstash); }

 return \(OP\*\)pmop;

Index​: perl/op.h --- perl/op.h.~1~ Sun Mar 4 22​:30​:19 2001 +++ perl/op.h Sun Mar 4 22​:30​:19 2001 @​@​ -242\,6 +242\,11 @​@​ U16 op_pmflags; U16 op_pmpermflags; U8 op_pmdynflags; +#ifdef USE_ITHREADS + char * op_pmstashpv; +#else + HV * op_pmstash; +#endif };

#define PMdf_USED 0x01 /* pm has been used once already */ @​@​ -267\,6 +272\,20 @​@​

/* mask of bits stored in regexp->reganch */ #define PMf_COMPILETIME (PMf_MULTILINE|PMf_SINGLELINE|PMf_LOCALE|PMf_FOLD|PMf_EXTENDED) + +#ifdef USE_ITHREADS +# define PmopSTASHPV(o) ((o)->op_pmstashpv) +# define PmopSTASHPV_set(o\,pv) ((o)->op_pmstashpv = ((pv) ? savepv(pv) : Nullch)) +# define PmopSTASH(o) (PmopSTASHPV(o) \ + ? gv_stashpv(PmopSTASHPV(o)\,GV_ADD) : Nullhv) +# define PmopSTASH_set(o\,hv) PmopSTASHPV_set(o\, (hv) ? HvNAME(hv) : Nullch) +#else +# define PmopSTASH(o) ((o)->op_pmstash) +# define PmopSTASH_set(o\,hv) ((o)->op_pmstash = (hv)) +# define PmopSTASHPV(o) (PmopSTASH(o) ? HvNAME(PmopSTASH(o)) : Nullch) + /* op_pmstash is not refcounted */ +# define PmopSTASHPV_set(o\,pv) PmopSTASH_set((o)\, gv_stashpv(pv\,GV_ADD)) +#endif

struct svop { BASEOP End of Patch.

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

On 4 Mar 2001\, at 20​:50\, Gurusamy Sarathy wrote​:

On Sun\, 04 Mar 2001 22​:39​:12 CST\, Jarkko Hietaniemi wrote​:

That said\, C\<reset;> is somewhat of an abomination. Why can't we deprecate it in 5.8 and just move along? It is not like the test

Deprecated as in will give a -w warning or deprecated as in gone?

Deprecated as in documented as being unsupported\, maybe.

It should probably generate a warning under -w as well (possibly phased out later\, like the "@​ in string" warning) IMO.

Cheers\, Philip

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

The second half of this patch (to op.h) didn't get into the diffs/9033.gz

opmini.o​: In function `S_op_clear'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:847​: undefined reference to `PmopSTASH' opmini.o​: In function `Perl_ck_split'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' opmini.o​: In function `Perl_newPMOP'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' collect2​: ld returned 1 exit status

The op.h patch applied to bleadperl (through patch 9041) but I see make test fails (op/qu.t has been failing for a few days\, but I didn't notice lib/1_compile and lib/dosglob failing before)

op/qu.................Backslash found where operator expected at op/qu.t line 16\, near "abc\" Backslash found where operator expected at op/qu.t line 19\, near "abc\" syntax error at op/qu.t line 16\, near "abc\" syntax error at op/qu.t line 19\, near "abc\" Execution of op/qu.t aborted due to compilation errors. Scalars leaked​: 1 FAILED at test 0

lib/1_compile.........FAILED at test 13 lib/dosglob...........FAILED at test 10

On Sun\, 4 Mar 2001\, Gurusamy Sarathy wrote​:

From​: Gurusamy Sarathy \gsar@&#8203;ActiveState\.com To​: Jarkko Hietaniemi \jhi@&#8203;iki\.fi Cc​: Gurusamy Sarathy \gsar@&#8203;ActiveState\.com\,   Alan Burlison \Alan\.Burlison@&#8203;uk\.sun\.com\,   Vadim Konovalov \watman@&#8203;inbox\.ru\, perl5-porters@​perl.org Date​: Sun\, 04 Mar 2001 22​:33​:15 -0800 Subject​: Re​: [ID 20010301.005] corrupt memory (since @​8531\, in 5.6.1-T2)

On Sun\, 04 Mar 2001 22​:53​:49 CST\, Jarkko Hietaniemi wrote​:

The bug is rather inconvenient\, though. It came as up as I was trying to add more test cases to pat.t... somewhat disconcerting\, add test number 240 or so and test 25 or so starts to core dump...

Try this (untested!) patch.

Sarathy gsar@​ActiveState.com

Inline Patch ```diff -----------------------------------8<----------------------------------- Index: perl/op.h --- perl/op.h.~1~ Sun Mar 4 22:30:19 2001 +++ perl/op.h Sun Mar 4 22:30:19 2001 @@ -242,6 +242,11 @@ U16 op_pmflags; U16 op_pmpermflags; U8 op_pmdynflags; +#ifdef USE_ITHREADS + char * op_pmstashpv; +#else + HV * op_pmstash; +#endif }; #define PMdf_USED 0x01 /* pm has been used once already */ @@ -267,6 +272,20 @@ /* mask of bits stored in regexp->reganch */ #define PMf_COMPILETIME (PMf_MULTILINE|PMf_SINGLELINE|PMf_LOCALE|PMf_FOLD|PMf_EXTENDED) + +#ifdef USE_ITHREADS +# define PmopSTASHPV(o) ((o)->op_pmstashpv) +# define PmopSTASHPV_set(o,pv) ((o)->op_pmstashpv = ((pv) ? savepv(pv) : Nullch)) +# define PmopSTASH(o) (PmopSTASHPV(o) \ + ? gv_stashpv(PmopSTASHPV(o),GV_ADD) : Nullhv) +# define PmopSTASH_set(o,hv) PmopSTASHPV_set(o, (hv) ? HvNAME(hv) : Nullch) +#else +# define PmopSTASH(o) ((o)->op_pmstash) +# define PmopSTASH_set(o,hv) ((o)->op_pmstash = (hv)) +# define PmopSTASHPV(o) (PmopSTASH(o) ? HvNAME(PmopSTASH(o)) : Nullch) + /* op_pmstash is not refcounted */ +# define PmopSTASHPV_set(o,pv) PmopSTASH_set((o), gv_stashpv(pv,GV_ADD)) +#endif struct svop { BASEOP End of Patch. ```
p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

I beleive those tests are failed due to some different reason than applying patch which deals *only* with a crash only for "reset" function. (I still vote for elimination of "reset" function)

Looking at your compiler errors makes me 95% sure that you're still including elder "op.h"\, probably because you did not cleaned perl's include directory.

Just a guess.

Best wishes\, \<!ENTITY Vadim REALLIFE "Vadim V.Konovalov\, St.Petersburg\, Russia"> &Vadim;

-----Original Message----- From​: David Dyck [mailto​:dcd@​tc.fluke.com] Sent​: Monday\, March 05\, 2001 9​:11 PM To​: Jarkko Hietaniemi Cc​: Gurusamy Sarathy; Perl Porters Subject​: Re​: [ID 20010301.005] corrupt memory (since @​8531\, in 5.6.1-T2)

The second half of this patch (to op.h) didn't get into the diffs/9033.gz

opmini.o​: In function `S_op_clear'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:847​: undefined reference to `PmopSTASH' opmini.o​: In function `Perl_ck_split'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' opmini.o​: In function `Perl_newPMOP'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' collect2​: ld returned 1 exit status

The op.h patch applied to bleadperl (through patch 9041) but I see make test fails (op/qu.t has been failing for a few days\, but I didn't notice lib/1_compile and lib/dosglob failing before)

op/qu.................Backslash found where operator expected at op/qu.t line 16\, near "abc\" Backslash found where operator expected at op/qu.t line 19\, near "abc\" syntax error at op/qu.t line 16\, near "abc\" syntax error at op/qu.t line 19\, near "abc\" Execution of op/qu.t aborted due to compilation errors. Scalars leaked​: 1 FAILED at test 0

lib/1_compile.........FAILED at test 13 lib/dosglob...........FAILED at test 10

On Sun\, 4 Mar 2001\, Gurusamy Sarathy wrote​:

From​: Gurusamy Sarathy \gsar@&#8203;ActiveState\.com To​: Jarkko Hietaniemi \jhi@&#8203;iki\.fi Cc​: Gurusamy Sarathy \gsar@&#8203;ActiveState\.com\, Alan Burlison \Alan\.Burlison@&#8203;uk\.sun\.com\, Vadim Konovalov \watman@&#8203;inbox\.ru\, perl5-porters@​perl.org Date​: Sun\, 04 Mar 2001 22​:33​:15 -0800 Subject​: Re​: [ID 20010301.005] corrupt memory (since @​8531\, in 5.6.1-T2)

On Sun\, 04 Mar 2001 22​:53​:49 CST\, Jarkko Hietaniemi wrote​:

The bug is rather inconvenient\, though. It came as up as I was trying to add more test cases to pat.t... somewhat disconcerting\, add test number 240 or so and test 25 or so starts to core dump...

Try this (untested!) patch.

Sarathy gsar@​ActiveState.com -----------------------------------8\<------------------------- ---------- Index​: perl/op.h --- perl/op.h.~1~ Sun Mar 4 22​:30​:19 2001 +++ perl/op.h Sun Mar 4 22​:30​:19 2001 @​@​ -242\,6 +242\,11 @​@​ U16 op_pmflags; U16 op_pmpermflags; U8 op_pmdynflags; +#ifdef USE_ITHREADS + char * op_pmstashpv; +#else + HV * op_pmstash; +#endif };

#define PMdf_USED 0x01 /* pm has been used once already */ @​@​ -267\,6 +272\,20 @​@​

/* mask of bits stored in regexp->reganch */ #define PMf_COMPILETIME
(PMf_MULTILINE|PMf_SINGLELINE|PMf_LOCALE|PMf_FOLD|PMf_EXTENDED) + +#ifdef USE_ITHREADS +# define PmopSTASHPV(o) ((o)->op_pmstashpv) +# define PmopSTASHPV_set(o\,pv) ((o)->op_pmstashpv = ((pv) ? savepv(pv) : Nullch)) +# define PmopSTASH(o) (PmopSTASHPV(o) \ + ? gv_stashpv(PmopSTASHPV(o)\,GV_ADD) : Nullhv) +# define PmopSTASH_set(o\,hv) PmopSTASHPV_set(o\, (hv) ? HvNAME(hv) : Nullch) +#else +# define PmopSTASH(o) ((o)->op_pmstash) +# define PmopSTASH_set(o\,hv) ((o)->op_pmstash = (hv)) +# define PmopSTASHPV(o) (PmopSTASH(o) ? HvNAME(PmopSTASH(o)) : Nullch) + /* op_pmstash is not refcounted */ +# define PmopSTASHPV_set(o\,pv) PmopSTASH_set((o)\, gv_stashpv(pv\,GV_ADD)) +#endif

struct svop { BASEOP End of Patch.

p5pRT commented 23 years ago

From @jhi

On Mon\, Mar 05\, 2001 at 10​:11​:14AM -0800\, David Dyck wrote​:

The second half of this patch (to op.h) didn't get into the diffs/9033.gz

opmini.o​: In function `S_op_clear'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:847​: undefined reference to `PmopSTASH' opmini.o​: In function `Perl_ck_split'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' opmini.o​: In function `Perl_newPMOP'​: /hdb2/home/dcd/CPAN/perl-current/opmini.c​:2960​: undefined reference to `PmopSTASH_set' collect2​: ld returned 1 exit status

Oops. Now checked in as #9044.

The op.h patch applied to bleadperl (through patch 9041) but I see make test fails (op/qu.t has been failing for a few days\,

rm t/op/qu.t