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

Corrupt cx stack in pp_caller threaded #12131

Closed
p5pRT opened this issue May 23, 2012 · 17 comments
Closed

Corrupt cx stack in pp_caller threaded #12131

p5pRT opened this issue May 23, 2012 · 17 comments

Comments

@p5pRT
Copy link

p5pRT commented May 23, 2012

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

Searchable as RT113060$

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

From @rurban

This is a bug report for perl from rurban@​cpanel.net,
generated with the help of perlbug 1.39 running under perl 5.16.0.


AddressSanitizer detected a corrupt cx context in pp_caller.

e.g. with ./miniperl -w -Ilib -MExporter -e '<?>'

READ of size 1 at 0x7fa73ab95388 thread T0
  #0 0x6ae785 in Perl_gv_stashpvn
  #/usr/src/perl/build-5.16.0d@​asan/gv.c​:1335
  Copy(name, tmpbuf, namelen, char); name="main",namelen=9,flags=1

Note​: "main" with len 9 is wrong

  #1 0x169920f in Perl_pp_caller
  #/usr/src/perl/build-5.16.0d@​asan/pp_ctl.c​:1900
  HvNAME_HEK((HV*)CopSTASH(cx->blk_oldcop)) count=7

Note​: cx already corrupt here.

  #2 0xd0876e in Perl_runops_debug
  #/usr/src/perl/build-5.16.0d@​asan/dump.c​:2119
  #3 0x1776d1b in S_docatch /usr/src/perl/build-5.16.0d@​asan/pp_ctl.c​:3297
  #4 0x175aefc in Perl_pp_require
  #/usr/src/perl/build-5.16.0d@​asan/pp_ctl.c​:4213
  #5 0xd0876e in Perl_runops_debug
  #/usr/src/perl/build-5.16.0d@​asan/dump.c​:2119
  #6 0x476341 in Perl_call_sv /usr/src/perl/build-5.16.0d@​asan/perl.c​:2690
  #7 0x1bdbf2e in Perl__core_swash_init
  #/usr/src/perl/build-5.16.0d@​asan/utf8.c​:2832
  #8 0x1bbfeac in Perl_swash_init
  #/usr/src/perl/build-5.16.0d@​asan/utf8.c​:2740
  #9 0x1bbb8a9 in S_is_utf8_common
  #/usr/src/perl/build-5.16.0d@​asan/utf8.c​:1889
  #10 0x1bbc2f4 in Perl__is_utf8__perl_idstart
  #/usr/src/perl/build-5.16.0d@​asan/utf8.c​:1939
  #11 0x6e3793 in Perl_gv_fetchpvn_flags
  #/usr/src/perl/build-5.16.0d@​asan/gv.c​:1524
  #12 0x712271 in Perl_gv_fetchsv
  #/usr/src/perl/build-5.16.0d@​asan/gv.c​:1390
  #13 0x141ab51 in S_rv2gv /usr/src/perl/build-5.16.0d@​asan/pp.c​:231
  #14 0x140dd4c in Perl_pp_rv2gv /usr/src/perl/build-5.16.0d@​asan/pp.c​:250
  #15 0xd0876e in Perl_runops_debug
  #/usr/src/perl/build-5.16.0d@​asan/dump.c​:2119
  #16 0x477080 in Perl_call_sv
  #/usr/src/perl/build-5.16.0d@​asan/perl.c​:2705
  #17 0x446033 in Perl_call_list
  #/usr/src/perl/build-5.16.0d@​asan/perl.c​:4787
  #18 0x5e6283 in S_process_special_blocks
  #/usr/src/perl/build-5.16.0d@​asan/op.c​:6884

I added the following assertions to narrow it down​:

Inline Patch
--- ../blead/perl-git/pp_ctl.c	2012-04-27 08:58:31.962299840 -0500
+++ pp_ctl.c	2012-05-23 10:40:42.009392113 -0500
@@ -1897,6 +1897,9 @@
 	RETURN;
     }
 
