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

Combination of tie() and loop aliasing can cause perl to crash. #8842

Closed
p5pRT opened this issue Mar 21, 2007 · 8 comments
Closed

Combination of tie() and loop aliasing can cause perl to crash. #8842

p5pRT opened this issue Mar 21, 2007 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 21, 2007

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

Searchable as RT41948$

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2007

From mjcarman@mchsi.com

This is a bug report for perl from mjcarman@​mchsi.com,
generated with the help of perlbug 1.35 running under perl v5.8.4.


It's possible to cause a crash in perl by using a combination of tie() and the
implicit aliasing of loop variables. I have observed this on both v5.8.4 and
v5.8.7. The following example reproduces the problem.

#!perl
use strict;
use warnings;

package Death;
sub TIEARRAY { bless [], __PACKAGE__ }
sub FETCH { { a => 1, b => 2} }
sub FETCHSIZE { 1 }

package main;

tie my @​array, 'Death';

foreach my $p (@​array) {
  my %h = (a => $p->{a}, b => $p->{b}); # Aaaghh!
}

__END__

The code in the example isn't useful (obviously). It's a distilled version of
what I was doing when I encountered the problem. The important part is that
FETCH returns a reference to a different anonymous structure each time it's
called.

The crash only occurs when the loop variable appears multiple times in the same
statement, but it depends on the statement​:

%h = (a => $p->{a}, b => $p->{b}); # crashes
%h = (a => $p->{a}); # okay
print $p->{a}, ' ', $p->{b}, "\n"; # crashes
print $p->{a} . ' ' . $p->{b} . "\n"; # okay
print "$p->{a} $p->{b}\n"; # okay
printf("%s %s\n", $p->{a}, $p->{b}); # doesn't crash but $p->{a} is undefined
$p->{a}, $p->{b}; # okay (just void context warnings)
@​a = ($p->{a}, $p->{b}); # crashes
$a = ($p->{a}, $p->{b}); # okay (just void context warnings)

It isn't often that Perl grants me a peek at its juicy innards, but I caught a
glimpse here. I've learned that when the docs say that the loop varaible is an
implicit alias for the element of the list/array, they mean *exactly* that. The
FETCH isn't occuring as part of the loop entry. It's happening each time the
loop variable is used as if I had written "$array[0]" instead of "$p." That
suprised me a little. I would have expected a more direct aliasing that didn't
require a FETCH for each occurance.

From here on I'm speculating, but it appears that *both* fetches are being done
before the hash lookups. Presumably the problem is that the two occurances of $p
have different values but the way the bytecode is organized/optimized it expects
them (not unreasonably) to have the same value. I don't know why this results in
a crash instead of a behavior where both lookups use the second hashref
returned. This could lead to subtle bugs if the contents of the hashes were
supposed to be different, but that's another issue.



Flags​:
  category=core
  severity=low


Site configuration information for perl v5.8.4​:

Configured by ActiveState at Tue Jun 1 11​:52​:09 2004.

Summary of my perl5 (revision 5 version 8 subversion 4) configuration​:
  Platform​:
  osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
  uname=''
  config_args='undef'
  hint=recommended, useposix=true, d_sigaction=undef
  usethreads=undef use5005threads=undef 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='cl', ccflags ='-nologo -Gf -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT -DHAVE_DES_FCRYPT -DNO_HASH_SEED -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX',
  optimize='-MD -Zi -DNDEBUG -O1',
  cppflags='-DWIN32'
  ccversion='', gccversion='', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='link', ldflags ='-nologo -nodefaultlib -debug -opt​:ref,icf -libpath​:"C​:\Perl\lib\CORE" -machine​:x86'
  libpth=C​:\PROGRA1\MICROS3\VC98\lib
  libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib
  perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib
  libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib
  gnulibc_version='undef'
  Dynamic Linking​:
  dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt​:ref,icf -libpath​:"C​:\Perl\lib\CORE" -machine​:x86'

Locally applied patches​:
  ACTIVEPERL_LOCAL_PATCHES_ENTRY
  22751 Update to Test.pm 1.25
  21540 Fix backward-compatibility issues in if.pm


@​INC for perl v5.8.4​:
  C​:/Perl/lib
  C​:/Perl/site/lib
  .


Environment for perl v5.8.4​:
  HOME=U​:\
  LANG (unset)
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=C​:\WINDOWS\system32;C​:\WINDOWS;C​:\WINDOWS\System32\Wbem;C​:\Program Files\Dazel\Output Envoy\bin\;C​:\Program Files\Rational\ClearCase\bin;C​:\Perl\bin;C​:\Mingw\bin;C​:\Tools
  PERL_BADLANG (unset)
  SHELL (unset)

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2007

