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

Invalid free in t/op/write.t #11369

Closed
p5pRT opened this issue May 18, 2011 · 19 comments
Closed

Invalid free in t/op/write.t #11369

p5pRT opened this issue May 18, 2011 · 19 comments

Comments

@p5pRT
Copy link

p5pRT commented May 18, 2011

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

Searchable as RT91032$

@p5pRT
Copy link
Author

p5pRT commented May 18, 2011

From @khwilliamson

This is a bug report for perl from khw@​karl.(none),
generated with the help of perlbug 1.39 running under perl 5.15.0.


This is reproducible.

not ok 326 # TODO [ID 20020227.005] format bug with undefined _TOP
# Failed at op/write.t line 603
# got "-257"
# expected "9"
ok 327
ok 328 - glob assign
ok 329 - write to duplicated format
ok 330 - \#64562 - Segmentation fault with redefined formats and warnings
*** glibc detected *** /home/khw/perl/blead/perl​: free()​: invalid
pointer​: 0x08c04d70 ***
======= Backtrace​: =========
/lib/libc.so.6(+0x6c501)[0xb7555501]
/lib/libc.so.6(+0x6dd70)[0xb7556d70]
/lib/libc.so.6(cfree+0x6d)[0xb7559e5d]
/home/khw/perl/blead/perl(Perl_safesysfree+0x163)[0x816867b]
/home/khw/perl/blead/perl(perl_free+0xcb)[0x808cd29]
/home/khw/perl/blead/perl(main+0x16f)[0x805d253]
/lib/libc.so.6(__libc_start_main+0xe7)[0xb74ffce7]
/home/khw/perl/blead/perl[0x805d051]
======= Memory map​: ========
08048000-0847f000 r-xp 00000000 08​:09 4209945 /home/khw/perl/blead/perl
0847f000-08480000 r--p 00436000 08​:09 4209945 /home/khw/perl/blead/perl
08480000-08482000 rw-p 00437000 08​:09 4209945 /home/khw/perl/blead/perl
08be4000-08c47000 rw-p 00000000 00​:00 0 [heap]
b7100000-b7121000 rw-p 00000000 00​:00 0
b7121000-b7200000 ---p 00000000 00​:00 0
b72bc000-b72d6000 r-xp 00000000 08​:05 407279 /lib/libgcc_s.so.1
b72d6000-b72d7000 r--p 00019000 08​:05 407279 /lib/libgcc_s.so.1
b72d7000-b72d8000 rw-p 0001a000 08​:05 407279 /lib/libgcc_s.so.1
b72e7000-b74e7000 r--p 00000000 08​:07 131867
/usr/lib/locale/locale-archive
b74e7000-b74e9000 rw-p 00000000 00​:00 0
b74e9000-b7640000 r-xp 00000000 08​:05 407505 /lib/libc-2.12.1.so
b7640000-b7642000 r--p 00157000 08​:05 407505 /lib/libc-2.12.1.so
b7642000-b7643000 rw-p 00159000 08​:05 407505 /lib/libc-2.12.1.so
b7643000-b7646000 rw-p 00000000 00​:00 0
b7646000-b765b000 r-xp 00000000 08​:05 407519 /lib/libpthread-2.12.1.so
b765b000-b765c000 ---p 00015000 08​:05 407519 /lib/libpthread-2.12.1.so
b765c000-b765d000 r--p 00015000 08​:05 407519 /lib/libpthread-2.12.1.so
b765d000-b765e000 rw-p 00016000 08​:05 407519 /lib/libpthread-2.12.1.so
b765e000-b7660000 rw-p 00000000 00​:00 0
b7660000-b7662000 r-xp 00000000 08​:05 407524 /lib/libutil-2.12.1.so
b7662000-b7663000 r--p 00001000 08​:05 407524 /lib/libutil-2.12.1.so
b7663000-b7664000 rw-p 00002000 08​:05 407524 /lib/libutil-2.12.1.so
b7664000-b766d000 r-xp 00000000 08​:05 407507 /lib/libcrypt-2.12.1.so
b766d000-b766e000 r--p 00008000 08​:05 407507 /lib/libcrypt-2.12.1.so
b766e000-b766f000 rw-p 00009000 08​:05 407507 /lib/libcrypt-2.12.1.so
b766f000-b7696000 rw-p 00000000 00​:00 0
b7696000-b76ba000 r-xp 00000000 08​:05 407509 /lib/libm-2.12.1.so
b76ba000-b76bb000 r--p 00023000 08​:05 407509 /lib/libm-2.12.1.so
b76bb000-b76bc000 rw-p 00024000 08​:05 407509 /lib/libm-2.12.1.so
b76bc000-b76bd000 rw-p 00000000 00​:00 0
b76bd000-b76bf000 r-xp 00000000 08​:05 407508 /lib/libdl-2.12.1.so
b76bf000-b76c0000 r--p 00001000 08​:05 407508 /lib/libdl-2.12.1.so
b76c0000-b76c1000 rw-p 00002000 08​:05 407508 /lib/libdl-2.12.1.so
b76c1000-b76d4000 r-xp 00000000 08​:05 407511 /lib/libnsl-2.12.1.so
b76d4000-b76d5000 r--p 00012000 08​:05 407511 /lib/libnsl-2.12.1.so
b76d5000-b76d6000 rw-p 00013000 08​:05 407511 /lib/libnsl-2.12.1.so
b76d6000-b76d8000 rw-p 00000000 00​:00 0
b76e6000-b76e7000 r--p 002a1000 08​:07 131867
/usr/lib/locale/locale-archive
b76e7000-b76e9000 rw-p 00000000 00​:00 0
b76e9000-b76ea000 r-xp 00000000 00​:00 0 [vdso]
b76ea000-b7706000 r-xp 00000000 08​:05 407502 /lib/ld-2.12.1.so
b7706000-b7707000 r--p 0001b000 08​:05 407502 /lib/ld-2.12.1.so
b7707000-b7708000 rw-p 0001c000 08​:05 407502 /lib/ld-2.12.1.so
bff57000-bff78000 rw-p 00000000 00​:00 0 [stack]
not ok 331 - \#79532 - formline coerces its arguments
# Failed at ./test.pl line 828
# got ">ARRAY<\ncrunch_eth\nAborted"
# expected ">ARRAY<\ncrunch_eth"
# PROG​:
# use strict;
# use warnings;
# my $zamm = ['crunch_eth'];
# formline $zamm;
# printf ">%s<\n", ref $zamm;
# print "$zamm->[0]\n";
# STATUS​: 34304
ok 332 - open
ok 333
ok 334
ok 335
ok 336