+    DEBUG_CX("CALLER");
+    assert(CopSTASHPV(cx->blk_oldcop));
+    assert(SvOOK((HV*)CopSTASHPV(cx->blk_oldcop)));
     stash_hek = HvNAME_HEK((HV*)CopSTASH(cx->blk_oldcop));
     if (GIMME != G_ARRAY) {
         EXTEND(SP, 1);

but cx is already corrupt here, so the assert is not foolproof enough. I'm having trouble to find the wrong cx writer\.

Without the assert only Carp in make -c cpan/Archive-Extract/ caused the
bug to appear with asan. But with the assert I see that it is a general
problem.



Flags​:
  category=core
  severity=critical


Site configuration information for perl 5.16.0​:

Configured by rurban at Mon May 21 12​:07​:22 CDT 2012.

Summary of my perl5 (revision 5 version 16 subversion 0) configuration​:
 
  Platform​:
  osname=linux, osvers=3.2.0-2-amd64, archname=x86_64-linux-debug
  uname='linux reini 3.2.0-2-amd64 #1 smp tue mar 20 18​:36​:37 utc 2012 x86_64 gnulinux '
  config_args='-de -Dusedevel -Dinstallman1dir=none -Dinstallman3dir=none -Dinstallsiteman1dir=none -Dinstallsiteman3dir=none -DEBUGGING -Doptimize=-g3 -Uuseithreads -D'cc=/home/rurban/Software/llvm/build/Release+Asserts/bin/clang' -A'ccflags=-faddress-sanitizer' -Aldflags=-faddress-sanitizer -Alddlflags=-faddress-sanitizer -Dcf_email='rurban@​cpanel.net' -Dperladmin='rurban@​cpanel.net' -Duseshrplib'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='/home/rurban/Software/llvm/build/Release+Asserts/bin/clang', ccflags ='-faddress-sanitizer -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-g3',
  cppflags='-faddress-sanitizer -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.2.1 Compatible Clang 3.1 ((trunk 153453))', 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='/home/rurban/Software/llvm/build/Release+Asserts/bin/clang', ldflags ='-g3 -faddress-sanitizer -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=, so=so, useshrplib=true, libperl=libperl.so
  gnulibc_version='2.13'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/lib/perl5/5.16.0/x86_64-linux-debug/CORE'
  cccdlflags='-fPIC', lddlflags='-shared -g3 -faddress-sanitizer -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.16.0​:
  /usr/local/lib/perl5/site_perl/5.16.0/x86_64-linux-debug
  /usr/local/lib/perl5/site_perl/5.16.0
  /usr/local/lib/perl5/5.16.0/x86_64-linux-debug
  /usr/local/lib/perl5/5.16.0
  /usr/local/lib/perl5/site_perl
  .


Environment for perl 5.16.0​:
  HOME=/home/rurban
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/rurban/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

From @rurban

There was an obvious mistake in the pp_caller patch​:
  assert(CopSTASHPV(cx->blk_oldcop));
  assert(SvOOK((HV*)CopSTASH(cx->blk_oldcop)));

This patch was more interesting​:

Inline Patch
--- ../blead/perl-git/gv.c	2012-04-27 08:58:31.934300119 -0500
+++ gv.c	2012-05-23 18:11:19.141188997 -0500
@@ -1332,6 +1332,11 @@
 	tmpbuf = smallbuf;
     else
 	Newx(tmpbuf, tmplen, char);
+    DEBUG_v(PerlIO_printf(Perl_debug_log, "gv_stashpvn 
name=\"%s\",namelen=%d,flags=%d\n",name,namelen,flags)); \+ if \(namelen > strlen\(name\)\) \{ \+ PerlIO\_printf\(Perl\_debug\_log\, "CORE WARNING gv\_stashpvn​: namelen \(%d\) > strlen\(\\"%s\\"\) \(%ld\)\\n"\, \+ namelen\,name\,strlen\(name\)\); \+ \}   Copy\(name\, tmpbuf\, namelen\, char\);   tmpbuf\[namelen\] = '​:';   tmpbuf\[namelen\+1\] = '​:';

Which led to the wrong main namelen culprit.

../../miniperl -I../../lib Makefile.PL INSTALLDIRS=perl
INSTALLMAN1DIR=none INSTALLMAN3DIR=none PERL_CORE=1 LIBPERL_A=libperl.so
Writing Makefile for Archive​::Extract
Making all in cpan/Archive-Extract
make all PERL_CORE=1 LIBPERL_A=libperl.so
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)

=> heap overflow in gv_stashpvn, reading past name.
  Copy(name, tmpbuf, namelen, char);

So cx does not seem to be corrupt at all, it was only corrupt in
the clang gdb session which can be explained with DWARF-2
problems/mismatches between gdb and clang.
--
Reini Urban

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

From [Unknown Contact. See original ticket]

There was an obvious mistake in the pp_caller patch​:
  assert(CopSTASHPV(cx->blk_oldcop));
  assert(SvOOK((HV*)CopSTASH(cx->blk_oldcop)));

This patch was more interesting​:

