segmentation fault with array ties #9249

p5pRT · 2008-03-12T09:59:43Z

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

Searchable as RT51636$

p5pRT · 2008-03-12T09:59:44Z

From blino@mandriva.com

Created by blino@mandriva.com

Perl aborts with a segmentation fault when using array tie in array
ties. The following testcase allows to reproduce it.

#!/usr/bin/perl

my $view;
my @a;
tie @a, 'TiedList';
$view->{data} = \@a;
@{$view->{data}} = (1);

package TiedList;

use strict;
use Carp;

sub TIEARRAY {
my $class = shift;
return bless {}, $class;
}

sub FETCH { 0 }

sub STORE { # this, index, value
warn "store";
my @row;
tie @row, 'TiedRow';
warn "stored";
return $_[2];
}
sub FETCHSIZE { croak }
sub PUSH { croak }
sub POP { croak }
sub SHIFT { croak }
sub UNSHIFT { croak }
sub DELETE { croak }
sub CLEAR { [] }
sub EXISTS { croak }
sub EXTEND { warn "extend" }
sub STORESIZE { croak }
sub SPLICE { croak }

1;

package TiedRow;

use strict;
use Carp;

sub TIEARRAY {
my $class = shift;
return bless {}, $class;
}

sub FETCH { 0 }

sub STORE { $_[2] }

sub FETCHSIZE { croak }
sub EXISTS { croak }

sub EXTEND { croak }
sub CLEAR { croak }

sub POP { croak "pop called on a TiedRow, but you can't change its size"; }
sub PUSH { croak "push called on a TiedRow, but you can't change its size"; }
sub SHIFT { croak "shift called on a TiedRow, but you can't change its size"; }
sub UNSHIFT { croak "unshift called on a TiedRow, but you can't change its size"; }
sub SPLICE { croak "splice called on a TiedRow, but you can't change its size"; }
sub DELETE { croak "delete called on a TiedRow, but you can't change its size"; }
sub STORESIZE { carp "STORESIZE operation not supported"; }

1;

The testcase may look awkward, it has been written starting from
Gtk2::SimpleList (which originally triggered the bug).

The bug does not happen with perl 5.8.8, it looks like the bug has been
introduced by change 31770 ("optimize push @ISA").

This naive patch fixes the issue:

Inline Patch

