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

New lvalue subroutine roadblock in 5.10.1 breaks important functionality #10016

Closed
p5pRT opened this issue Dec 10, 2009 · 35 comments
Closed

New lvalue subroutine roadblock in 5.10.1 breaks important functionality #10016

p5pRT opened this issue Dec 10, 2009 · 35 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 10, 2009

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

Searchable as RT71172$

@p5pRT
Copy link
Author

p5pRT commented Dec 10, 2009

From deforest@boulder.swri.edu

Perl Data Language exploits what is normally a non-harmful, non-useful
case in Perl 5; a roadblock added in Perl 5.10.1 kills that case,
gutting PDL functionality.

The problem is that structured array slicing in PDL is implemented using
lvalue subroutines. Assigning to an array slice requires putting the
lvalue subroutine on, well, the left side of an assignment operator.

A frequent construction is​:

  $pdl_a->slice($slice_spec) .= $pdl_b;

where .= is overloaded to do elementwise assignment (copying), and
PDL​::slice() is an lvalue subroutine. Here, the return value from
slice()
is a temporary reference into a slice of the array in $pdl_a.

Under perl-5.10.1 that construction dies with the error message

  Can't return a temporary from lvalue subroutine

which is arbitrary and breaks virtually every PDL script every written.
Note that slicing is not the only operation affected​: many PDL methods
return temporary lvalues. All those methods break under this bug.

The roadblock was apparently put in by some well-meaning person who
noticed
a perceived useless construction, and tried to block it in the name of
finding
non-useful edge cases. The problem is that the construction is
actually useful.

Arguably the roadblock is not helpful because temporary lvalue
assignment is,
at worst, an expensive no-op. It might be useful as a warning under
some
variant of "use strict", but modules which use that functionality should
be able to switch off the warning.

Perl Info

Flags:
     category=core
     severity=critical


@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

From @tsee

Hi Craig,

craig deforest wrote​:

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

Perl Data Language exploits what is normally a non-harmful, non-useful
case in Perl 5; a roadblock added in Perl 5.10.1 kills that case,
gutting PDL functionality.

That's umm... severe.

I know it's not helping at all, but I must say I wonder why NONE of the
PDL users have tried any of the 5.10.1 release candidates?

The problem is that structured array slicing in PDL is implemented using
lvalue subroutines. Assigning to an array slice requires putting the
lvalue subroutine on, well, the left side of an assignment operator.

A frequent construction is​:

$pdl_a->slice($slice_spec) .= $pdl_b;

where .= is overloaded to do elementwise assignment (copying), and
PDL​::slice() is an lvalue subroutine. Here, the return value from
slice()
is a temporary reference into a slice of the array in $pdl_a.

Under perl-5.10.1 that construction dies with the error message

Can't return a temporary from lvalue subroutine

which is arbitrary and breaks virtually every PDL script every written.
Note that slicing is not the only operation affected​: many PDL methods
return temporary lvalues. All those methods break under this bug.

The roadblock was apparently put in by some well-meaning person who
noticed
a perceived useless construction, and tried to block it in the name of
finding
non-useful edge cases. The problem is that the construction is
actually useful.

Arguably the roadblock is not helpful because temporary lvalue
assignment is,
at worst, an expensive no-op. It might be useful as a warning under
some
variant of "use strict", but modules which use that functionality should
be able to switch off the warning.

I suspect it's potentially more than an expensive no-op and that the
code was changed to address a bug. But that's just my hunch.

The curious bit (for me anyway) is that the relevant lines appear to be
in place since 2001/2, i.e. way before 5.10.0. Apparently the code paths
have changed in a way that's not obvious to my untrained eye.

Best regards,
Steffen

PS​: What I am referring to is​:

e9f19e3 (Hugo van der Sanden 2002-11-03 16​:41​:24 +0000 2561)
  DIE(aTHX_ "Can't return %s from lvalue subroutine",
e9f19e3 (Hugo van der Sanden 2002-11-03 16​:41​:24 +0000 2562)
  SvREADONLY(TOPs) ? (TOPs == &PL_sv_undef) ? "undef"
e9f19e3 (Hugo van der Sanden 2002-11-03 16​:41​:24 +0000 2563)
  : "a readonly value" : "a temporary");

and

f206cdd (Abhijit Menon-Sen 2001-06-17 18​:34​:46 +0530 2594)
  DIE(aTHX_ "Can't return a %s from lvalue subroutine",
f206cdd (Abhijit Menon-Sen 2001-06-17 18​:34​:46 +0530 2595)
  SvREADONLY(TOPs) ? "readonly value" : "temporary");

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

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

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

From p5p@perl.wizbit.be

Citeren craig deforest <perlbug-followup@​perl.org>​:

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

Perl Data Language exploits what is normally a non-harmful, non-useful
case in Perl 5; a roadblock added in Perl 5.10.1 kills that case,
gutting PDL functionality.

Is it added in perl 5.10.1? Or was it added in 5.10.0? Or ...?
How did you determine when it was added? Read​: what commit added it
and how did you determine that commit made it in 5.10.1 and not in
5.10.0?

The problem is that structured array slicing in PDL is implemented using
lvalue subroutines. Assigning to an array slice requires putting the
lvalue subroutine on, well, the left side of an assignment operator.

A frequent construction is​:

$pdl_a->slice($slice_spec) .= $pdl_b;

where .= is overloaded to do elementwise assignment (copying), and
PDL​::slice() is an lvalue subroutine. Here, the return value from
slice()
is a temporary reference into a slice of the array in $pdl_a.

If you understand what is happening then please add a test case.
A test case using PDL would be a start, a test case that uses core
only modules would be better.

Under perl-5.10.1 that construction dies with the error message

Can't return a temporary from lvalue subroutine

which is arbitrary and breaks virtually every PDL script every written.