Inline Patch
--- ../blead/perl-git/gv.c	2012-04-27 08:58:31.934300119 -0500
+++ gv.c	2012-05-23 18:11:19.141188997 -0500
@@ -1332,6 +1332,11 @@
 	tmpbuf = smallbuf;
     else
 	Newx(tmpbuf, tmplen, char);
+    DEBUG_v(PerlIO_printf(Perl_debug_log, "gv_stashpvn 
name=\"%s\",namelen=%d,flags=%d\n",name,namelen,flags)); \+ if \(namelen > strlen\(name\)\) \{ \+ PerlIO\_printf\(Perl\_debug\_log\, "CORE WARNING gv\_stashpvn​: namelen \(%d\) > strlen\(\\"%s\\"\) \(%ld\)\\n"\, \+ namelen\,name\,strlen\(name\)\); \+ \}   Copy\(name\, tmpbuf\, namelen\, char\);   tmpbuf\[namelen\] = '​:';   tmpbuf\[namelen\+1\] = '​:';

Which led to the wrong main namelen culprit.

../../miniperl -I../../lib Makefile.PL INSTALLDIRS=perl
INSTALLMAN1DIR=none INSTALLMAN3DIR=none PERL_CORE=1 LIBPERL_A=libperl.so
Writing Makefile for Archive​::Extract
Making all in cpan/Archive-Extract
make all PERL_CORE=1 LIBPERL_A=libperl.so
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)

=> heap overflow in gv_stashpvn, reading past name.
  Copy(name, tmpbuf, namelen, char);

So cx does not seem to be corrupt at all, it was only corrupt in
the clang gdb session which can be explained with DWARF-2
problems/mismatches between gdb and clang.
--
Reini Urban

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @rurban

Now this is something for Dave Mitchell

On Wed, May 23, 2012 at 6​:30 PM, Reini Urban via RT
<perlbug-comment@​perl.org> wrote​:

There was an obvious mistake in the pp_caller patch​:
   assert(CopSTASHPV(cx->blk_oldcop));
   assert(SvOOK((HV*)CopSTASH(cx->blk_oldcop)));

This patch was more interesting​:
--- ../blead/perl-git/gv.c      2012-04-27 08​:58​:31.934300119 -0500
+++ gv.c        2012-05-23 18​:11​:19.141188997 -0500
@​@​ -1332,6 +1332,11 @​@​
       tmpbuf = smallbuf;
    else
       Newx(tmpbuf, tmplen, char);
+    DEBUG_v(PerlIO_printf(Perl_debug_log, "gv_stashpvn
name=\"%s\",namelen=%d,flags=%d\n",name,namelen,flags));
+    if (namelen > strlen(name)) {
+       PerlIO_printf(Perl_debug_log, "CORE WARNING gv_stashpvn​: namelen
(%d) > strlen(\"%s\") (%ld)\n",
+                     namelen,name,strlen(name));
+    }
    Copy(name, tmpbuf, namelen, char);
    tmpbuf[namelen]   = '​:';
    tmpbuf[namelen+1] = '​:';

Which led to the wrong main namelen culprit.

../../miniperl -I../../lib Makefile.PL INSTALLDIRS=perl
INSTALLMAN1DIR=none INSTALLMAN3DIR=none PERL_CORE=1 LIBPERL_A=libperl.so
Writing Makefile for Archive​::Extract
Making all in cpan/Archive-Extract
 make all PERL_CORE=1 LIBPERL_A=libperl.so
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)
CORE WARNING gv_stashpvn​: namelen (9) > strlen("main") (4)

=> heap overflow in gv_stashpvn, reading past name.
 Copy(name, tmpbuf, namelen, char);

So cx does not seem to be corrupt at all, it was only corrupt in
the clang gdb session which can be explained with DWARF-2
problems/mismatches between gdb and clang.

I finally found the culprit who writes the wrong cx->blk_oldcop->cop_stashlen.

I used this gdb trick​:
watch -l PL_compiling->cop_stashpv
watch -l PL_compiling->cop_stashlen

Perl_sv_compile_2op_is_broken() does at line 3354 a LEAVE_with_name("eval"),
a SSPOPSTR via SAVEt_SHARED_PVREF for the localized cop_stashpv, but
not for the
cop_stashlen.
The cop in question is PL_compiling, which was "AutoSplit" before with
len=9 and
restores it back to "main" but keeps len 9. Thus leading to the
heap-overflow in gv_stashpvn.
So I think blk_oldcop->cop_stashlen must be localized also.