diff -p -up perl-5.10.0/av.c.av_mg perl-5.10.0/av.c
--- perl-5.10.0/av.c.av_mg      2007-12-18 11:47:07.000000000 +0100
+++ perl-5.10.0/av.c    2008-03-12 10:40:56.000000000 +0100
@@ -433,7 +433,7 @@ Perl_av_clear(pTHX_ register AV *av)
     /* Give any tie a chance to cleanup first */
     if (SvRMAGICAL(av)) {
        const MAGIC* const mg = SvMAGIC(av);
-       if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+       if (PL_delaymagic && mg && mg->mg_type == PERL_MAGIC_isa)
            PL_delaymagic |= DM_ARRAY;
         else
            mg_clear((SV*)av);

Perl Info


Flags:
    category=core
    severity=medium

Site configuration information for perl 5.10.0:

Configured by Mandriva at Mon Mar 10 12:15:15 EDT 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.6.22.12-server-1mdv, archname=i386-linux-thread-multi
    uname='linux n5.mandriva.com 2.6.22.12-server-1mdv #1 smp tue nov 20 09:35:09 est 2007 i686 intel(r) xeon(tm) cpu 2.80ghz gnulinux '
    config_args='-des -Dinc_version_list=5.8.8 5.8.7 5.8.6 5.8.5 5.8.4 5.8.3 5.8.2 5.8.1 5.8.0 5.6.1 5.6.0 -Darchname=i386-linux -Dcc=gcc -Doptimize=-O2  -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr -Dsitebin=/usr/local/bin -Dsiteman1dir=/usr/local/share/man/man1 -Dsiteman3dir=/usr/local/share/man/man3 -Dman3ext=3pm -Dcf_by=Mandriva -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=root@localhost -Dd_dosuid -Ud_csh -Duseshrplib -Duseithreads'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='4.2.2 20071128 (prerelease) (4.2.2-2mdv2008.1)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lndbm -lgdbm -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.7.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.7'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib/perl5/5.10.0/i386-linux-thread-multi/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables -L/usr/local/lib'

Locally applied patches:
    Mandriva Linux patches


@INC for perl 5.10.0:
    /usr/lib/perl5/site_perl/5.10.0/i386-linux-thread-multi
    /usr/lib/perl5/site_perl/5.10.0
    /usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.10.0
    /usr/lib/perl5/5.10.0/i386-linux-thread-multi
    /usr/lib/perl5/5.10.0
    /usr/lib/perl5/site_perl/5.8.7
    /usr/lib/perl5/site_perl
    /usr/lib/perl5/vendor_perl/5.8.8
    /usr/lib/perl5/vendor_perl/5.8.7
    /usr/lib/perl5/vendor_perl/5.8.6
    /usr/lib/perl5/vendor_perl/5.8.5
    /usr/lib/perl5/vendor_perl/5.8.4
    /usr/lib/perl5/vendor_perl/5.8.3
    /usr/lib/perl5/vendor_perl
    .


Environment for perl 5.10.0:
    HOME=/home/blino
    LANG=fr_FR
    LANGUAGE=fr_FR:fr
    LC_ADDRESS=fr_FR
    LC_COLLATE=fr_FR
    LC_CTYPE=fr_FR
    LC_IDENTIFICATION=fr_FR
    LC_MEASUREMENT=fr_FR
    LC_MESSAGES=fr_FR
    LC_MONETARY=fr_FR
    LC_NAME=fr_FR
    LC_NUMERIC=fr_FR
    LC_PAPER=fr_FR
    LC_SOURCED=1
    LC_TELEPHONE=fr_FR
    LC_TIME=fr_FR
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/bin:/usr/bin:/usr/local/bin:/usr/games:/usr/lib/qt4/bin:/usr/lib/qt4/bin:/home/blino/bin:/usr/lib/qt4/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

p5pRT · 2008-03-12T15:01:50Z

From @rgs

On 12/03/2008, via RT blino @ mandriva. com wrote:

The bug does not happen with perl 5.8.8, it looks like the bug has been
introduced by change 31770 ("optimize push @ISA").

Thanks for this info!

This naive patch fixes the issue:
diff -p -up perl-5.10.0/av.c.av_mg perl-5.10.0/av.c
--- perl-5.10.0/av.c.av_mg 2007-12-18 11:47:07.000000000 +0100
+++ perl-5.10.0/av.c 2008-03-12 10:40:56.000000000 +0100
@@ -433,7 +433,7 @@ Perl_av_clear(pTHX_ register AV *av)
/* Give any tie a chance to cleanup first */
if (SvRMAGICAL(av)) {
const MAGIC* const mg = SvMAGIC(av);
- if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ if (PL_delaymagic && mg && mg->mg_type == PERL_MAGIC_isa)

The patch looks good and is obviously safe to apply, but I would like to
understand as well why mg ends up being null whereas SvRMAGICAL(av) is
still true. It might indicate a deeper bug in magic handling.

Not sure where the regression test should go. t/op/tiearray.t ?

p5pRT · 2008-03-12T15:01:52Z

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

p5pRT · 2008-03-12T16:39:06Z

From perl@profvince.com

- if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ if (PL_delaymagic && mg && mg->mg_type == PERL_MAGIC_isa)

The patch looks good and is obviously safe to apply, but I would like to
understand as well why mg ends up being null whereas SvRMAGICAL(av) is
still true. It might indicate a deeper bug in magic handling.

Not sure where the regression test should go. t/op/tiearray.t ?

The problem is that delaymagic is set. This happens because the second
array is created and deleted in the context of a tied array assignment,
which sets delaymagic to delay its own magic. av_clears thinks that the
magic is delayed for the inner array, while it's for the outer.

The attached patch fixes this behaviour by copying PL_delaymagic before
calling the magic callbacks, and setting it back after (ok with 33493). A
reduced test case is also attached.

p5pRT · 2008-03-12T16:39:06Z

From perl@profvince.com

tie.pl

p5pRT · 2008-03-12T16:39:06Z

From perl@profvince.com

delaymagic.patch

--- pp_hot.c	2008-03-10 18:39:48.000000000 +0100
+++ pp_hot.c	2008-03-12 17:03:58.000000000 +0100
@@ -1020,8 +1020,14 @@
 		*(relem++) = sv;
 		didstore = av_store(ary,i++,sv);
 		if (magic) {
-		    if (SvSMAGICAL(sv))
+		    if (SvSMAGICAL(sv)) {
+			/* More magic can happen in the mg_set callback, so we
+			 * backup the delaymagic for now. */
+			U16 dmbak = PL_delaymagic;
+			PL_delaymagic = 0;
 			mg_set(sv);
+			PL_delaymagic |= dmbak;
+		    }
 		    if (!didstore)
 			sv_2mortal(sv);
 		}
@@ -1051,8 +1057,12 @@
 			duplicates += 2;
 		    didstore = hv_store_ent(hash,sv,tmpstr,0);
 		    if (magic) {
-			if (SvSMAGICAL(tmpstr))
+			if (SvSMAGICAL(tmpstr)) {
+			    U16 dmbak = PL_delaymagic;
+			    PL_delaymagic = 0;
 			    mg_set(tmpstr);
+			    PL_delaymagic |= dmbak;
+			}
 			if (!didstore)
 			    sv_2mortal(tmpstr);
 		    }
@@ -1076,7 +1086,13 @@
 	    }
 	    else
 		sv_setsv(sv, &PL_sv_undef);
-	    SvSETMAGIC(sv);
+
+	    if (SvSMAGICAL(sv)) {
+		U16 dmbak = PL_delaymagic;
+		PL_delaymagic = 0;
+		mg_set(sv);
+		PL_delaymagic |= dmbak;
+	    }
 	    break;
 	}
     }