Then why does cpantesters shows PASSes on
http​://matrix.cpantesters.org/?dist=PDL+2.4.5 ? It clearly does not
break every script written unless PDL has virtually no test case.

The roadblock was apparently put in by some well-meaning person who
noticed
a perceived useless construction, and tried to block it in the name of
finding
non-useful edge cases. The problem is that the construction is
actually useful.

How did you determine this? To what commit are you refering?

Arguably the roadblock is not helpful because temporary lvalue
assignment is,
at worst, an expensive no-op.

How did you determine this?

If there was a test case then it would be easy to find the answers to
these questions.... but now someone has to spend time trying to write
a test case, trying to make it fail and then trying to fix it (if it
needs fixing).

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

From @tsee

Hi Craig,

craig deforest wrote​:

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

Perl Data Language exploits what is normally a non-harmful, non-useful
case in Perl 5; a roadblock added in Perl 5.10.1 kills that case,
gutting PDL functionality.

PDL tests run successfully on a freshly installed 5.10.1 on my ubuntu 9.04.

Cheers,
Steffen

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

From @nwc10

On Fri, Dec 11, 2009 at 10​:54​:46AM +0100, Steffen Mueller wrote​:

Hi Craig,

craig deforest wrote​:

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

Perl Data Language exploits what is normally a non-harmful, non-useful
case in Perl 5; a roadblock added in Perl 5.10.1 kills that case,
gutting PDL functionality.

PDL tests run successfully on a freshly installed 5.10.1 on my ubuntu 9.04.

Your test report doesn't include the output of perl -V from your platform.
Please could you include that, as it seems that it will reveal why it's
proving hard to replicate this problem.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Dec 11, 2009

From @ikegami

On Thu, Dec 10, 2009 at 11​:47 AM, craig deforest
<perlbug-followup@​perl.org>wrote​:

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

Arguably the roadblock is not helpful because temporary lvalue

assignment is, at worst, an expensive no-op.

The roadblock was apparently put in by some well-meaning person who

noticed a perceived useless construction, and tried to block it in the name
of
finding non-useful edge cases. The problem is that the construction is
actually useful.

I've looked at this area of the code working on [perl #67838]. My solution
for that bug requires the return a temp from an lvalue sub. My solution was
to relax the message to allow the return of TEMPs with SMG. It won't help
with returning overloaded objects.

The message is designed to prevent coding mistakes. Perl itself doesn't have
any problems with TEMPs being returned from lvalue subs.

But I found no recent changes in that area. I had looked into when the check
was added (to see why it was added), and it was ancient.

IIRC, the message is only triggered when the last expression of a Pure Perl
lvalue sub is a call to an XS function that returns a TEMP.

That presents itself with a workaround​: Make the PDL's slice func an XS sub.

- Eric

PS -- Here's my fix to pp_leavesublv for allowing the returns of SMG TEMPs​:

  if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