SAVECOPSTASH does SAVEI32(CopSTASH_len(c)) though, but I found no call
to SAVECOPSTASH in
Perl_sv_compile_2op_is_broken().

How to repro​:
cd cpan/Archive-Extract
rm ../../lib/Archive/Extract.pm
gdb --args ../../miniperl "-I../../lib" -MExtUtils​::Install -e
'pm_to_blib({@​ARGV}, '\''../../lib/auto'\'', q[], '\''755'\'')' -- \
  lib/Archive/Extract.pm ../../lib/Archive/Extract.pm

(gdb) bt
#0 Perl_create_eval_scope (my_perl=0x808010, flags=256) at pp_ctl.c​:4463
#1 0x0000000000427c46 in S_fold_constants (my_perl=0x808010,
o=0x9d2cd0) at op.c​:3008
#2 0x0000000000428e89 in Perl_convert (my_perl=0x808010, type=68,
flags=0, o=0x9d2cd0) at op.c​:3147
#3 0x00000000004c0f61 in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:771
#4 0x0000000000664c8a in S_doeval (my_perl=0x808010, gimme=2,
startop=0x0, outside=0x0, seq=100, hh=0x0) at pp_ctl.c​:3667
#5 0x000000000066bfe9 in Perl_pp_require (my_perl=0x808010) at pp_ctl.c​:4224
#6 0x0000000000523a81 in Perl_runops_debug (my_perl=0x808010) at dump.c​:2119
#7 0x000000000040f4b5 in Perl_call_sv (my_perl=0x808010, sv=0x83c688,
flags=13) at perl.c​:2705
#8 0x000000000041bcc3 in Perl_call_list (my_perl=0x808010,
oldscope=9, paramList=0x83c400) at perl.c​:4782
#9 0x000000000043a76d in S_process_special_blocks (my_perl=0x808010,
fullname=0x8356b8 "BEGIN", gv=0x83c658, cv=0x83c688) at op.c​:6882
#10 0x000000000043a427 in Perl_newATTRSUB_flags (my_perl=0x808010,
floor=173, o=0x83d590, proto=0x0, attrs=0x0, block=0x837aa0, flags=0)
  at op.c​:6852
#11 0x0000000000435ec3 in Perl_newATTRSUB (my_perl=0x808010,
floor=173, o=0x83d590, proto=0x0, attrs=0x0, block=0x837aa0) at
op.c​:6499
#12 0x000000000042ea82 in Perl_utilize (my_perl=0x808010, aver=1,
floor=173, version=0x0, idop=0x84a720, arg=0x0) at op.c​:4681
#13 0x00000000004bf7d0 in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:363
#14 0x0000000000664c8a in S_doeval (my_perl=0x808010, gimme=2,
startop=0x0, outside=0x0, seq=4294967248, hh=0x0) at pp_ctl.c​:3667
#15 0x000000000066bfe9 in Perl_pp_require (my_perl=0x808010) at pp_ctl.c​:4224
#16 0x0000000000523a81 in Perl_runops_debug (my_perl=0x808010) at dump.c​:2119
#17 0x000000000040f4b5 in Perl_call_sv (my_perl=0x808010, sv=0x82a6d0,
flags=13) at perl.c​:2705
#18 0x000000000041bcc3 in Perl_call_list (my_perl=0x808010,
oldscope=2, paramList=0x82a790) at perl.c​:4782
#19 0x000000000043a76d in S_process_special_blocks (my_perl=0x808010,
fullname=0x8356b8 "BEGIN", gv=0x82a700, cv=0x82a6d0) at op.c​:6882
#20 0x000000000043a427 in Perl_newATTRSUB_flags (my_perl=0x808010,
floor=37, o=0x835410, proto=0x0, attrs=0x0, block=0x835450, flags=0)
  at op.c​:6852
#21 0x0000000000435ec3 in Perl_newATTRSUB (my_perl=0x808010, floor=37,
o=0x835410, proto=0x0, attrs=0x0, block=0x835450) at op.c​:6499
#22 0x000000000042ea82 in Perl_utilize (my_perl=0x808010, aver=1,
floor=37, version=0x0, idop=0x8223c0, arg=0x0) at op.c​:4681
#23 0x00000000004bf7d0 in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:363
#24 0x000000000040c8bc in S_parse_body (my_perl=0x808010, env=0x0,
xsinit=0x45023b <xs_init>) at perl.c​:2246
#25 0x000000000040aeed in perl_parse (my_perl=0x808010,
xsinit=0x45023b <xs_init>, argc=9, argv=0x7fffffffe618, env=0x0) at
perl.c​:1633
#26 0x000000000045016c in main (argc=9, argv=0x7fffffffe618,
env=0x7fffffffe668) at miniperlmain.c​:117