p5pRT · 2008-03-12T16:46:56Z

From @nwc10

Since your mailer isn't using the dreaded text-mangling Format Flowed,
sending the patch inline would work safely.

On Wed, Mar 12, 2008 at 05:37:40PM +0100, Vincent Pit wrote:

The problem is that delaymagic is set. This happens because the second
array is created and deleted in the context of a tied array assignment,
which sets delaymagic to delay its own magic. av_clears thinks that the
magic is delayed for the inner array, while it's for the outer.

Nice debugging.

The attached patch fixes this behaviour by copying PL_delaymagic before
calling the magic callbacks, and setting it back after (ok with 33493). A
reduced test case is also attached.

You do this:

Inline Patch

--- pp_hot.c	2008-03-10 18:39:48.000000000 +0100
+++ pp_hot.c	2008-03-12 17:03:58.000000000 +0100
@@ -1020,8 +1020,14 @@
 		*(relem++) = sv;
 		didstore = av_store(ary,i++,sv);
 		if (magic) {
-		    if (SvSMAGICAL(sv))
+		    if (SvSMAGICAL(sv)) {
+			/* More magic can happen in the mg_set callback, so we
+			 * backup the delaymagic for now. */
+			U16 dmbak = PL_delaymagic;
+			PL_delaymagic = 0;
 			mg_set(sv);
+			PL_delaymagic |= dmbak;
+		    }
 		    if (!didstore)
 			sv_2mortal(sv);
 		}