- !(SvRMAGICAL(TOPs) && mg_find(TOPs, PERL_MAGIC_tiedelem))) {
+ !SvSMAGICAL(TOPs)) {

@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2009

From @ikegami

On Thu, Dec 10, 2009 at 11​:47 AM, craig deforest
<perlbug-followup@​perl.org>wrote​:

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

[...]

A frequent construction is​:

$pdl_a->slice($slice_spec) .= $pdl_b;

[...]

Under perl-5.10.1 that construction dies with the error message

Can't return a temporary from lvalue subroutine

I cannot reproduce it. Could you give a simple program that produces that
error?

use PDL;
my $pdl_a = ones(5,5);
my $pdl_b = zeroes(5);
print($pdl_a);
print($pdl_b);
$pdl_a->slice('​:,(2)') .= $pdl_b;
print($pdl_a);

perl a.pl

[
[1 1 1 1 1]
[1 1 1 1 1]
[1 1 1 1 1]
[1 1 1 1 1]
[1 1 1 1 1]
]
[0 0 0 0 0]
[
[1 1 1 1 1]
[1 1 1 1 1]
[0 0 0 0 0]
[1 1 1 1 1]
[1 1 1 1 1]
]

perl -v

This is perl, v5.10.1 built for MSWin32-x86-multi-thread
(with 2 registered patches, see perl -V for more detail)

Copyright 1987-2009, Larry Wall

Binary build 1006 [291086] provided by ActiveState
http​://www.ActiveState.com
Built Aug 24 2009 13​:48​:26

[...]

perl -MPDL -E"say $PDL​::VERSION"
2.4.5

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2010

From dcmertens@gmail.com

Hello everybody -

It seems that an important detail from the original PDL mailing list
didn't make it to Craig's posting of the problem. It is not a problem
with Perl 5.10.1 per se, but with the Perl 5.10.1 _debugger_. My
investigations seem to indicate that it's a problem particular to PDL.
For a simple example of this failing when using PDL, enter this on the
command line​:

perl -d -MPDL -e'$a=sequence(9); $a->slice("1") .= -9; print $a, "\n"'

DB<1> c
Can't return a temporary from lvalue subroutine at -e line 1,
  <DATA> line 206.
at -e line 1

Like I said, if you run this on a plain-old perl lvalue sub, it doesn't
give trouble. For example (modified from perlsub)​:

perl -d -e 'my $val; sub canmod : lvalue {$val;} canmod() = 5; print
"$val\n"'

DB<1> c
Debugged program terminated...

So it looks like there's something fishy going on in PDL's lvalue
returns, perhaps?

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2010

From @doughunt

Hi all​: After a bit more investigation, I think I've figured out what
is going on.

First of all, Craig did not note in his initial bug report that this
problem appears only when using the debugger under 5.10.1--there is no
problem when not using the debugger.

Here is a simple test case​:

perl -MPDL -e '$p=zeroes(9); $p->slice("1") .= 9; print $p'
[0 9 0 0 0 0 0 0 0]

This works fine under 5.10.0 with and without the debugger, but under
5.10.1 it breaks​:


[dhunt@​arcana ~]$ perl -d -MPDL -e '$p=zeroes(9); $p->slice("1") .= 9;
print $p'

main​::(-e​:1)​: $p=zeroes(9); $p->slice("1") .= 9; print $p
  DB<1> c
Can't return a temporary from lvalue subroutine at -e line 1.
at -e line 1


I think the problem is with setting the 'lvalue' attribute using the
'attributes' pragma under 5.10.1.

This fails under 5.10.1 when using the debugger, but succeeds under
5.10.0 and 5.10.1 with no debug flag​:


sub canmod { $val }
use attributes __PACKAGE__ => \&canmod, "lvalue";
canmod() = 5;


This, however, always works​:


sub canmod : lvalue { $val }
canmod() = 5;


The 'attributes' pragma is used by PDL to mark the 'slice' routine as
having the 'lvalue' attribute.

So, there has been a change for the worse from perl 5.10.0 to 5.10.1 in
how the 'use attributes' pragma works for setting the lvalue attribute.

Regards,

  Doug Hunt

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2010

From @doughunt

Hi again​: I trying to find what is different between perl 5.10.1 and
5.10.0.

When I compile perl 5.10.1 with -DEBUGGING=both and run it though GDB​:

gdb /opstest/tools/bin/perl

(gdb) b pp_hot.c​:2509
(gdb) run -d -MPDL​::Lite -e'$p=PDL->zeroes(9); $p->slice("1") .= 9;
print $p'

It fails as expected in leavesublv.

But when I do the same thing for perl 5.10.0, it completes successfully
but it does not even stop in leavesublv! I was expecting it at least to
enter this routine and then avoid the error message, but it never even
gets there.

Just another data point...

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2010

From @doughunt

Hi all​: I've spent more time debugging this problem and have attached a
patch (including a new test case). Here is what I understand currently
of the problem​:

--It appears to have been caused by the patch applied in perl 5.10.1 for
bug 7013 ('lvalue subs not working inside debugger')

--It seems that the lvalue attribute when applied to a subroutine​:

  sub canmod : lvalue

works properly under the debugger, but when applied using
'attributes.pm' it does not work​:

use attributes __PACKAGE__ => \&canmod, "lvalue";

--The patch I attach does not address the question of how the lvalue
attribute is assigned, but simply changes pp_hot.c​:pp_leavesublv to
allow the SV on the top of the stack to have the

sv.h​:#define SVs_TEMP 0x00080000 /* string is stealable? */

flag set without generating the error​:

Can't return a temporary from lvalue subroutine

This patch fixes the problem the PDL folks see with their lvalue
subroutines and also passes the perl test suite. I have added a new
test to perl5db.t to check for this problem.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable
folks should examine this change closely.

I'm going to be using this patch for our local production machines, so I
should know shortly if it has any negative side-effects not checked for
in the perl test suite.

Best Regards,

  Doug Hunt

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2010

From @doughunt

lvalue_71172.patch
diff -r -u -N perl-5.10.1.orig/lib/perl5db/t/lvalue-bug1 perl-5.10.1/lib/perl5db/t/lvalue-bug1
--- perl-5.10.1.orig/lib/perl5db/t/lvalue-bug1	1969-12-31 17:00:00.000000000 -0700
+++ perl-5.10.1/lib/perl5db/t/lvalue-bug1	2010-02-08 10:46:11.000000000 -0700
@@ -0,0 +1,12 @@
+#!/usr/bin/perl
+#
+# This code is used by lib/perl5db.t !!!
+#
+
+# Perl bug 71172.  Test that 'use attributes' works correctly to
+# set a subroutine as an lvalue under the perl debugger.
+
+sub canmod { $val }
+use attributes __PACKAGE__ => \&canmod, "lvalue";
+canmod() = 5;
+print "No error\n";
diff -r -u -N perl-5.10.1.orig/lib/perl5db.t perl-5.10.1/lib/perl5db.t
--- perl-5.10.1.orig/lib/perl5db.t	2009-07-25 16:37:19.000000000 -0600
+++ perl-5.10.1/lib/perl5db.t	2010-02-08 10:58:05.990615948 -0700
@@ -26,7 +26,7 @@
     }
 }
 
-plan(2);
+plan(3);
 
 sub rc {
     open RC, ">", ".perldb" or die $!;
@@ -82,6 +82,14 @@
     like($output, qr/foo is defined/, 'lvalue subs work in the debugger');
 }
 