From @iabyn

On Wed, Mar 21, 2007 at 09​:18​:16AM -0700, mjcarman @​ mchsi. com wrote​:

It's possible to cause a crash in perl by using a combination of tie() and the
implicit aliasing of loop variables. I have observed this on both v5.8.4 and
v5.8.7. The following example reproduces the problem.

#!perl
use strict;
use warnings;

package Death;
sub TIEARRAY { bless [], __PACKAGE__ }
sub FETCH { { a => 1, b => 2} }
sub FETCHSIZE { 1 }

package main;

tie my @​array, 'Death';

foreach my $p (@​array) {
my %h = (a => $p->{a}, b => $p->{b}); # Aaaghh!
}

The fault can be reduced to the following​:

  sub TIEARRAY { bless [] }
  sub FETCH { [ 1 ] }

  tie my @​array, 'main';

  my $p = \$array[0];
  my @​h = ($$p->[0], $$p->[0]);

It's yet another manifestation of the fact that items on the stack aren't
refcounted.

$p is a reference to a proxy object (a PVLV) that calls FETCH
each time its value is accessed. The first access ($$p->[0]) causes $$p to
be set to an RV pointing to the returned [1]; the '1' inside that is
pushed onto the stack.
The second ($$p->[0]) discards the old [1], thus prematurely freeing the
'1' still on the stack.

Actually its slightly more subtle than that; there's a hack in
Perl_sv_unref_flags() that sometimes delays freeing a referent when the
referrer is overwritten, by adding the referent to the tmps stack instead
of immediately freeing it. This allows code like

  $a = $a->[0]

to work (otherwise the value in the array would be freed just after $a
stops being a ref, but just before that now-freed value has been assigned
to $a).

In the case of ties however, the call to FETCH triggers a freetmps which
defeats the attempt to extend the life.

I can't see any simple way of fixing this (short of making the stack
refcounted (ah, a new hobby for 5.12 ????)

Dave

--
Thank God I'm an atheist.....

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2007

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

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2007

From @davidnicol

On 3/21/07, Dave Mitchell <davem@​iabyn.com> wrote​:

Actually its slightly more subtle than that; there's a hack in
Perl_sv_unref_flags() that sometimes delays freeing a referent when the
referrer is overwritten, by adding the referent to the tmps stack instead
of immediately freeing it. This allows code like

$a = $a\->\[0\]

to work (otherwise the value in the array would be freed just after $a
stops being a ref, but just before that now-freed value has been assigned
to $a).

In the case of ties however, the call to FETCH triggers a freetmps which
defeats the attempt to extend the life.

I can't see any simple way of fixing this (short of making the stack
refcounted (ah, a new hobby for 5.12 ????)

Dave

alternative complex proposal​:
What if there was a second tmp heap used for composing lists, or the
tmps were partitioned or flagged somehow, so a freetmps encountered
during composition of a list would not free elements which were so flagged?

Having two tmp zones might be easier than adding flags​: regular freetmps
would clear them all, a weakfreetmps would be introduced that got one
but not the other.

simple proposal with possibly acceptable side effects​:
How long would the leak last if FETCH no longer freed temporaries? Based
on the above, that might be the simplest way to avoid this crash, aside from

nonstarter docpatch proposal​:
registering a highly nuanced and far-reaching "so don't do that" that would
extend to things such as "do not pass pointers to tied containers to methods
that will fetch more than one element from the container within a single
expression"

@p5pRT
Copy link
Author

p5pRT commented Dec 26, 2012

From @bulk88

On Wed Mar 21 09​:18​:15 2007, mjcarman@​mchsi.com wrote​:

This is a bug report for perl from mjcarman@​mchsi.com,
generated with the help of perlbug 1.35 running under perl v5.8.4.

-----------------------------------------------------------------
It's possible to cause a crash in perl by using a combination of tie()
and the
implicit aliasing of loop variables. I have observed this on both
v5.8.4 and
v5.8.7. The following example reproduces the problem.

#!perl
use strict;
use warnings;

package Death;
sub TIEARRAY { bless [], __PACKAGE__ }
sub FETCH { { a => 1, b => 2} }
sub FETCHSIZE { 1 }

package main;

tie my @​array, 'Death';

foreach my $p (@​array) {
my %h = (a => $p->{a}, b => $p->{b}); # Aaaghh!
}

__END__

The following
__________________________________________________________________
use strict;
use warnings;