Flags​:
  category=core
  severity=high


Site configuration information for perl 5.15.0​:

Configured by khw at Wed May 18 13​:24​:20 MDT 2011.

Summary of my perl5 (revision 5 version 15 subversion 0) configuration​:
  Commit id​: 10914c7
  Platform​:
  osname=linux, osvers=2.6.35-28-generic-pae,
archname=i686-linux-thread-multi-64int-ld
  uname='linux karl 2.6.35-28-generic-pae #50-ubuntu smp fri mar 18
20​:43​:15 utc 2011 i686 gnulinux '
  config_args='-des -Dprefix=/home/khw/blead -Dusedevel
-D'optimize=-ggdb3' -A'optimize=-ggdb3' -A'optimize=-O0' -Dman1dir=none
-Dman3dir=none -DDEBUGGING -Dusemorebits -Dusethreads'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=undef, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING
-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O0 -ggdb3',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING
-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.5', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
  ivtype='long long', ivsize=8, nvtype='long double', nvsize=12,
Off_t='off_t', lseeksize=8
  alignbytes=4, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/../lib /usr/lib/../lib /lib /usr/lib
/usr/lib/i686-linux-gnu
  libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=/lib/libc-2.12.1.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.12.1'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -ggdb3 -ggdb3 -O0
-L/usr/local/lib -fstack-protector'

Locally applied patches​:


@​INC for perl 5.15.0​:

/home/khw/blead/lib/perl5/site_perl/5.15.0/i686-linux-thread-multi-64int-ld
  /home/khw/blead/lib/perl5/site_perl/5.15.0
  /home/khw/blead/lib/perl5/5.15.0/i686-linux-thread-multi-64int-ld
  /home/khw/blead/lib/perl5/5.15.0
  /home/khw/blead/lib/perl5/site_perl
  .


Environment for perl 5.15.0​:
  HOME=/home/khw
  LANG=en_US.UTF-8
  LANGUAGE=en_US​:en
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)