+# Test for lvalue problem under debugger:  perl bug 71172.  Make sure 'attributes.pm'
+# can be used to set a subroutine as an lvalue when used under the debugger.
+{
+    local $ENV{PERLDB_OPTS} = "ReadLine=0";
+    my $output = runperl(switches => [ '-d' ], progfile => '../lib/perl5db/t/lvalue-bug1');
+    like($output, qr/No error/, 'lvalue subs work in the debugger (test 2)');
+}
+
 # clean up.
 
 END {
diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c	2009-04-19 16:28:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c	2010-02-08 10:58:05.993615795 -0700
@@ -2559,7 +2559,7 @@
 	    if (MARK == SP) {
 		/* Temporaries are bad unless they happen to be elements
 		 * of a tied hash or array */
-		if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
+		if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
 		    !(SvRMAGICAL(TOPs) && mg_find(TOPs, PERL_MAGIC_tiedelem))) {
 		    LEAVE;
 		    cxstack_ix--;

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @rgarcia

On 8 February 2010 21​:56, Doug Hunt via RT <perlbug-followup@​perl.org> wrote​:

Hi all​:  I've spent more time debugging this problem and have attached a
patch (including a new test case).  Here is what I understand currently
of the problem​:

--It appears to have been caused by the patch applied in perl 5.10.1 for
bug 7013 ('lvalue subs not working inside debugger')

But that patch only affects code run in the debugger or with some kind
of debugging enabled, right ?

--It seems that the lvalue attribute when applied to a subroutine​:

 sub canmod : lvalue

works properly under the debugger, but when applied using
'attributes.pm' it does not work​:

use attributes __PACKAGE__ => \&canmod, "lvalue";

It would be helpful to have a reproducible test case for this, that
does not involve the debugger, if possible.

--The patch I attach does not address the question of how the lvalue
attribute is assigned, but simply changes pp_hot.c​:pp_leavesublv to
allow the SV on the top of the stack to have the

sv.h​:#define SVs_TEMP   0x00080000  /* string is stealable? */

flag set without generating the error​:

Can't return a temporary from lvalue subroutine

This patch fixes the problem the PDL folks see with their lvalue
subroutines and also passes the perl test suite.  I have added a new
test to perl5db.t to check for this problem.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable
folks should examine this change closely.

I'll have to look at that more closely. However I note at 1st glance
that the effect of this patch is _not_ limited to the debugger.

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From gerard@ggoossen.net

The lvalue attribute can't be added to an already existing subroutine.
Adding "lvalue" using "sub canmod : lvalue" after "canmod" is already defined
has recently been patched to not set the lvalue flag and give a warning.
The "attributes" module must be patched in a similar way.

Gerard Goossen.

On Mon, Feb 08, 2010 at 12​:56​:57PM -0800, Doug Hunt via RT wrote​:

Hi all​: I've spent more time debugging this problem and have attached a
patch (including a new test case). Here is what I understand currently
of the problem​:

--It appears to have been caused by the patch applied in perl 5.10.1 for
bug 7013 ('lvalue subs not working inside debugger')

--It seems that the lvalue attribute when applied to a subroutine​:

sub canmod : lvalue

works properly under the debugger, but when applied using
'attributes.pm' it does not work​:

use attributes __PACKAGE__ => \&canmod, "lvalue";

--The patch I attach does not address the question of how the lvalue
attribute is assigned, but simply changes pp_hot.c​:pp_leavesublv to
allow the SV on the top of the stack to have the

sv.h​:#define SVs_TEMP 0x00080000 /* string is stealable? */

flag set without generating the error​:

Can't return a temporary from lvalue subroutine

This patch fixes the problem the PDL folks see with their lvalue
subroutines and also passes the perl test suite. I have added a new
test to perl5db.t to check for this problem.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable
folks should examine this change closely.

I'm going to be using this patch for our local production machines, so I
should know shortly if it has any negative side-effects not checked for
in the perl test suite.

Best Regards,

Doug Hunt

diff -r -u -N perl-5.10.1.orig/lib/perl5db/t/lvalue-bug1 perl-5.10.1/lib/perl5db/t/lvalue-bug1
--- perl-5.10.1.orig/lib/perl5db/t/lvalue-bug1 1969-12-31 17​:00​:00.000000000 -0700
+++ perl-5.10.1/lib/perl5db/t/lvalue-bug1 2010-02-08 10​:46​:11.000000000 -0700
@​@​ -0,0 +1,12 @​@​
+#!/usr/bin/perl
+#
+# This code is used by lib/perl5db.t !!!
+#
+
+# Perl bug 71172. Test that 'use attributes' works correctly to
+# set a subroutine as an lvalue under the perl debugger.
+
+sub canmod { $val }
+use attributes __PACKAGE__ => \&canmod, "lvalue";
+canmod() = 5;
+print "No error\n";
diff -r -u -N perl-5.10.1.orig/lib/perl5db.t perl-5.10.1/lib/perl5db.t
--- perl-5.10.1.orig/lib/perl5db.t 2009-07-25 16​:37​:19.000000000 -0600
+++ perl-5.10.1/lib/perl5db.t 2010-02-08 10​:58​:05.990615948 -0700
@​@​ -26,7 +26,7 @​@​
}
}

-plan(2);
+plan(3);

sub rc {
open RC, ">", ".perldb" or die $!;
@​@​ -82,6 +82,14 @​@​
like($output, qr/foo is defined/, 'lvalue subs work in the debugger');
}

+# Test for lvalue problem under debugger​: perl bug 71172. Make sure 'attributes.pm'
+# can be used to set a subroutine as an lvalue when used under the debugger.
+{
+ local $ENV{PERLDB_OPTS} = "ReadLine=0";
+ my $output = runperl(switches => [ '-d' ], progfile => '../lib/perl5db/t/lvalue-bug1');
+ like($output, qr/No error/, 'lvalue subs work in the debugger (test 2)');
+}
+
# clean up.

