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

eval {} still clobbers $@ in a DESTROY #14473

Closed
p5pRT opened this issue Feb 5, 2015 · 9 comments
Closed

eval {} still clobbers $@ in a DESTROY #14473

p5pRT opened this issue Feb 5, 2015 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 5, 2015

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

Searchable as RT123738$

@p5pRT
Copy link
Author

p5pRT commented Feb 5, 2015

From @FGasper

Created by @FGasper

#!/usr/bin/env perl

package Destroyer;

sub new { bless {} }

sub DESTROY { eval {} }

package main;

eval {
  for ( Destroyer->new() ) {
  die 'haha';
  }
};
print "The error from Perl $^V​: $@​";


The above code demonstrates that the change in Perl 5\.14 that was intended to
ensure that DESTROY handlers don’t clobber $@​ did not account for this case\.

<details><summary>Perl Info</summary>

```

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.14.4:

Configured by cPanel at Tue Dec 16 16:26:36 CST 2014.

Summary of my perl5 (revision 5 version 14 subversion 4) configuration:
   
  Platform:
    osname=linux, osvers=2.6.32-358.23.2.el6.i686, archname=i386-linux-64int
    uname='linux rpmb-32-centos-6 2.6.32-358.23.2.el6.i686 #1 smp wed oct 16 17:21:31 utc 2013 i686 i686 i386 gnulinux '
    config_args='-des -Dusedevel -Darchname=i386-linux-64int -Dcc=/usr/bin/gcc -Dcpp=/usr/bin/cpp -DDEBUGGING=none -Doptimize=-Os -Dusemymalloc=n -Duseshrplib -Duselargefiles=yes -Duseposix=true -Dhint=recommended -Duseperlio=yes -Dccflags=-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -Dcppflags=-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -Dldflags=-Wl,-rpath -Wl,/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -Dprefix=/usr/local/cpanel/3rdparty/perl/514 -Dsiteprefix=/opt/cpanel/perl5/514 -Dsitebin=/opt/cpanel/perl5/514/bin -Dsitelib=/opt/cpanel/perl5/514/site_lib -Dusevendorprefix=true -Dvendorbin=/usr/local/cpanel/3rdparty/perl/514/bin -Dvendorprefix=/usr/local/cpanel/3rdparty/perl/514/lib/perl5 -Dvendorlib=/usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib -Dprivlib=/usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4 -Dman1dir=none -Dman3dir=none -Dscriptdir=/usr/local/cpanel/3rdparty/perl/514/bin -Dscriptdirexp=/usr/local/cpanel/3rdparty/perl/514/bin -Dsiteman1dir=none -Dsiteman3dir=none -Dinstallman1dir=none -Dversiononly=no -Dinstallusrbinperl=no -Dcf_by=cPanel -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=support@cpanel.net -Di_dbm=/usr/local/cpanel/3rdparty/include -Di_gdbm=/usr/local/cpanel/3rdparty/include -Di_ndbm=/usr/local/cpanel/3rdparty/include -Ud_dosuid -Uuserelocatableinc -Umad -Uusethreads -Uusemultiplicity -Uusesocks -Uuselongdouble -Ui_db -Aldflags=-L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm -Dlocincpth=/usr/local/cpanel/3rdparty/perl/514/include /usr/local/cpanel/3rdparty/include /usr/local/include  -Duse64bitint -Uuse64bitall -Acflags=-fPIC -DPIC -m32 -I/usr/local/cpanel/3rdparty/perl/514/include -I/usr/local/cpanel/3rdparty/include -Dlibpth=/usr/local/cpanel/3rdparty/perl/514/lib /usr/local/cpanel/3rdparty/lib /usr/local/lib /lib /usr/lib '
    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='/usr/bin/gcc', ccflags ='-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-Os',
    cppflags='-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.7 20120313 (Red Hat 4.4.7-3)', 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='/usr/bin/gcc', ldflags ='-Wl,-rpath -Wl,/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.12.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.12'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4/i386-linux-64int/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -Os -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -L/usr/local/lib -fstack-protector'

Locally applied patches:
    cPanel patches
    cPanel INC path changes
    Disabled some unstable tests on a kvm build server
    Cherry pick of 49498ca from p5p (RT 113514)
    Remove improper use of each() in B::walksymtable (5cc8528c90)


@INC for perl 5.14.4:
    /usr/local/cpanel
    /usr/local/cpanel
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4
    /opt/cpanel/perl5/514/site_lib/i386-linux-64int
    /opt/cpanel/perl5/514/site_lib
    .


Environment for perl 5.14.4:
    HOME=/root
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/root/.opt/bin:/usr/local/cpanel/3rdparty/perl/514/bin:/usr/local/cpanel/3rdparty/bin:/usr/local/cpanel/3rdparty/node/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
    PERL5LIB=/usr/local/cpanel
    PERL_BADLANG (unset)
    SHELL=/bin/bash

```
</details>

@p5pRT
Copy link
Author

p5pRT commented Feb 5, 2015

From zefram@fysh.org

felipe@​felipegasper.com wrote​:

The above code demonstrates that the change in Perl 5.14 that was intended to
ensure that DESTROY handlers don't clobber $@​ did not account for this case.

Interesting. This is specific to the "for(Destroyer->new)" formulation;
if that's replaced with "my $x = Destroyer-&gt;new;" then $@​ comes
out correctly. The inner eval{} does not need to be an eval​: "$@​ =
'wibble'" works just as well to clobber $@​.

It is presumably a matter of order of destruction. The output suggests
that the localised $_ is being destroyed after the outer eval thinks
it's finished unwinding. I don't immediately see how this happens, but
then I wouldn't, because the outer eval's idea of "finished unwinding"
is the embodiment of mine. It was certainly my intent, in coding the
5.14 change, that code like this should see the "haha" exception in $@​
immediately after the outer eval.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Feb 5, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2015

From @ap

* Zefram <zefram@​fysh.org> [2015-02-05 12​:45]​:

This is specific to the "for(Destroyer->new)" formulation; if that's
replaced with "my $x = Destroyer-&gt;new;" then $@​ comes out correctly.
The inner eval{} does not need to be an eval​: "$@​ = 'wibble'" works
just as well to clobber $@​.

The output suggests that the localised $_ is being destroyed after the
outer eval thinks it's finished unwinding. I don't immediately see how
this happens, but then I wouldn't, because the outer eval's idea of
"finished unwinding" is the embodiment of mine. It was certainly my
intent, in coding the 5.14 change, that code like this should see the
"haha" exception in $@​ immediately after the outer eval.

With

  #!/usr/bin/env perl
  package Destroyer {
  sub new { bless {} }
  sub DESTROY { warn 'in destroy'; eval {} }
  }
  warn eval {
  for ( Destroyer->new() ) {
  die 'haha';
  }
  };
  warn 'after eval​: ', $@​;

I get this output​:

  $ perl t.pl
  haha at t.pl line 8.
  ...caught at t.pl line 6.
  in destroy at t.pl line 4.
  after eval​: at t.pl line 11.

Note how the `warn` around the `eval` actually picks up the inside exception.

You can change this to the statement modifier form of `for` and you can change
it to an explicitly loop iterator variable, both package global (`for our $x`)
and lexical (`for my $x`) and reproduce the bug anyway.

So I would guess the bug is due to `foreach` aliasing – specifically that the
per-iteration aliasing is normally undone at the end of the iteration but that
that is preempted by the `die` here, such that the unaliasing winds up delayed
until all the way after the statement. So the `warn` *inside* that statement
picks up the right exception (and would do so even without the anti-clobber fix
if my guess is right), but then the unaliasing catches up so that by the *next*
statement the exception is gone.

This readily suggests a workaround for running under existing perls​: if you
change the `warn eval` line to

  eval { ... }, my $e = $@​;

and then add a line

  warn 'from same statement​: ', $e;

then this extra line appears in the output​:

  from same statement​: haha at t.pl line 8.

So something like Try​::Tiny can work around this issue already, at least.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2015

From @ap

* Aristotle Pagaltzis <pagaltzis@​gmx.de> [2015-02-06 11​:00]​:

So I would guess the bug is due to `foreach` aliasing – specifically
that the per-iteration aliasing is normally undone at the end of the
iteration but that that is preempted by the `die` here, such that the
unaliasing winds up delayed until all the way after the statement. So
the `warn` *inside* that statement picks up the right exception (and
would do so even without the anti-clobber fix if my guess is right),
but then the unaliasing catches up so that by the *next* statement the
exception is gone.

OTOH – a `warn $_` during the same statement doesn’t see the Destroyer
instance, so the loop iterator variable is already unaliased at that
point, meaning it can’t be the unaliasing as such. But my guess is that
it still has to have to do with some aspect of how aliasing works (such
as how it interacts with the refcount, or whatever – I don’t know enough
perlguts to take a shot), since the bug is not dependent on extra scopes
(statement modifier `for` will do just fine) but does require `for` (any
regularly assigned variable will not exhibit this behaviour).

--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Jun 5, 2017

From @iabyn

On Wed, Feb 04, 2015 at 09​:51​:23PM -0800, felipe@​felipegasper.com wrote​:

# New Ticket Created by felipe@​felipegasper.com
# Please include the string​: [perl #123738]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=123738 >

This is a bug report for perl from felipe@​felipegasper.com,
generated with the help of perlbug 1.39 running under perl 5.14.4.

-----------------------------------------------------------------
[Please describe your issue here]

#!/usr/bin/env perl

package Destroyer;

sub new { bless {} }

sub DESTROY { eval {} }

package main;

eval {
for ( Destroyer->new() ) {
die 'haha';
}
};
print "The error from Perl $^V​: $@​";


The above code demonstrates that the change in Perl 5\.14 that was intended to
ensure that DESTROY handlers don’t clobber $@&#8203; did not account for this case\.

Now fixed v5.27.0-119-gb66d79a

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

@p5pRT
Copy link
Author

p5pRT commented Jun 5, 2017

@iabyn - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0, this and 185 other issues have been
resolved.

Perl 5.28.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

@khwilliamson - Status changed from 'pending release' 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