PATH=/home/khw/bin​:/home/khw/print/bin​:/bin​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/usr/games​:/home/khw/cxoffice/bin
  PERL5OPT=-w
  PERL_BADLANG (unset)
  SHELL=/bin/ksh

@p5pRT
Copy link
Author

p5pRT commented May 21, 2011

From @khwilliamson

Git bisect results​:

37ffbfc is the first bad commit
commit 37ffbfc
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Mon May 2 12​:37​:30 2011 +0100

  In S_doparseform(), don't force the pattern to a string. This
resolves #79532
 
  Previously S_doparseform() was using SvPV_force(), because the
pattern had to
  be forced to a string, because the compiled format was stored in the
string's
  buffer. Now that the compiled format is stored in the magic struct,
this isn't
  necessary.
 
  Additionally, removing the call to SvPV_force() removes the need to
hack with
  the SvREADONLY() flag in pp_formline.

@p5pRT
Copy link
Author

p5pRT commented May 21, 2011

From [Unknown Contact. See original ticket]

Git bisect results​:

37ffbfc is the first bad commit
commit 37ffbfc
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Mon May 2 12​:37​:30 2011 +0100

  In S_doparseform(), don't force the pattern to a string. This
resolves #79532
 
  Previously S_doparseform() was using SvPV_force(), because the
pattern had to
  be forced to a string, because the compiled format was stored in the
string's
  buffer. Now that the compiled format is stored in the magic struct,
this isn't
  necessary.
 
  Additionally, removing the call to SvPV_force() removes the need to
hack with
  the SvREADONLY() flag in pp_formline.

@p5pRT
Copy link
Author

p5pRT commented May 21, 2011

@khwilliamson - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

From rmbarker.cpan@btinternet.com

On Sat, 2011-05-21 at 08​:56 -0700, Karl Williamson via RT wrote​:

Git bisect results​:

37ffbfc is the first bad commit
commit 37ffbfc
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Mon May 2 12​:37​:30 2011 +0100

But this is just when the test in t/op/write.t was introduced.

The underlying failure is present in perl5.12.0

env PERL_DESTRUCT_LEVEL=1 perl5.12.0 -e 'my $foo=[qw(foo)]; formline
$foo'
*** glibc detected *** perl5.12.0​: free()​: invalid next size (fast)​:
0x08ab3f90 ***
...
Abort

@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

From rmbarker.cpan@btinternet.com

Created by rmbarker.cpan@btinternet.com