END {
diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c 2009-04-19 16​:28​:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c 2010-02-08 10​:58​:05.993615795 -0700
@​@​ -2559,7 +2559,7 @​@​
if (MARK == SP) {
/* Temporaries are bad unless they happen to be elements
* of a tied hash or array */
- if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
+ if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
!(SvRMAGICAL(TOPs) && mg_find(TOPs, PERL_MAGIC_tiedelem))) {
LEAVE;
cxstack_ix--;

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @ikegami

On Mon, Feb 8, 2010 at 3​:56 PM, Doug Hunt via RT
<perlbug-followup@​perl.org>wrote​:

diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c 2009-04-19 16​:28​:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c 2010-02-08 10​:58​:05.993615795 -0700
@​@​ -2559,7 +2559,7 @​@​
if (MARK == SP) {
/* Temporaries are bad unless they happen to be elements
* of a tied hash or array */
- if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
+ if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
!(SvRMAGICAL(TOPs) && mg_find(TOPs, PERL_MAGIC_tiedelem))) {
LEAVE;
cxstack_ix--;

I disagree with this fix. That check was intentional. It's meant to detect
lvalue subs that return non-lvalues.

It's hard to test since only lvalue XS functions can trigger that check.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable

folks should examine this change closely.

It is set on mortals. When this flag is set, the SV's string can be stolen
instead.

sub f { $x } $y = f(); # $x's string buffer must be copied into $y
sub f { uc($x) } $y = f(); # uc creates a TEMP whose buffer can be stolen

It's being used as an indicator to detect the return of non-values by a
lvalue sub.

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @doughunt

Hi Rafael​: Thanks for looking into this.

The patch for bug 7013 added lvalue support to debugged subroutines via
the DB​::lsub wrapper and a change to pp_hot.c that calls DB​::lsub
instead of DB​::sub. Before this, lvalue subs did not seem to be
correctly handled in the debugger, but at least one could assign values
to them without generating the error message​:

  Can't return a temporary from lvalue subroutine

After reading Eric Brine's response (above) that​:

"The message is designed to prevent coding mistakes. Perl itself doesn't
have any problems with TEMPs being returned from lvalue subs."

and after being unable to determine why the SVs_TEMP flag on the stack
top would be set (causing the error message after the check at
pp_hot.c​:2562) when you run this script under the debugger​:

  sub canmod { $val }
  use attributes __PACKAGE__ => \&canmod, "lvalue";
  canmod() = 5;

but not when you run this script​:

  sub canmod : lvalue { $val }
  canmod() = 5;

I decided to take the more direct approach and take SVs_TEMP out of the
check in pp_hot.c​:pp_leavesublv at line 2562.

One odd thing is that when running normally (not under the debugger),
the first script never executes pp_leavesublv, just pp_leavesub.

So, I have these four cases​:

script1, no debugger​: executes pp_leavesub, not pp_leavesublv so
  never hits message.
script2, no debugger​: exucutes pp_leavesublv, but does not hit message
  because SVs_TEMP is not set.
script1, debugger​: executes pp_leavesublv, hits message *bug*
script2, debugger​: executes pp_leavesublv, SVs_TEMP not set.

So there are perhaps 2 problems​:

1) Setting 'lvalue' via attributes.pm does not seem to work properly
when not running the debugger.

2) SVs_TEMP is being set (perhaps inappropriately) for the stack top
when returning from DB​::lsub

My patch took the simpler tack of just not worrying about SVs_TEMP.
This patched perl passes the test suite as well as the new test I put
in. I have also installed this patched perl in our large satellite data
processing processing facility and run regression tests with no ill effects.

My fix might not get at the root of the problem, but it is simple and
effective and does not seem to cause any problems.

Hope this all makes sense!

Regards,

  Doug Hunt

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @doughunt

Thanks, Gerard​:

On Wed Feb 10 09​:28​:44 2010, ggoossen wrote​:

The lvalue attribute can't be added to an already existing subroutine.
Adding "lvalue" using "sub canmod : lvalue" after "canmod" is already
defined
has recently been patched to not set the lvalue flag and give a
warning.
The "attributes" module must be patched in a similar way.

Gerard Goossen.

This is interesting and news to me. The PDL folks use attributes.pm to
assign lvalue-ness to all it's lvalue subs.
Many of these are XS routines. So, I guess my question is​: How does
one add the lvalue attribute to an XS routine? Without some way of
assigning the lvalue attribute to XS routines, PDL is broken.

Regards,

  Doug Hunt

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2010

From @smpeters

On Wed, Feb 10, 2010 at 1​:36 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

On Mon, Feb 8, 2010 at 3​:56 PM, Doug Hunt via RT
<perlbug-followup@​perl.org>wrote​:

diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c    2009-04-19 16​:28​:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c    2010-02-08 10​:58​:05.993615795 -0700
@​@​ -2559,7 +2559,7 @​@​
         if (MARK == SP) {
         /* Temporaries are bad unless they happen to be elements
          * of a tied hash or array */
-        if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
+        if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
             !(SvRMAGICAL(TOPs) && mg_find(TOPs, PERL_MAGIC_tiedelem))) {
             LEAVE;
             cxstack_ix--;

I disagree with this fix. That check was intentional. It's meant to detect
lvalue subs that return non-lvalues.

It's hard to test since only lvalue XS functions can trigger that check.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable

folks should examine this change closely.

It is set on mortals. When this flag is set, the SV's string can be stolen
instead.

sub f { $x     } $y = f();  # $x's string buffer must be copied into $y
sub f { uc($x) } $y = f();  # uc creates a TEMP whose buffer can be stolen

It's being used as an indicator to detect the return of non-values by a
lvalue sub.

Agreed. That is not even the code that was changed for that fix. For
reference, please see the pp_hot.c changes mentioned at
http​://perl5.git.perl.org/perl.git/commitdiff/1ad62f6

Steve

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2010

From @doughunt

Hi all​: OK, so if getting rid of the SVs_TEMP check in
pp_hot.c​:pp_leavesublv is not the right answer, that still leaves PDL
broken by a change between 5.10.0 and 5.10.1.

PDL uses 'attributes.pm' to set the lvalue attribute on its subs (XS and
otherwise). If it is true that as Gerard Goossen says one cannot assign
the lvalue attribute to an already defined subroutine then what is there
to do? Under perl 5.10.0 and earlier, PDL *could* use attributes.pm to
assign the lvalue attribute and at least perl did not complain. This is
still a change in perl's behavior that breaks a significant body PDL
code in the field.

I cannot find any documentation of how one might give an XS routine the
lvalue attribute, does anyone know if this is possible?

Any other ideas on how to proceed? For the moment, I'm going to use the
pp_hot/SVs_TEMP patch for our local installation, because it works and I
don't want to be stuck at 5.10.0. I'm open to any better fixes, whether
they require changes to PDL or not.

Regards,

  Doug Hunt

On Wed Feb 10 20​:01​:15 2010, stmpeters wrote​:

On Wed, Feb 10, 2010 at 1​:36 PM, Eric Brine <ikegami@​adaelis.com> wrote​:

On Mon, Feb 8, 2010 at 3​:56 PM, Doug Hunt via RT
<perlbug-followup@​perl.org>wrote​:

diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c    2009-04-19 16​:28​:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c    2010-02-08 10​:58​:05.993615795 -0700
@​@​ -2559,7 +2559,7 @​@​
         if (MARK == SP) {
         /* Temporaries are bad unless they happen to be elements
          * of a tied hash or array */
-        if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY) &&
+        if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
             !(SvRMAGICAL(TOPs) && mg_find(TOPs,
PERL_MAGIC_tiedelem))) {
             LEAVE;
             cxstack_ix--;

I disagree with this fix. That check was intentional. It's meant to
detect
lvalue subs that return non-lvalues.

It's hard to test since only lvalue XS functions can trigger that check.

I don't quite understand the meaning of SVs_TEMP, so more knowledgeable

folks should examine this change closely.

It is set on mortals. When this flag is set, the SV's string can be
stolen
instead.

sub f { $x     } $y = f();  # $x's string buffer must be copied into $y
sub f { uc($x) } $y = f();  # uc creates a TEMP whose buffer can be
stolen

It's being used as an indicator to detect the return of non-values by a
lvalue sub.

Agreed. That is not even the code that was changed for that fix. For
reference, please see the pp_hot.c changes mentioned at
http​://perl5.git.perl.org/perl.git/commitdiff/1ad62f6

Steve

@p5pRT
Copy link
Author

p5pRT commented Feb 13, 2010

From deforest@boulder.swri.edu

Hmmm... That seems to be the nub of the problem. On the Perl side of
things, PDLs are blessed scalar refs that contain actual C pointers.
In many cases, functions return temporary PDL objects that are
expected to be treated as lvalues​: the underlying C structure is not
temporary, only the pointer to it is.

The SVs_TEMP check may have been intentional, but it is unnecessarily
restrictive -- in normal use, modifying a temporary value (before
garbage collection) is useless, but not dangerous.

On Feb 10, 2010, at 11​:37 AM, Eric Brine via RT wrote​:

On Mon, Feb 8, 2010 at 3​:56 PM, Doug Hunt via RT
<perlbug-followup@​perl.org>wrote​:

diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c 2009-04-19 16​:28​:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c 2010-02-08 10​:58​:05.993615795 -0700
@​@​ -2559,7 +2559,7 @​@​
if (MARK == SP) {
/* Temporaries are bad unless they happen to be elements
* of a tied hash or array */
- if (SvFLAGS(TOPs) & (SVs_TEMP | SVs_PADTMP | SVf_READONLY)
&&
+ if (SvFLAGS(TOPs) & (SVs_PADTMP | SVf_READONLY) &&
!(SvRMAGICAL(TOPs) && mg_find(TOPs,
PERL_MAGIC_tiedelem))) {
LEAVE;
cxstack_ix--;

I disagree with this fix. That check was intentional. It's meant to
detect
lvalue subs that return non-lvalues.

It's hard to test since only lvalue XS functions can trigger that
check.

I don't quite understand the meaning of SVs_TEMP, so more
knowledgeable

folks should examine this change closely.

It is set on mortals. When this flag is set, the SV's string can be
stolen
instead.

sub f { $x } $y = f(); # $x's string buffer must be copied into
$y
sub f { uc($x) } $y = f(); # uc creates a TEMP whose buffer can be
stolen

It's being used as an indicator to detect the return of non-values
by a
lvalue sub.

@p5pRT
Copy link
Author

p5pRT commented Feb 14, 2010

From @ikegami

On Wed, Feb 10, 2010 at 2​:49 PM, Craig DeForest
<deforest@​boulder.swri.edu>wrote​:

Hmmm... That seems to be the nub of the problem. On the Perl side of
things, PDLs are blessed scalar refs that contain actual C pointers. In
many cases, functions return temporary PDL objects that are expected to be
treated as lvalues​: the underlying C structure is not temporary, only the
pointer to it is.

Values that are SvSMAGICAL should already be excused, but I can see that it
would be a problem if a reference to an object with overloaded operators is
returned. Is that the case here?

Since this only affects ops and XS, since it's harmless and since any
resulting problems from removing the test are easily detectable even without
the test, I remove my objection to the change in general (although others
have objected to the change as a fix for this bug).

@p5pRT
Copy link
Author

p5pRT commented Feb 23, 2010

From @iabyn

On Sat, Feb 13, 2010 at 10​:07​:04PM -0500, Eric Brine wrote​:

On Wed, Feb 10, 2010 at 2​:49 PM, Craig DeForest
<deforest@​boulder.swri.edu>wrote​:

Hmmm... That seems to be the nub of the problem. On the Perl side of
things, PDLs are blessed scalar refs that contain actual C pointers. In
many cases, functions return temporary PDL objects that are expected to be
treated as lvalues​: the underlying C structure is not temporary, only the
pointer to it is.

Values that are SvSMAGICAL should already be excused, but I can see that it
would be a problem if a reference to an object with overloaded operators is
returned. Is that the case here?

Since this only affects ops and XS, since it's harmless and since any
resulting problems from removing the test are easily detectable even without
the test, I remove my objection to the change in general (although others
have objected to the change as a fix for this bug).

I've just spent a bit of time having a look at this PDL / lvalue / debugger
issue, and I think I have a better handle now at to what's going on. I
think there are a few separate issues which have been getting conflated in
the various discussions that have taken place.

But first, I don't think that anyone disagrees that the only issues with PDL
are ones under the debugger; normal use of PDL isn't affected. In which
case I assume this isn't a 5.12 blocker.

Now then...

Factoid 1​: The fatal check for TEMPs returned from lvalue subs only occurs
in the leavesublv op, which is usually compiled as the last op of a sub
declared :lvalue. Note that this means only perl-level subs get the check
(only perl-level subs execute a sequence of perl ops, the last one being
leavesublv). In other words, XS-level lvalue subs are free to return
whatever TEMP stuff they like without any checks being performed.

Factoid 2​: The main difference between a normal sub and an lvalue (perl)
sub (apart from the CVf_LVALUE flag set on the CV), is that the latter is
compiled with a leavesublv op rather than a leavesub op, and the main
difference between these two ops (apart from the TEMP checking) is that
leavesub makes and returns mortal copies of all the returned values, while
leavesublv returns the actual values.

Factoid 3​: when you retrospectively mark a sub as lvalue by using

  sub foo {...}
  use attributes __PACKAGE__ => \&foo, "lvalue"

rather than the direct

  sub foo :lvalue {...}

then the sub gets the CVf_LVALUE flag set, but the leavesub op is *not*
replaced with a leavesublv op. This means that perl-level retrospective
lvalue subs are partially broken. This has to be bourne in mind in any
previous discussions and code snippets - i.e. what you saw may well have
been a red herring. Again, this does *not* affect XS subs, which don't run
leavesublv. So the 'use attributes' method in PDL is kosher(ish).

Factoid 4​: When the debugger is enabled, all function calls are intercepted
and effectively replaced with a wrapper; so

  foo(1,2)

effectively becomes

  $DB​::sub = 'foo'
  DB​::sub(1,2)

where DB​:sub() does whatever it needs to do debugging-wise, then calls the
original sub, i.e. &$DB​::sub.

Factoid 5​: Recently, ie 5.10.1, perl was enhanced to support debugging
lvalue subs. The hook is that rather than calling DB​::sub(), it instead (if
it exists), calls DB​::lsub() instead for lvalue subs. So, a bit of code
like

  MyXSlvalFunc() = 1;

when run with -d, gets interpreted roughly as​:

  $DB​::sub = 'MyXSlvalFunc';
  DB​::lval() = 1;

  sub DB​::lval : lvalue {
  ....
  &$DB​::sub; # call the real sub
  }

So what happens is that DB​::lval() calls MyXSlvalFunc, which returns TEMPs
etc; DB​::lval() then returns, and being an lvalue sub, does leavesublv,
which triggers the error.

So, the ideal fix would be for DB​::lval() to return in such a way to avoid
that, but I can't think of any easy to to that at the moment. Perhaps
make 'goto &$sub' work with lvalue subs? (Or of course, as has been
suggested, just weaken or remove the TEMP check).

Anyway, this has just been a brain dump so that I don't forget everything
I've just learnt when I (or someone else) comes back to look at this again
sometime.

--
The crew of the Enterprise encounter an alien life form which is
surprisingly neither humanoid nor made from pure energy.
  -- Things That Never Happen in "Star Trek" #22

@p5pRT
Copy link
Author

p5pRT commented Feb 24, 2010

From @ikegami

On Tue, Feb 23, 2010 at 3​:15 PM, Dave Mitchell <davem@​iabyn.com> wrote​:

Factoid 1​: The fatal check for TEMPs returned from lvalue subs only occurs
in the leavesublv op, which is usually compiled as the last op of a sub
declared :lvalue. Note that this means only perl-level subs get the check
(only perl-level subs execute a sequence of perl ops, the last one being
leavesublv). In other words, XS-level lvalue subs are free to return
whatever TEMP stuff they like without any checks being performed.

That's my understanding as well. When I said only an lvalue XS sub or an
lvalue builtin can trigger the check, the exact scenario is

sub xs :lvalue; # Returns a (probably magical) TEMP
sub perl :lvalue { xs() }

(Note that no lvalue builtins currently return TEMPs, and that I want to
change that.)

@p5pRT
Copy link
Author

p5pRT commented Feb 25, 2010

From @obra

On Tue 23.Feb'10 at 20​:15​:05 +0000, Dave Mitchell wrote​:

I've just spent a bit of time having a look at this PDL / lvalue / debugger
issue, and I think I have a better handle now at to what's going on. I
think there are a few separate issues which have been getting conflated in
the various discussions that have taken place.

But first, I don't think that anyone disagrees that the only issues with PDL
are ones under the debugger; normal use of PDL isn't affected. In which
case I assume this isn't a 5.12 blocker.

Oh good :) Thanks for digging into it.

Best,
Jesse

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2010

From chm@cpan.org

The breakage for perl -d with programs using PDL
continues to be a problem. Whenever I need to
debug a program, I have to edit all the lvalue
sub reference uses so that the debugger will not
exit. As this is a known problem, is there any
hope for a real fix? It would be nice to have
a patch that could work for earlier perls as well.

Thanks much,
Chris

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2010

From @doughunt

Chris​: Regarding this perl bug, I've just been applying this patch
(shown complete above and pasted here for reference)​:

Inline Patch
diff -r -u -N perl-5.10.1.orig/pp_hot.c perl-5.10.1/pp_hot.c
--- perl-5.10.1.orig/pp_hot.c 2009-04-19 16:28:36.000000000 -0600
+++ perl-5.10.1/pp_hot.c 2010-02-08 10:58:05.993615795 -0700
@@ -2559,7 +2559,7 @@
if (MARK == SP) { /\* Temporaries are bad unless they happen to be elements \* of a tied hash or array \*/ \- if \(SvFLAGS\(TOPs\) & \(SVs\_TEMP | SVs\_PADTMP | SVf\_READONLY\) && \+ if \(SvFLAGS\(TOPs\) & \(SVs\_PADTMP | SVf\_READONLY\) && \!\(SvRMAGICAL\(TOPs\) && mg\_find\(TOPs\, PERL\_MAGIC\_tiedelem\)\)\) \{ LEAVE; cxstack\_ix\-\-;

For a year now to both perl 5.10 and 5.12. I've been using this in a
large body of perl/PDL code in production, no problems.

I would just apply this patch to your local perl distribution, even if
the perl folks won't!

Regards,

  Doug

On Fri Oct 15 17​:40​:36 2010, chm wrote​:

The breakage for perl -d with programs using PDL
continues to be a problem. Whenever I need to
debug a program, I have to edit all the lvalue
sub reference uses so that the debugger will not
exit. As this is a known problem, is there any
hope for a real fix? It would be nice to have
a patch that could work for earlier perls as well.

Thanks much,
Chris

@p5pRT
Copy link
Author

p5pRT commented May 31, 2011

From @cpansprout

This has hopefully now been fixed by commit b724cc1, which takes away
the TEMP check in pp_leavesublv.

Now there is a warning (introduced shortly before in commit 8fe85e3),
which is only triggered if a value is assigned to a returned TEMP.

So the error has basically been downgraded.

@p5pRT
Copy link
Author

p5pRT commented May 31, 2011

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

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

p5pRT commented Jan 8, 2012

From @jmdh

On Mon May 30 21​:35​:27 2011, sprout wrote​:

This has hopefully now been fixed by commit b724cc1, which takes away
the TEMP check in pp_leavesublv.

Now there is a warning (introduced shortly before in commit 8fe85e3),
which is only triggered if a value is assigned to a returned TEMP.

So the error has basically been downgraded.

And this was re-fixed in fd6c41c in a
different way.

There has been a request for the earlier patch for 5.10.1 on this ticket
to be applied in Debian (http​://bugs.debian.org/654387) and I would be
interested in opinions about whether this is sensible to apply.

@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2012

From @cpansprout

On Sun Jan 08 14​:49​:34 2012, dom wrote​:

On Mon May 30 21​:35​:27 2011, sprout wrote​:

This has hopefully now been fixed by commit b724cc1, which takes away
the TEMP check in pp_leavesublv.

Now there is a warning (introduced shortly before in commit 8fe85e3),
which is only triggered if a value is assigned to a returned TEMP.

So the error has basically been downgraded.

And this was re-fixed in fd6c41c in a
different way.

There has been a request for the earlier patch for 5.10.1 on this ticket
to be applied in Debian (http​://bugs.debian.org/654387) and I would be
interested in opinions about whether this is sensible to apply.

The earlier patch is no good. It allows existing bugs (see
<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=78194>) to occur in more
cases.

The later patch is probably OK for 5.10.1, but I have not checked
whether it depends on any other commits. I don’t think it does.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2012

From @cpansprout

On Sun Jan 08 14​:56​:04 2012, sprout wrote​:

On Sun Jan 08 14​:49​:34 2012, dom wrote​:

On Mon May 30 21​:35​:27 2011, sprout wrote​:

This has hopefully now been fixed by commit b724cc1, which takes away
the TEMP check in pp_leavesublv.

Now there is a warning (introduced shortly before in commit 8fe85e3),
which is only triggered if a value is assigned to a returned TEMP.

So the error has basically been downgraded.

And this was re-fixed in fd6c41c in a
different way.

There has been a request for the earlier patch for 5.10.1 on this
ticket
to be applied in Debian (http​://bugs.debian.org/654387) and I would be
interested in opinions about whether this is sensible to apply.

The earlier patch is no good. It allows existing bugs (see
<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=78194>) to occur in more
cases.

Clarification​: If, by the earlier patch, you mean b724cc1, that is no good.

The earlier patch in this ticket
(<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=71172#txn-773706>),
which checks for magic, might be safer than fd6c41c.

The later patch is probably OK for 5.10.1, but I have not checked
whether it depends on any other commits. I don’t think it does.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 9, 2012

From 654387@bugs.debian.org

Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s)​:
Niko Tyni <ntyni@​debian.org>

If you wish to submit further information on this problem, please
send it to 654387@​bugs.debian.org.

Please do not send mail to owner@​bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.

--
654387​: http​://bugs.debian.org/cgi-bin/bugreport.cgi?bug=654387
Debian Bug Tracking System
Contact owner@​bugs.debian.org with problems

1 similar comment
@p5pRT
Copy link
Author

p5pRT commented Jan 9, 2012

From 654387@bugs.debian.org

Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s)​:
Niko Tyni <ntyni@​debian.org>

If you wish to submit further information on this problem, please
send it to 654387@​bugs.debian.org.

Please do not send mail to owner@​bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.

--
654387​: http​://bugs.debian.org/cgi-bin/bugreport.cgi?bug=654387
Debian Bug Tracking System
Contact owner@​bugs.debian.org with problems

@p5pRT
Copy link
Author

p5pRT commented Jan 9, 2012

From @jmdh

On Sun, Jan 08, 2012 at 03​:51​:32PM -0800, Father Chrysostomos via RT wrote​:

On Sun Jan 08 14​:56​:04 2012, sprout wrote​:

On Sun Jan 08 14​:49​:34 2012, dom wrote​:

On Mon May 30 21​:35​:27 2011, sprout wrote​:

This has hopefully now been fixed by commit b724cc1, which takes away
the TEMP check in pp_leavesublv.

Now there is a warning (introduced shortly before in commit 8fe85e3),
which is only triggered if a value is assigned to a returned TEMP.

So the error has basically been downgraded.

And this was re-fixed in fd6c41c in a
different way.

There has been a request for the earlier patch for 5.10.1 on this
ticket
to be applied in Debian (http​://bugs.debian.org/654387) and I would be
interested in opinions about whether this is sensible to apply.

The earlier patch is no good. It allows existing bugs (see
<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=78194>) to occur in more
cases.

Clarification​: If, by the earlier patch, you mean b724cc1, that is no good.

The earlier patch in this ticket
(<https://rt-archive.perl.org/perl5/Ticket/Display.html?id=71172#txn-773706>),
which checks for magic, might be safer than fd6c41c.

The later patch is probably OK for 5.10.1, but I have not checked
whether it depends on any other commits. I don’t think it does.

I meant the earlier patch in this ticket; sorry, and thanks for the
response.

--
Dominic Hargreaves | http​://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)

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