I'm not sure if I'm suggesting cargo-culting here based on previous experiences with global variables\, but would it be better to do this with the save stack $um\, SAVEI16\($\, er\, isn't quite U16\)? $Which I figure is going to add the expense of an ENTER and LEAVE around the call$

Or if an exception is thrown within the mg_set(), does it propagate up so
far that the temporary state of PL_delaymagic doesn't matter? In this case,
it's not an allocated value that is able to be leaked.

Nicholas Clark

p5pRT · 2008-03-12T16:52:53Z

From @smpeters

I've applied this patch as change #33495 since it passes all the tests
and and stops the core dump. Some of the other patches may be helpful
as well. I'll test them all out in a moment.

Steve

On Wed, Mar 12, 2008 at 4:59 AM, via RT blino @ mandriva. com
<perlbug-followup@perl.org> wrote:

# New Ticket Created by blino@mandriva.com
# Please include the string: [perl #51636]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=51636 >

This is a bug report for perl from blino@mandriva.com,
generated with the help of perlbug 1.36 running under perl 5.10.0.

-----------------------------------------------------------------
[Please enter your report here]
Perl aborts with a segmentation fault when using array tie in array
ties. The following testcase allows to reproduce it.

#!/usr/bin/perl

my $view;
my @a;
tie @a, 'TiedList';
$view->{data} = \@a;
@{$view->{data}} = (1);

package TiedList;

use strict;
use Carp;

sub TIEARRAY {
my $class = shift;
return bless {}, $class;
}

sub FETCH { 0 }

sub STORE { # this, index, value
warn "store";
my @row;
tie @row, 'TiedRow';
warn "stored";
return $_[2];
}
sub FETCHSIZE { croak }
sub PUSH { croak }
sub POP { croak }
sub SHIFT { croak }
sub UNSHIFT { croak }
sub DELETE { croak }
sub CLEAR { [] }
sub EXISTS { croak }
sub EXTEND { warn "extend" }
sub STORESIZE { croak }
sub SPLICE { croak }

1;

package TiedRow;

use strict;
use Carp;

sub TIEARRAY {
my $class = shift;
return bless {}, $class;
}

sub FETCH { 0 }

sub STORE { $_[2] }

sub FETCHSIZE { croak }
sub EXISTS { croak }

sub EXTEND { croak }
sub CLEAR { croak }

sub POP { croak "pop called on a TiedRow, but you can't change its size"; }
sub PUSH { croak "push called on a TiedRow, but you can't change its size"; }
sub SHIFT { croak "shift called on a TiedRow, but you can't change its size"; }
sub UNSHIFT { croak "unshift called on a TiedRow, but you can't change its size"; }
sub SPLICE { croak "splice called on a TiedRow, but you can't change its size"; }
sub DELETE { croak "delete called on a TiedRow, but you can't change its size"; }
sub STORESIZE { carp "STORESIZE operation not supported"; }

1;

The testcase may look awkward, it has been written starting from
Gtk2::SimpleList (which originally triggered the bug).

The bug does not happen with perl 5.8.8, it looks like the bug has been
introduced by change 31770 ("optimize push @ISA").

This naive patch fixes the issue:
diff -p -up perl-5.10.0/av.c.av_mg perl-5.10.0/av.c
--- perl-5.10.0/av.c.av_mg 2007-12-18 11:47:07.000000000 +0100
+++ perl-5.10.0/av.c 2008-03-12 10:40:56.000000000 +0100
@@ -433,7 +433,7 @@ Perl_av_clear(pTHX_ register AV *av)
/* Give any tie a chance to cleanup first */
if (SvRMAGICAL(av)) {
const MAGIC* const mg = SvMAGIC(av);
- if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ if (PL_delaymagic && mg && mg->mg_type == PERL_MAGIC_isa)
PL_delaymagic |= DM_ARRAY;
else
mg_clear((SV*)av);

[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags:
category=core
severity=medium
---
Site configuration information for perl 5.10.0:

Configured by Mandriva at Mon Mar 10 12:15:15 EDT 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
Platform:
osname=linux, osvers=2.6.22.12-server-1mdv, archname=i386-linux-thread-multi
uname='linux n5.mandriva.com 2.6.22.12-server-1mdv #1 smp tue nov 20 09:35:09 est 2007 i686 intel(r) xeon(tm) cpu 2.80ghz gnulinux '
config_args='-des -Dinc_version_list=5.8.8 5.8.7 5.8.6 5.8.5 5.8.4 5.8.3 5.8.2 5.8.1 5.8.0 5.6.1 5.6.0 -Darchname=i386-linux -Dcc=gcc -Doptimize=-O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr -Dsitebin=/usr/local/bin -Dsiteman1dir=/usr/local/share/man/man1 -Dsiteman3dir=/usr/local/share/man/man3 -Dman3ext=3pm -Dcf_by=Mandriva -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=root@localhost -Dd_dosuid -Ud_csh -Duseshrplib -Duseithreads'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
optimize='-O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables',
cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
ccversion='', gccversion='4.2.2 20071128 (prerelease) (4.2.2-2mdv2008.1)', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='gcc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib
libs=-lnsl -lndbm -lgdbm -ldl -lm -lcrypt -lutil -lpthread -lc
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
libc=/lib/libc-2.7.so, so=so, useshrplib=true, libperl=libperl.so
gnulibc_version='2.7'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib/perl5/5.10.0/i386-linux-thread-multi/CORE'
cccdlflags='-fPIC', lddlflags='-shared -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fomit-frame-pointer -march=i586 -mtune=generic -fasynchronous-unwind-tables -L/usr/local/lib'

Locally applied patches:
Mandriva Linux patches

---
@INC for perl 5.10.0:
/usr/lib/perl5/site_perl/5.10.0/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.10.0
/usr/lib/perl5/vendor_perl/5.10.0/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.10.0
/usr/lib/perl5/5.10.0/i386-linux-thread-multi
/usr/lib/perl5/5.10.0
/usr/lib/perl5/site_perl/5.8.7
/usr/lib/perl5/site_perl
/usr/lib/perl5/vendor_perl/5.8.8
/usr/lib/perl5/vendor_perl/5.8.7
/usr/lib/perl5/vendor_perl/5.8.6
/usr/lib/perl5/vendor_perl/5.8.5
/usr/lib/perl5/vendor_perl/5.8.4
/usr/lib/perl5/vendor_perl/5.8.3
/usr/lib/perl5/vendor_perl
.

---
Environment for perl 5.10.0:
HOME=/home/blino
LANG=fr_FR
LANGUAGE=fr_FR:fr
LC_ADDRESS=fr_FR
LC_COLLATE=fr_FR
LC_CTYPE=fr_FR
LC_IDENTIFICATION=fr_FR
LC_MEASUREMENT=fr_FR
LC_MESSAGES=fr_FR
LC_MONETARY=fr_FR
LC_NAME=fr_FR
LC_NUMERIC=fr_FR
LC_PAPER=fr_FR
LC_SOURCED=1
LC_TELEPHONE=fr_FR
LC_TIME=fr_FR
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/bin:/usr/bin:/usr/local/bin:/usr/games:/usr/lib/qt4/bin:/usr/lib/qt4/bin:/home/blino/bin:/usr/lib/qt4/bin
PERL_BADLANG (unset)
SHELL=/bin/bash

p5pRT · 2008-03-12T17:04:17Z

From perl@profvince.com

You do this:

--- pp_hot.c 2008-03-10 18:39:48.000000000 +0100
+++ pp_hot.c 2008-03-12 17:03:58.000000000 +0100
@@ -1020,8 +1020,14 @@
*(relem++) = sv;
didstore = av_store(ary,i++,sv);
if (magic) {
- if (SvSMAGICAL(sv))
+ if (SvSMAGICAL(sv)) {
+ /* More magic can happen in the mg_set callback, so we
+ * backup the delaymagic for now. */
+ U16 dmbak = PL_delaymagic;
+ PL_delaymagic = 0;
mg_set(sv);
+ PL_delaymagic |= dmbak;
+ }
if (!didstore)
sv_2mortal(sv);
}

I'm not sure if I'm suggesting cargo-culting here based on previous
experiences with global variables, but would it be better to do this with
the save stack (um, SAVEI16(), er, isn't quite U16)? (Which I figure is
going to add the expense of an ENTER and LEAVE around the call)

Or if an exception is thrown within the mg_set(), does it propagate up so
far that the temporary state of PL_delaymagic doesn't matter? In this
case,
it's not an allocated value that is able to be leaked.

Yes, we could localize delaymagic, just to be on the safe side. However,
it seems that it's used both rarely and boldly in the rest of code, so it
might not be necessary to do it. Besides that, I really don't know.

p5pRT · 2008-04-26T18:23:52Z

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

p5pRT · 2008-05-02T11:45:42Z

From @rgs

2008/3/12 Vincent Pit <perl@profvince.com>:

- if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ if (PL_delaymagic && mg && mg->mg_type == PERL_MAGIC_isa)

The patch looks good and is obviously safe to apply, but I would like to
understand as well why mg ends up being null whereas SvRMAGICAL(av) is
still true. It might indicate a deeper bug in magic handling.

Not sure where the regression test should go. t/op/tiearray.t ?

The problem is that delaymagic is set. This happens because the second
array is created and deleted in the context of a tied array assignment,
which sets delaymagic to delay its own magic. av_clears thinks that the
magic is delayed for the inner array, while it's for the outer.

The attached patch fixes this behaviour by copying PL_delaymagic before
calling the magic callbacks, and setting it back after (ok with 33493). A
reduced test case is also attached.

Thanks, applied as #33778.

p5pRT · 2010-06-04T20:56:05Z

From @iabyn

A belated followup. See the following commit:

commit 8ef2424
Author: David Mitchell <davem@iabyn.com>
AuthorDate: Fri Jun 4 21:01:43 2010 +0100
Commit: David Mitchell <davem@iabyn.com>
CommitDate: Fri Jun 4 21:01:43 2010 +0100

Revert "Re: [perl #51636] segmentation fault with array ties"

This reverts commit 90630e3.

This fix turns out to be wrong, and also made ($<,$>)=(...) fail
(RT #75212).

The original problem was a SEGV in av_clear(). This was mis-diagnosed
as recursive PL_delaymagic issue, and the fix was to temprarily reset
PL_delaymagic to zero. This stopped the mg_set() of $> and $> being
delayed.

The real problem was that mg_free wasn't clearing the [GSR]MG flags
after freeing xmg_magic. This was independently fixed by commit
68f8932.

p5pRT closed this as completed Apr 26, 2008

p5pRT added Severity Medium distro-Linux type-core labels Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

segmentation fault with array ties #9249

segmentation fault with array ties #9249

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

p5pRT commented Apr 26, 2008

p5pRT commented May 2, 2008

p5pRT commented Jun 4, 2010

segmentation fault with array ties #9249

segmentation fault with array ties #9249

Comments

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

From blino@mandriva.com

Created by blino@mandriva.com

p5pRT commented Mar 12, 2008

From @rgs

p5pRT commented Mar 12, 2008

p5pRT commented Mar 12, 2008

From perl@profvince.com

p5pRT commented Mar 12, 2008

From perl@profvince.com

p5pRT commented Mar 12, 2008

From perl@profvince.com

p5pRT commented Mar 12, 2008

From @nwc10

p5pRT commented Mar 12, 2008

From @smpeters

p5pRT commented Mar 12, 2008

From perl@profvince.com

p5pRT commented Apr 26, 2008

p5pRT commented May 2, 2008

From @rgs

p5pRT commented Jun 4, 2010

From @iabyn