The missing cop_stashlen write​:
(gdb)
3445 (void) doeval(G_SCALAR, startop, PL_compcv, PL_cop_seqmax, NULL);
(gdb) c
Continuing.
Hardware watchpoint 7​: -location PL_compiling->cop_stashpv

Old value = 0x9c7340 "\360É"
New value = 0x811640 "main"
Perl_leave_scope (my_perl=0x808010, base=347) at scope.c​:768
768 break;
(gdb) bt
#0 Perl_leave_scope (my_perl=0x808010, base=347) at scope.c​:768
#1 0x000000000063737f in Perl_pop_scope (my_perl=0x808010) at scope.c​:110
#2 0x0000000000662e30 in Perl_sv_compile_2op_is_broken
(my_perl=0x808010, sv=0x9c5a50, startop=0x7fffffff98b8, code=0x76efa7
"re",
  padp=0x7fffffff98b0) at pp_ctl.c​:3454
#3 0x00000000004f3896 in S_reg (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, paren=123, flagp=0x7fffffff9e94, depth=9)
at regcomp.c​:7921
#4 0x00000000004fb659 in S_regatom (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7fffffff9fe0, depth=8) at
regcomp.c​:9054
#5 0x00000000004f80fd in S_regpiece (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7fffffffa088, depth=7) at
regcomp.c​:8544
#6 0x00000000004f7aab in S_regbranch (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7fffffffa470, first=0, depth=6) at
regcomp.c​:8484
#7 0x00000000004f6637 in S_reg (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, paren=58, flagp=0x7fffffffa704, depth=5)
at regcomp.c​:8337
#8 0x00000000004fb659 in S_regatom (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7fffffffa850, depth=4) at
regcomp.c​:9054
#9 0x00000000004f80fd in S_regpiece (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7fffffffa8f8, depth=3) at
regcomp.c​:8544
#10 0x00000000004f7aab in S_regbranch (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, flagp=0x7ffffffface0, first=1, depth=2) at
regcomp.c​:8484
#11 0x00000000004f60e2 in S_reg (my_perl=0x808010,
pRExC_state=0x7fffffffadc0, paren=0, flagp=0x7fffffffb404, depth=1) at
regcomp.c​:8295
#12 0x00000000004e76e3 in Perl_re_compile (my_perl=0x808010,
pattern=0x9c5a38, orig_pm_flags=8) at regcomp.c​:5235
#13 0x00000000004e505f in Perl_pregcomp (my_perl=0x808010,
pattern=0x9c5a38, flags=8) at regcomp.c​:4861
#14 0x000000000042cc44 in Perl_pmruntime (my_perl=0x808010,
o=0x9c7360, expr=0x9c7230, isreg=true) at op.c​:4273
#15 0x00000000004c2c8b in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:1263
#16 0x0000000000664c8a in S_doeval (my_perl=0x808010, gimme=2,
startop=0x0, outside=0x0, seq=100, hh=0x0) at pp_ctl.c​:3667
#17 0x000000000066bfe9 in Perl_pp_require (my_perl=0x808010) at pp_ctl.c​:4224
#18 0x0000000000523a81 in Perl_runops_debug (my_perl=0x808010) at dump.c​:2119
#19 0x000000000040f4b5 in Perl_call_sv (my_perl=0x808010, sv=0x83a658,
flags=13) at perl.c​:2705
#20 0x000000000041bcc3 in Perl_call_list (my_perl=0x808010,
oldscope=9, paramList=0x83a3d0) at perl.c​:4782
#21 0x000000000043a76d in S_process_special_blocks (my_perl=0x808010,
fullname=0x8356b8 "BEGIN", gv=0x83a628, cv=0x83a658) at op.c​:6882
#22 0x000000000043a427 in Perl_newATTRSUB_flags (my_perl=0x808010,
floor=173, o=0x83b560, proto=0x0, attrs=0x0, block=0x835a30, flags=0)
  at op.c​:6852