package Death;
sub TIEARRAY { bless [], __PACKAGE__ }
sub FETCH { { a => 1, b => 2} }
sub FETCHSIZE { 1 }

package main;

tie my @​array, 'Death';

foreach my $p (@​array) {
my %h = (a => $p->{a}, b => $p->{b}); # Aaaghh!
}
________________________________________________________________

with threaded win32 perls,

5.10
__________________________________________________________________
panic​: attempt to copy freed scalar 1839704 to 1839724 at n11.pl line 24.
__________________________________________________________________

5.12
___________________________________________________________________
panic​: attempt to copy freed scalar 83b28c to 829ef4 at n11.pl line 24.
___________________________________________________________________

5.14.2 win64
-empty console (no panic)

5.17.6 win32
-empty console (no panic)

I think this has been fixed by now, last discussion was in 2007, which
was 5.8/5.8/early 5.10 era. I would guess without looking at any code a
savetmps and freetmps around the tied/magic method call would fix the
problem.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Dec 30, 2012

From @iabyn

On Wed, Dec 26, 2012 at 01​:20​:06PM -0800, bulk88 via RT wrote​:

use strict;
use warnings;

package Death;
sub TIEARRAY { bless [], __PACKAGE__ }
sub FETCH { { a => 1, b => 2} }
sub FETCHSIZE { 1 }

package main;

tie my @​array, 'Death';

foreach my $p (@​array) {
my %h = (a => $p->{a}, b => $p->{b}); # Aaaghh!
}
________________________________________________________________

with threaded win32 perls,

5.10
__________________________________________________________________
panic​: attempt to copy freed scalar 1839704 to 1839724 at n11.pl line 24.
__________________________________________________________________

5.12
___________________________________________________________________
panic​: attempt to copy freed scalar 83b28c to 829ef4 at n11.pl line 24.
___________________________________________________________________

5.14.2 win64
-empty console (no panic)

5.17.6 win32
-empty console (no panic)

I think this has been fixed by now.

Bisect shows it was fixed by my fd69380.

last discussion was in 2007, which
was 5.8/5.8/early 5.10 era. I would guess without looking at any code a
savetmps and freetmps around the tied/magic method call would fix the
problem.

If you think the problem's fixed, why do you think the problem needs
fixing???

although I can't remember the details now, I remember changing the tmps
freeing behaviour of magic method calls when I did a big rationalisation
of that code with this​:

commit efaf367
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Sun Apr 25 00​:56​:32 2010 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Sun Apr 25 00​:56​:32 2010 +0100

  add Perl_magic_methcall
 
  Add a new function that wraps the setup needed to call a magic method like
  FETCH (the existing S_magic_methcall function has been renamed
  S_magic_methcall1).
 
  There is one functional change, done mainly to allow for a single clean
  wrapper function, and that is that the method calls are no longer wrapped
  with SAVETMPS/FREETMPS. Previously only about half of them had this, so
  some relied on the caller to free, some didn't. At least we're consistent
  now. Doing it this way is necessary because otherwise magic_methcall()
  can't return an SV (eg for POP) because it'll be a temp and get freed by
  FREETMPS before it gets returned. So you'd have to copy everything, which
  would slow things down.

--
Overhead, without any fuss, the stars were going out.
  -- Arthur C Clarke

@p5pRT
Copy link
Author

p5pRT commented Dec 30, 2012

From @bulk88

On Sun Dec 30 05​:27​:42 2012, davem wrote​:

On Wed, Dec 26, 2012 at 01​:20​:06PM -0800, bulk88 via RT wrote​:

I think this has been fixed by now.

Bisect shows it was fixed by my
fd69380.

Good. Technical reasoning to this bug is now known.

last discussion was in 2007, which
was 5.8/5.8/early 5.10 era. I would guess without looking at any
code a
savetmps and freetmps around the tied/magic method call would fix
the
problem.

If you think the problem's fixed, why do you think the problem needs
fixing???

I never debugged the details on this bug, so I can not 100% claim "it
was fixed". Without an actual code change or design details, to point
out, that was responsible for stopping the panic, it is unknown if the
bug was actually fixed internally or symptoms simply stopped yet the
"bug" remains. I dont use bisect. The symptoms of this bug have stopped
around 2 years ago, yet nobody noticed this report. It is possible the
unknown author of the commit/s that fixed or stopped symptoms of this
bug 2 years ago, is unaware of this report, or is not active in Perl
anymore. So if my post will be the last in this ticket before it is
closed, some kind of guess is better than "not observed with a supported
Perl, rejected" with no other words. Thanks for responding to this ticket.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Jan 1, 2013

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

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