This is a cutdown of a failure (abort) from t/op/write.t test #331
(#79532 - formline coerces its arguments)

I'm sorry I don't understand this stuff.

env PERL_DESTRUCT_LEVEL=1 perl -e 'formline []'
*** glibc detected *** perl​: free()​: invalid next size (fast)​:
0x092cb9c8 ***
======= Backtrace​: =========
/lib/libc.so.6(+0x6c501)[0x17c501]
/lib/libc.so.6(+0x6dd70)[0x17dd70]
/lib/libc.so.6(cfree+0x6d)[0x180e5d]
perl(Perl_sv_clear+0x3ec)[0x8151f8c]
======= Memory map​: ========
00110000-00267000 r-xp 00000000 08​:01 2310 /lib/libc-2.12.1.so
00267000-00269000 r--p 00157000 08​:01 2310 /lib/libc-2.12.1.so
00269000-0026a000 rw-p 00159000 08​:01 2310 /lib/libc-2.12.1.so
0026a000-0026d000 rw-p 00000000 00​:00 0
00378000-00381000 r-xp 00000000 08​:01 2324 /lib/libcrypt-2.12.1.so
00381000-00382000 r--p 00008000 08​:01 2324 /lib/libcrypt-2.12.1.so
00382000-00383000 rw-p 00009000 08​:01 2324 /lib/libcrypt-2.12.1.so
00383000-003aa000 rw-p 00000000 00​:00 0
003ea000-003fd000 r-xp 00000000 08​:01 2372 /lib/libnsl-2.12.1.so
003fd000-003fe000 r--p 00012000 08​:01 2372 /lib/libnsl-2.12.1.so
003fe000-003ff000 rw-p 00013000 08​:01 2372 /lib/libnsl-2.12.1.so
003ff000-00401000 rw-p 00000000 00​:00 0
007a0000-007a2000 r-xp 00000000 08​:01 10952 /lib/libutil-2.12.1.so
007a2000-007a3000 r--p 00001000 08​:01 10952 /lib/libutil-2.12.1.so
007a3000-007a4000 rw-p 00002000 08​:01 10952 /lib/libutil-2.12.1.so
007d7000-007fb000 r-xp 00000000 08​:01 2348 /lib/libm-2.12.1.so
007fb000-007fc000 r--p 00023000 08​:01 2348 /lib/libm-2.12.1.so
007fc000-007fd000 rw-p 00024000 08​:01 2348 /lib/libm-2.12.1.so
008f3000-0090f000 r-xp 00000000 08​:01 2257 /lib/ld-2.12.1.so
0090f000-00910000 r--p 0001b000 08​:01 2257 /lib/ld-2.12.1.so
00910000-00911000 rw-p 0001c000 08​:01 2257 /lib/ld-2.12.1.so
00b2c000-00b48000 r-xp 00000000 08​:01
132905 /usr/local/lib/libgcc_s.so.1
00b48000-00b49000 rw-p 0001b000 08​:01
132905 /usr/local/lib/libgcc_s.so.1
00c44000-00c46000 r-xp 00000000 08​:01 2333 /lib/libdl-2.12.1.so
00c46000-00c47000 r--p 00001000 08​:01 2333 /lib/libdl-2.12.1.so
00c47000-00c48000 rw-p 00002000 08​:01 2333 /lib/libdl-2.12.1.so
00e0c000-00e0d000 r-xp 00000000 00​:00 0 [vdso]
08048000-08323000 r-xp 00000000 08​:01 131612 /usr/local/bin/perl
08323000-08325000 rw-p 002db000 08​:01 131612 /usr/local/bin/perl
08325000-08326000 rw-p 00000000 00​:00 0
092b7000-092d8000 rw-p 00000000 00​:00 0 [heap]
b7500000-b7521000 rw-p 00000000 00​:00 0
b7521000-b7600000 ---p 00000000 00​:00 0
b7602000-b7802000 r--p 00000000 08​:01
262160 /usr/lib/locale/locale-archive
b7802000-b7805000 rw-p 00000000 00​:00 0
b7814000-b7815000 r--p 00299000 08​:01
262160 /usr/lib/locale/locale-archive
b7815000-b7817000 rw-p 00000000 00​:00 0
bfe4f000-bfe70000 rw-p 00000000 00​:00 0 [stack]
Abort

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.14.0:

Configured by robin at Sat May 14 22:14:50 BST 2011.

Summary of my perl5 (revision 5 version 14 subversion 0) configuration:
  Commit id: a35ef416833511da752c4b5b836b7a8915712aab
  Platform:
    osname=linux, osvers=2.6.35-28-generic, archname=i686-linux-64int
    uname='linux spade-ubuntu 2.6.35-28-generic #50-ubuntu smp fri mar
18 19:00:26 utc 2011 i686 gnulinux '
    config_args='-des -Dcc=gcc -Doptimize=-O2
-Wno-unused-but-set-variable -DDEBUGGING -Duse64bitint -Dman3dir=none
-Dcf_email=rmbarker.cpan@btinternet.com'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe
-fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64',
    optimize='-O2 -Wno-unused-but-set-variable -g',
    cppflags='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
    ccversion='', gccversion='4.6.0', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8,
Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /usr/lib64
    libs=-lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.12.1.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.12.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2
-Wno-unused-but-set-variable -g -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.14.0:
    /usr/local/lib/perl5/site_perl/5.14.0/i686-linux-64int
    /usr/local/lib/perl5/site_perl/5.14.0
    /usr/local/lib/perl5/5.14.0/i686-linux-64int
    /usr/local/lib/perl5/5.14.0
    /usr/local/lib/perl5/site_perl/5.12.3
    /usr/local/lib/perl5/site_perl/5.12.1
    /usr/local/lib/perl5/site_perl/5.12.0
    /usr/local/lib/perl5/site_perl
    .


Environment for perl 5.14.0:
    HOME=/home/robin
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/usr/lib:/usr/local/lib
    LOGDIR (unset)

PATH=/home/robin/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/tcsh


@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

From @iabyn

On Sat, May 21, 2011 at 07​:32​:15PM +0100, Nicholas Clark wrote​:

It's this​:

commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

\[perl \#7391\] Perl crashes with certain write\(\) formats\.

I'm certainly not going to get a chance to look at this for at least 36 hours
(and maybe not even then)

I'll have a look

--
"Foul and greedy Dwarf - you have eaten the last candle."
  -- "Hordes of the Things", BBC Radio.

@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

From @nwc10

On Sat, May 21, 2011 at 06​:09​:31PM +0100, Robin Barker wrote​:

On Sat, 2011-05-21 at 08​:56 -0700, Karl Williamson via RT wrote​:

Git bisect results​:

37ffbfc is the first bad commit
commit 37ffbfc
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Mon May 2 12​:37​:30 2011 +0100

But this is just when the test in t/op/write.t was introduced.

The underlying failure is present in perl5.12.0

env PERL_DESTRUCT_LEVEL=1 perl5.12.0 -e 'my $foo=[qw(foo)]; formline
$foo'
*** glibc detected *** perl5.12.0​: free()​: invalid next size (fast)​:
0x08ab3f90 ***
...
Abort

Bisecting with this​:

#!/bin/sh
git clean -dxf
touch .patchnum
touch .sha1
touch unpushed.h
# If you can use ccache, add -Dcc=ccache\ gcc -Dld=gcc to the Configure line
# if Encode is not needed for the test, you can speed up the bisect by
# excluding it from the runs with -Dnoextensions=Encode
sh Configure -des -Dusedevel -Uusethreads -Doptimize="-g" -Dcc=ccache\ gcc -Dld=gcc -Dnoextensions=IPC/SysV\ Encode\ DB_File
test -f config.sh || exit 125
# Correct makefile for newer GNU gcc
perl -ni -we 'print unless /<(?​:built-in|command)/' makefile x2p/makefile
# if you just need miniperl, replace test_prep with miniperl
make -j3 miniperl
[ -x ./miniperl ] || exit 125
PERL_DESTRUCT_LEVEL=2 valgrind --error-exitcode=1 ./miniperl -Ilib -e 'my $foo=[qw(foo)]; formline $foo'
ret=$?
[ $ret -gt 127 ] && ret=127
git clean -dxf
exit $ret

It's this​:

commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

  [perl #7391] Perl crashes with certain write() formats.
  Message-ID​: <20030510004523.GC20871@​fdgroup.com>
 
  p4raw-id​: //depot/perl@​19496

Inline Patch
diff --git a/pp_ctl.c b/pp_ctl.c
index 91fc2ca..8665678 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3573,11 +3573,20 @@ S_doparseform(pTHX_ SV *sv)
     U16 *linepc = 0;
     register I32 arg;
     bool ischop;
+    int maxops = 2; /* FF_LINEMARK + FF_END) */
 
     if (len == 0)
 	Perl_croak(aTHX_ "Null picture in formline");
 
-    New(804, fops, (send - s)*3+10, U16);    /* Almost certainly too long... */
+    /* estimate the buffer size needed */
+    for (base = s; s <= send; s++) {
+	if (*s == '\n' || *s == '@' || *s == '^')
+	    maxops += 10;
+    }
+    s = base;
+    base = Nullch;
+
+    New(804, fops, maxops, U16);
     fpc = fops;
 
     if (s < send) {
@@ -3740,6 +3749,7 @@ S_doparseform(pTHX_ SV *sv)
     }
     *fpc++ = FF_END;
 
+    assert (fpc <= fops + maxops); /* ensure our buffer estimate was valid */
     arg = fpc - fops;
     { /* need to jump to the next word */
         int z;



I'm certainly not going to get a chance to look at this for at least 36 hours \(and maybe not even then\)

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

From @cpansprout

This is a known issue (#91032) and Dave Mitchell is working on it. I’ll
merge the tickets.

@p5pRT
Copy link
Author

p5pRT commented May 23, 2011

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

@p5pRT
Copy link
Author

p5pRT commented May 25, 2011

From rmbarker.cpan@btinternet.com

This misbehaviour first appeared at
commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

The code prior to that change suggests that C<maxops += 10> should be
C<maxopss += 3>.

This patch follows that suggestion and includes the previous patch from
Nicholas Clark and passes all tests.

Robin

@p5pRT
Copy link
Author

p5pRT commented May 25, 2011

From rmbarker.cpan@btinternet.com

0001-fix-e-formline.patch
From d4249ec0c12855264ef8b61fa504a3d2e83dc65e Mon Sep 17 00:00:00 2001
From: Robin Barker <rmbarker@cpan.org>
Date: Tue, 24 May 2011 14:00:40 +0100
Subject: [PATCH] fix -e 'formline []'

The misbehavour of -e 'formline []' first appeared at 
commit 815f25c6e302f84ecce02c74fa717a19d787f662
Prior to that change maxops=10+3*l (where l=length) 
after than change maxops=3+10*x (for some x<l).
I think this was a transposition of 3 and 10.

This patch reverts the multiplier to 3, as well 
as including the real fix for the misbehaviour 
from Nicholas Clark.
---
 pp_ctl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pp_ctl.c b/pp_ctl.c
index f86f55c..f10e8bb 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -4940,8 +4940,8 @@ S_doparseform(pTHX_ SV *sv)
 
     /* estimate the buffer size needed */
     for (base = s; s <= send; s++) {
-	if (*s == '\n' || *s == '@' || *s == '^')
-	    maxops += 10;
+	if ( *s == '\0' || *s == '\n' || *s == '@' || *s == '^' )
+	    maxops += 3;
     }
     s = base;
     base = NULL;
-- 
1.7.1

@p5pRT
Copy link
Author

p5pRT commented May 29, 2011

From @iabyn

On Sat, May 21, 2011 at 10​:05​:49PM +0100, Dave Mitchell wrote​:

On Sat, May 21, 2011 at 07​:32​:15PM +0100, Nicholas Clark wrote​:

It's this​:

commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

\[perl \#7391\] Perl crashes with certain write\(\) formats\.

I'm certainly not going to get a chance to look at this for at least 36 hours
(and maybe not even then)

I'll have a look

Now looked at and fixed.

The basic problem is that when a format is compiled, it keeps indexes
into the original string (so for example it can print out the literal
chunks of the format). If the PVX slot of the SV can change (tie) or
doesn't exist at at all (stringification of a reference), then it all goes
to pot. Worse, in that last case, it estimates the output buffer size to
be zero, which it immediately overruns.

Frankly, pp_formline was a big mess and full of bugs and potential bombs.

I've fixed many things within it with the 20 commits running between

  3808a68

and

  a701009

inclusive, with the first one most closely addressing the bug in this ticket.

--
In the 70's we wore flares because we didn't know any better.
What possible excuse does the current generation have?

@p5pRT
Copy link
Author

p5pRT commented May 29, 2011

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

@p5pRT p5pRT closed this as completed May 29, 2011
@p5pRT
Copy link
Author

p5pRT commented May 29, 2011

From @cpansprout

On Wed May 25 12​:40​:13 2011, rmbarker.cpan@​btinternet.com wrote​:

This misbehaviour first appeared at
commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

The code prior to that change suggests that C<maxops += 10> should be
C<maxopss += 3>.

This patch follows that suggestion and includes the previous patch from
Nicholas Clark and passes all tests.

Did you compile with -DDEBUGGING? I tried your patch and got an
assertion failure​:

Assertion failed​: (fpc <= fops + maxops), function S_doparseform, file
pp_ctl.c, line 5118.

I’m probably too late in responding, because I see Dave Mitchell has
made about two dozens commits with various fixes to formline, notably
3808a68.

@p5pRT
Copy link
Author

p5pRT commented May 29, 2011

From @iabyn

On Sun, May 29, 2011 at 12​:54​:45PM -0700, Father Chrysostomos via RT wrote​:

On Wed May 25 12​:40​:13 2011, rmbarker.cpan@​btinternet.com wrote​:

This misbehaviour first appeared at
commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

The code prior to that change suggests that C<maxops += 10> should be
C<maxopss += 3>.

This patch follows that suggestion and includes the previous patch from
Nicholas Clark and passes all tests.

Did you compile with -DDEBUGGING? I tried your patch and got an
assertion failure​:

Assertion failed​: (fpc <= fops + maxops), function S_doparseform, file
pp_ctl.c, line 5118.

I’m probably too late in responding, because I see Dave Mitchell has
made about two dozens commits with various fixes to formline, notably
3808a68.

Hang about, I'm confused now. All the failures I saw were in pp_parseform,
related to buffer overruns in PL_formtarget. I never saw the assertion
failure in S_doparseform, and none of my commits would have addressed such
a failure.

--
All wight. I will give you one more chance. This time, I want to hear
no Wubens. No Weginalds. No Wudolf the wed-nosed weindeers.
  -- Life of Brian

@p5pRT
Copy link
Author

p5pRT commented May 31, 2011

From @iabyn

On Sun, May 29, 2011 at 09​:29​:49PM +0100, Dave Mitchell wrote​:

On Sun, May 29, 2011 at 12​:54​:45PM -0700, Father Chrysostomos via RT wrote​:

On Wed May 25 12​:40​:13 2011, rmbarker.cpan@​btinternet.com wrote​:

This misbehaviour first appeared at
commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

The code prior to that change suggests that C<maxops += 10> should be
C<maxopss += 3>.

This patch follows that suggestion and includes the previous patch from
Nicholas Clark and passes all tests.

Did you compile with -DDEBUGGING? I tried your patch and got an
assertion failure​:

Assertion failed​: (fpc <= fops + maxops), function S_doparseform, file
pp_ctl.c, line 5118.

I’m probably too late in responding, because I see Dave Mitchell has
made about two dozens commits with various fixes to formline, notably
3808a68.

Hang about, I'm confused now. All the failures I saw were in pp_parseform,
related to buffer overruns in PL_formtarget. I never saw the assertion
failure in S_doparseform, and none of my commits would have addressed such
a failure.

But regardless, Robin's patch looks successful.

Can anyone see the assertion failure since my series of commits?

--
"Strange women lying in ponds distributing swords is no basis for a system
of government. Supreme executive power derives from a mandate from the
masses, not from some farcical aquatic ceremony."
  -- Dennis, "Monty Python and the Holy Grail"

@p5pRT
Copy link
Author

p5pRT commented May 31, 2011

From @cpansprout

On Tue May 31 05​:56​:57 2011, davem wrote​:

On Sun, May 29, 2011 at 09​:29​:49PM +0100, Dave Mitchell wrote​:

On Sun, May 29, 2011 at 12​:54​:45PM -0700, Father Chrysostomos via RT
wrote​:

On Wed May 25 12​:40​:13 2011, rmbarker.cpan@​btinternet.com wrote​:

This misbehaviour first appeared at
commit 815f25c
Author​: Dave Mitchell <davem@​fdisolutions.com>
Date​: Sat May 10 02​:45​:23 2003 +0100

The code prior to that change suggests that C<maxops += 10>
should be
C<maxopss += 3>.

This patch follows that suggestion and includes the previous
patch from
Nicholas Clark and passes all tests.

Did you compile with -DDEBUGGING? I tried your patch and got an
assertion failure​:

Assertion failed​: (fpc <= fops + maxops), function S_doparseform,
file
pp_ctl.c, line 5118.

I’m probably too late in responding, because I see Dave Mitchell
has
made about two dozens commits with various fixes to formline,
notably
3808a68.

Hang about, I'm confused now. All the failures I saw were in
pp_parseform,
related to buffer overruns in PL_formtarget. I never saw the
assertion
failure in S_doparseform, and none of my commits would have
addressed such
a failure.

I only got an assertion failure when I tried out Robin’s patch. So I
didn’t push it.

But regardless, Robin's patch looks successful.

?

Can anyone see the assertion failure since my series of commits?

No. Nor was there one before that.

@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2011

From @iabyn

On Tue, May 31, 2011 at 08​:33​:07AM -0700, Father Chrysostomos via RT wrote​:

Hang about, I'm confused now. All the failures I saw were in
pp_parseform,
related to buffer overruns in PL_formtarget. I never saw the
assertion
failure in S_doparseform, and none of my commits would have
addressed such
a failure.

I only got an assertion failure when I tried out Robin’s patch. So I
didn’t push it.

But regardless, Robin's patch looks successful.

?

Can anyone see the assertion failure since my series of commits?

No. Nor was there one before that.

Ok I've looked more closely at this, and I'm happy now that the buffer
estimation code as-is now is correct, and that Robin's patch was wrong.

Basically each of \n, @​ and ^ can add up to 10 ops, while \0 can add 10
only if it's the last char in the string. So 12 + 10*(number of [\n@​^]) is
right.

--
print+qq&$}$"$/$s$,$a$d$g$s$@​$.$q$,$​:$.$q$^$,$@​$a$$;$.$q$m&if+map{m,^\d{0\,},,${$​::{$'}}=chr($"+=$&amp;||1)}q&10m22,42}6​:17a22.3@​3;^2dg3q/s"&=~m*\d\*.*g

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