#23 0x0000000000435ec3 in Perl_newATTRSUB (my_perl=0x808010,
floor=173, o=0x83b560, proto=0x0, attrs=0x0, block=0x835a30) at
op.c​:6499
#24 0x000000000042ea82 in Perl_utilize (my_perl=0x808010, aver=1,
floor=173, version=0x0, idop=0x847e20, arg=0x0) at op.c​:4681
#25 0x00000000004bf7d0 in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:363
#26 0x0000000000664c8a in S_doeval (my_perl=0x808010, gimme=2,
startop=0x0, outside=0x0, seq=4294967248, hh=0x0) at pp_ctl.c​:3667
#27 0x000000000066bfe9 in Perl_pp_require (my_perl=0x808010) at pp_ctl.c​:4224
#28 0x0000000000523a81 in Perl_runops_debug (my_perl=0x808010) at dump.c​:2119
#29 0x000000000040f4b5 in Perl_call_sv (my_perl=0x808010, sv=0x82a6d0,
flags=13) at perl.c​:2705
#30 0x000000000041bcc3 in Perl_call_list (my_perl=0x808010,
oldscope=2, paramList=0x82a790) at perl.c​:4782
#31 0x000000000043a76d in S_process_special_blocks (my_perl=0x808010,
fullname=0x8356b8 "BEGIN", gv=0x82a700, cv=0x82a6d0) at op.c​:6882
#32 0x000000000043a427 in Perl_newATTRSUB_flags (my_perl=0x808010,
floor=37, o=0x835410, proto=0x0, attrs=0x0, block=0x835450, flags=0)
  at op.c​:6852
#33 0x0000000000435ec3 in Perl_newATTRSUB (my_perl=0x808010, floor=37,
o=0x835410, proto=0x0, attrs=0x0, block=0x835450) at op.c​:6499
#34 0x000000000042ea82 in Perl_utilize (my_perl=0x808010, aver=1,
floor=37, version=0x0, idop=0x8223c0, arg=0x0) at op.c​:4681
#35 0x00000000004bf7d0 in Perl_yyparse (my_perl=0x808010,
gramtype=258) at perly.y​:363
#36 0x000000000040c8bc in S_parse_body (my_perl=0x808010, env=0x0,
xsinit=0x45023b <xs_init>) at perl.c​:2246
#37 0x000000000040aeed in perl_parse (my_perl=0x808010,
xsinit=0x45023b <xs_init>, argc=9, argv=0x7fffffffe618, env=0x0) at
perl.c​:1633
---Type <return> to continue, or q <return> to quit---n
#38 0x000000000045016c in main (argc=9, argv=0x7fffffffe618,
env=0x7fffffffe668) at miniperlmain.c​:117

--
Reini Urban
http​://cpanel.net/   http​://www.perl-compiler.org/

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @iabyn

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and then
see if the problem has gone away? Or is this something I need to look
into now?

--
Red sky at night - gerroff my land!
Red sky at morning - gerroff my land!
  -- old farmers' sayings #14

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @cpansprout

On Tue May 29 12​:10​:26 2012, davem wrote​:

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a
LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it
does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and
then
see if the problem has gone away? Or is this something I need to look
into now?

I think this is a regression in 5.16.0, due to one of my fumbling commits.

So fixing it for backport to 5.16.1 would be good. Please don’t merge
your regexp branch yet!

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

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

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @rurban

On Tue, May 29, 2012 at 2​:36 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Tue May 29 12​:10​:26 2012, davem wrote​:

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a
LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it
does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and
then
see if the problem has gone away? Or is this something I need to look
into now?

I think this is a regression in 5.16.0, due to one of my fumbling commits.

So fixing it for backport to 5.16.1 would be good.  Please don’t merge
your regexp branch yet!

Found the proper patch finally (see attachment), for 5.16.1 and blead.
#define SAVECOPSTASH_FREE(c) (SAVESHAREDPV(CopSTASHPV(c)), \
  SAVEI32(CopSTASH_len(c)))
Even if the hek stores the len, the cop needs to be updated also.
All tests pass now.

This patch is independent of sv_compile_2op_is_broken and dave's branch.
--
Reini Urban
http​://cpanel.net/   http​://www.perl-compiler.org/

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @rurban

0001-perl-113060-Save-cop_stashlen-threaded-even-with-sha.patch
From 515d7310f01ebe2ca3af7bc3186a8a7504be12cd Mon Sep 17 00:00:00 2001
From: Reini Urban <rurban@x-ray.at>
Date: Tue, 29 May 2012 15:46:13 -0500
Subject: [PATCH] [perl #113060] Save cop_stashlen threaded even with shared
 cop pv

Perl_sv_compile_2op_is_broken() does at line 3354 a LEAVE_with_name("eval"),
a SSPOPSTR via SAVEt_SHARED_PVREF for the localized cop_stashpv, but
not for the cop_stashlen.
The cop in question is PL_compiling, which was "AutoSplit" before with
len=9 and restores it back to "main" but keeps len 9. Thus leading to a
heap-overflow in gv_stashpvn.
---
 scope.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scope.h b/scope.h
index aa04a79..38da244 100644
--- a/scope.h
+++ b/scope.h
@@ -237,7 +237,8 @@ scope has the given name. Name must be a literal string.
 #ifdef USE_ITHREADS
 #  define SAVECOPSTASH(c)	(SAVEPPTR(CopSTASHPV(c)), \
 				 SAVEI32(CopSTASH_len(c)))
-#  define SAVECOPSTASH_FREE(c)	SAVESHAREDPV(CopSTASHPV(c))
+#  define SAVECOPSTASH_FREE(c)	(SAVESHAREDPV(CopSTASHPV(c)), \
+	                         SAVEI32(CopSTASH_len(c)))
 #  define SAVECOPFILE(c)	SAVEPPTR(CopFILE(c))
 #  define SAVECOPFILE_FREE(c)	SAVESHAREDPV(CopFILE(c))
 #else
-- 
1.7.10

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @rurban

The attached 2nd patch also adds some useful assertions I needed while
debugging
[perl #113060].
--
Reini Urban
http​://cpanel.net/   http​://www.perl-compiler.org/

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @rurban

0002-related-to-perl-113060-assert-CopSTASH-cx-blk_oldcop.patch
From 14be56261fc9be8a1ef3b5dff74fce98a0b747b4 Mon Sep 17 00:00:00 2001
From: Reini Urban <rurban@x-ray.at>
Date: Tue, 29 May 2012 16:06:00 -0500
Subject: [PATCH] related to [perl #113060] assert CopSTASH(cx->blk_oldcop)

HvNAME_HEK((HV*)CopSTASH(cx->blk_oldcop)) does some implicit assumptions, which should
better be asserted. Also record CX pp_caller reads besides PUSH and POP.
---
 pp_ctl.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/pp_ctl.c b/pp_ctl.c
index 4e54bd3..34fe990 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1897,6 +1897,9 @@ PP(pp_caller)
 	RETURN;
     }
 
+    DEBUG_CX("CALLER");
+    assert(CopSTASHPV(cx->blk_oldcop));
+    assert(SvOOK((HV*)CopSTASH(cx->blk_oldcop)));
     stash_hek = HvNAME_HEK((HV*)CopSTASH(cx->blk_oldcop));
     if (GIMME != G_ARRAY) {
         EXTEND(SP, 1);
-- 
1.7.10

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

From @cpansprout

On Tue May 29 13​:53​:41 2012, rurban wrote​:

On Tue, May 29, 2012 at 2​:36 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Tue May 29 12​:10​:26 2012, davem wrote​:

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a
LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it
does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and
then
see if the problem has gone away? Or is this something I need to look
into now?

I think this is a regression in 5.16.0, due to one of my fumbling
commits.

So fixing it for backport to 5.16.1 would be good.  Please don’t merge
your regexp branch yet!

Found the proper patch finally (see attachment), for 5.16.1 and blead.
#define SAVECOPSTASH_FREE(c) (SAVESHAREDPV(CopSTASHPV(c)), \
SAVEI32(CopSTASH_len(c)))
Even if the hek stores the len, the cop needs to be updated also.
All tests pass now.

This patch is independent of sv_compile_2op_is_broken and dave's branch.

Thank you. I’ve applied this as 014243a and your second patch as
fb55fee.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented May 29, 2012

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

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

p5pRT commented Jun 1, 2012

From @cpansprout

On Tue May 29 13​:53​:41 2012, rurban wrote​:

On Tue, May 29, 2012 at 2​:36 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Tue May 29 12​:10​:26 2012, davem wrote​:

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a
LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it
does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and
then
see if the problem has gone away? Or is this something I need to look
into now?

I think this is a regression in 5.16.0, due to one of my fumbling
commits.

So fixing it for backport to 5.16.1 would be good.  Please don’t merge
your regexp branch yet!

Found the proper patch finally (see attachment), for 5.16.1 and blead.
#define SAVECOPSTASH_FREE(c) (SAVESHAREDPV(CopSTASHPV(c)), \
SAVEI32(CopSTASH_len(c)))
Even if the hek stores the len, the cop needs to be updated also.
All tests pass now.

This patch is independent of sv_compile_2op_is_broken and dave's branch.

I’ve been thinking about moving CopSTASH into the pad for threaded
perls. If we store a pad offset in cop->cop_stashoff, and update the
macros to deal with it, we could simplify a lot of code.

One thing that concerns me, though, is PL_compiling. Would it be
possible for that to contain a pad offset? Or would PL_compiling have
to be treated differently?

Another difficulty is SAVECOPSTASH(_FREE). Currently only newCONSTSUB
uses SAVECOPSTASH and only sv_compile_2op_is_broken uses
SAVECOPSTASH_FREE. The former doesn’t even need it, I don’t think. (I
have a patch on my sprout/copstash branch, but I’m still running tests
locally.) If the latter is going away, that solves the other half of
the problem.

Dave, can you merge your re_eval branch asap? :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 4, 2012

From @rurban

On Fri, Jun 1, 2012 at 1​:12 AM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Tue May 29 13​:53​:41 2012, rurban wrote​:

On Tue, May 29, 2012 at 2​:36 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Tue May 29 12​:10​:26 2012, davem wrote​:

On Tue, May 29, 2012 at 01​:05​:59PM -0500, Reini Urban wrote​:

Now this is something for Dave Mitchell
[snip lots of stuff I haven't read in detail yet]
Perl_sv_compile_2op_is_broken() does at line 3354 a
LEAVE_with_name("eval"),

In my re_eval branch, Perl_sv_compile_2op_is_broken() and the way it
does
stuff has completely gone.

Is it likely that what you're seeing is related to the bad code around
there? If so, can we wait until re_eval is merged back into blead and
then
see if the problem has gone away? Or is this something I need to look
into now?

I think this is a regression in 5.16.0, due to one of my fumbling
commits.

So fixing it for backport to 5.16.1 would be good.  Please don’t merge
your regexp branch yet!

Found the proper patch finally (see attachment), for 5.16.1 and blead.
#define SAVECOPSTASH_FREE(c)  (SAVESHAREDPV(CopSTASHPV(c)), \
                               SAVEI32(CopSTASH_len(c)))
Even if the hek stores the len, the cop needs to be updated also.
All tests pass now.

This patch is independent of sv_compile_2op_is_broken and dave's branch.

I’ve been thinking about moving CopSTASH into the pad for threaded
perls.  If we store a pad offset in cop->cop_stashoff, and update the
macros to deal with it, we could simplify a lot of code.

One thing that concerns me, though, is PL_compiling.  Would it be
possible for that to contain a pad offset?  Or would PL_compiling have
to be treated differently?

Another difficulty is SAVECOPSTASH(_FREE).  Currently only newCONSTSUB
uses SAVECOPSTASH and only sv_compile_2op_is_broken uses
SAVECOPSTASH_FREE.  The former doesn’t even need it, I don’t think.  (I
have a patch on my sprout/copstash branch, but I’m still running tests
locally.)  If the latter is going away, that solves the other half of
the problem.

I was thinking something else.
cop_stashlen is redundant at all.

Since stashes are now always stored shared as hek, we can just use the
hek_len for this.
There is also a utf8 flag in hek's.
There was one use-case while testing (involving gv_stashpvn and
AUTOLOAD) that the passed gv_stashpvn
len was different to the existing hek_len, i.e. the new stash was a
substr to the existing stash, but this does not
involve the need to hold a cop field for this, as the shorter stash
was copied then into a new strtab key.

I"ll investigate.
--
Reini Urban
http​://cpanel.net/   http​://www.perl-compiler.org/

@p5pRT
Copy link
Author

p5pRT commented Jun 5, 2012

From @cpansprout

On Mon Jun 04 16​:21​:22 2012, rurban wrote​:

I was thinking something else.
cop_stashlen is redundant at all.

Since stashes are now always stored shared as hek, we can just use the
hek_len for this.
There is also a utf8 flag in hek's.
There was one use-case while testing (involving gv_stashpvn and
AUTOLOAD) that the passed gv_stashpvn
len was different to the existing hek_len, i.e. the new stash was a
substr to the existing stash, but this does not
involve the need to hold a cop field for this, as the shorter stash
was copied then into a new strtab key.

I"ll investigate.

With commit d4d0394 I’ve changed the way CopSTASH is stored under threads.

I’ve just realised I stopped B​::COP​::stashlen from working.

But what you say suggests that it is not needed.

B​::COP​::stashpv doesn’t currently work with UTF8 and embedded nulls, but
I can fix that. Once it is fixed, is that sufficient for your needs?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2012

From @cpansprout

On Mon Jun 04 20​:32​:06 2012, sprout wrote​:

B​::COP​::stashpv doesn’t currently work with UTF8 and embedded nulls, but
I can fix that.

Which has now happened with commit 9922583.

--

Father Chrysostomos

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