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
Comments
From deforest@boulder.swri.eduPerl Data Language exploits what is normally a non-harmful, non-useful The problem is that structured array slicing in PDL is implemented using A frequent construction is: $pdl_a->slice($slice_spec) .= $pdl_b; where .= is overloaded to do elementwise assignment (copying), and 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. The roadblock was apparently put in by some well-meaning person who Arguably the roadblock is not helpful because temporary lvalue Perl Info
|
From @tseeHi Craig, craig deforest wrote:
That's umm... severe. I know it's not helping at all, but I must say I wonder why NONE of the
I suspect it's potentially more than an expensive no-op and that the The curious bit (for me anyway) is that the relevant lines appear to be Best regards, PS: What I am referring to is: e9f19e3 (Hugo van der Sanden 2002-11-03 16:41:24 +0000 2561) and f206cdd (Abhijit Menon-Sen 2001-06-17 18:34:46 +0530 2594) |
The RT System itself - Status changed from 'new' to 'open' |
From p5p@perl.wizbit.beCiteren craig deforest <perlbug-followup@perl.org>:
Is it added in perl 5.10.1? Or was it added in 5.10.0? Or ...?
If you understand what is happening then please add a test case.
Then why does cpantesters shows PASSes on
How did you determine this? To what commit are you refering?
How did you determine this? If there was a test case then it would be easy to find the answers to Best regards, Bram |
From @tseeHi Craig, craig deforest wrote:
PDL tests run successfully on a freshly installed 5.10.1 on my ubuntu 9.04. Cheers, |
From @nwc10On Fri, Dec 11, 2009 at 10:54:46AM +0100, Steffen Mueller wrote:
Your test report doesn't include the output of perl -V from your platform. Nicholas Clark |
From @ikegamiOn Thu, Dec 10, 2009 at 11:47 AM, craig deforest
Arguably the roadblock is not helpful because temporary lvalue
The roadblock was apparently put in by some well-meaning person who
I've looked at this area of the code working on [perl #67838]. My solution The message is designed to prevent coding mistakes. Perl itself doesn't have But I found no recent changes in that area. I had looked into when the check IIRC, the message is only triggered when the last expression of a Pure Perl 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) && |
From @ikegamiOn Thu, Dec 10, 2009 at 11:47 AM, craig deforest
[...]
[...]
I cannot reproduce it. Could you give a simple program that produces that use PDL;
[
This is perl, v5.10.1 built for MSWin32-x86-multi-thread Copyright 1987-2009, Larry Wall Binary build 1006 [291086] provided by ActiveState [...]
|
From dcmertens@gmail.comHello everybody - It seems that an important detail from the original PDL mailing list perl -d -MPDL -e'$a=sequence(9); $a->slice("1") .= -9; print $a, "\n"' DB<1> c Like I said, if you run this on a plain-old perl lvalue sub, it doesn't perl -d -e 'my $val; sub canmod : lvalue {$val;} canmod() = 5; print DB<1> c So it looks like there's something fishy going on in PDL's lvalue |
From @doughuntHi all: After a bit more investigation, I think I've figured out what First of all, Craig did not note in his initial bug report that this Here is a simple test case: perl -MPDL -e '$p=zeroes(9); $p->slice("1") .= 9; print $p' This works fine under 5.10.0 with and without the debugger, but under [dhunt@arcana ~]$ perl -d -MPDL -e '$p=zeroes(9); $p->slice("1") .= 9; main::(-e:1): $p=zeroes(9); $p->slice("1") .= 9; print $p I think the problem is with setting the 'lvalue' attribute using the This fails under 5.10.1 when using the debugger, but succeeds under sub canmod { $val } This, however, always works: sub canmod : lvalue { $val } The 'attributes' pragma is used by PDL to mark the 'slice' routine as So, there has been a change for the worse from perl 5.10.0 to 5.10.1 in Regards, Doug Hunt |
From @doughuntHi again: I trying to find what is different between perl 5.10.1 and 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 It fails as expected in leavesublv. But when I do the same thing for perl 5.10.0, it completes successfully Just another data point... |
From @doughuntHi all: I've spent more time debugging this problem and have attached a --It appears to have been caused by the patch applied in perl 5.10.1 for --It seems that the lvalue attribute when applied to a subroutine: sub canmod : lvalue works properly under the debugger, but when applied using use attributes __PACKAGE__ => \&canmod, "lvalue"; --The patch I attach does not address the question of how the lvalue 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 I don't quite understand the meaning of SVs_TEMP, so more knowledgeable I'm going to be using this patch for our local production machines, so I Best Regards, Doug Hunt |
From @doughuntlvalue_71172.patchdiff -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--;
|
From @rgarciaOn 8 February 2010 21:56, Doug Hunt via RT <perlbug-followup@perl.org> wrote:
But that patch only affects code run in the debugger or with some kind
It would be helpful to have a reproducible test case for this, that
I'll have to look at that more closely. However I note at 1st glance |
From gerard@ggoossen.netThe lvalue attribute can't be added to an already existing subroutine. Gerard Goossen. On Mon, Feb 08, 2010 at 12:56:57PM -0800, Doug Hunt via RT wrote:
|
From @ikegamiOn Mon, Feb 8, 2010 at 3:56 PM, Doug Hunt via RT
I disagree with this fix. That check was intentional. It's meant to detect 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
It is set on mortals. When this flag is set, the SV's string can be stolen sub f { $x } $y = f(); # $x's string buffer must be copied into $y It's being used as an indicator to detect the return of non-values by a |
From @doughuntHi Rafael: Thanks for looking into this. The patch for bug 7013 added lvalue support to debugged subroutines via 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 and after being unable to determine why the SVs_TEMP flag on the stack sub canmod { $val } but not when you run this script: sub canmod : lvalue { $val } I decided to take the more direct approach and take SVs_TEMP out of the One odd thing is that when running normally (not under the debugger), So, I have these four cases: script1, no debugger: executes pp_leavesub, not pp_leavesublv so So there are perhaps 2 problems: 1) Setting 'lvalue' via attributes.pm does not seem to work properly 2) SVs_TEMP is being set (perhaps inappropriately) for the stack top My patch took the simpler tack of just not worrying about SVs_TEMP. My fix might not get at the root of the problem, but it is simple and Hope this all makes sense! Regards, Doug Hunt |
From @doughuntThanks, Gerard: On Wed Feb 10 09:28:44 2010, ggoossen wrote:
This is interesting and news to me. The PDL folks use attributes.pm to Regards, Doug Hunt |
From @smpetersOn Wed, Feb 10, 2010 at 1:36 PM, Eric Brine <ikegami@adaelis.com> wrote:
Agreed. That is not even the code that was changed for that fix. For Steve |
From @doughuntHi all: OK, so if getting rid of the SVs_TEMP check in PDL uses 'attributes.pm' to set the lvalue attribute on its subs (XS and I cannot find any documentation of how one might give an XS routine the Any other ideas on how to proceed? For the moment, I'm going to use the Regards, Doug Hunt On Wed Feb 10 20:01:15 2010, stmpeters wrote:
|
From deforest@boulder.swri.eduHmmm... That seems to be the nub of the problem. On the Perl side of The SVs_TEMP check may have been intentional, but it is unnecessarily On Feb 10, 2010, at 11:37 AM, Eric Brine via RT wrote:
|
From @ikegamiOn Wed, Feb 10, 2010 at 2:49 PM, Craig DeForest
Values that are SvSMAGICAL should already be excused, but I can see that it Since this only affects ops and XS, since it's harmless and since any |
From @iabynOn Sat, Feb 13, 2010 at 10:07:04PM -0500, Eric Brine wrote:
I've just spent a bit of time having a look at this PDL / lvalue / debugger But first, I don't think that anyone disagrees that the only issues with PDL Now then... Factoid 1: The fatal check for TEMPs returned from lvalue subs only occurs Factoid 2: The main difference between a normal sub and an lvalue (perl) Factoid 3: when you retrospectively mark a sub as lvalue by using sub foo {...} rather than the direct sub foo :lvalue {...} then the sub gets the CVf_LVALUE flag set, but the leavesub op is *not* Factoid 4: When the debugger is enabled, all function calls are intercepted foo(1,2) effectively becomes $DB::sub = 'foo' where DB:sub() does whatever it needs to do debugging-wise, then calls the Factoid 5: Recently, ie 5.10.1, perl was enhanced to support debugging MyXSlvalFunc() = 1; when run with -d, gets interpreted roughly as: $DB::sub = 'MyXSlvalFunc'; sub DB::lval : lvalue { So what happens is that DB::lval() calls MyXSlvalFunc, which returns TEMPs So, the ideal fix would be for DB::lval() to return in such a way to avoid Anyway, this has just been a brain dump so that I don't forget everything -- |
From @ikegamiOn Tue, Feb 23, 2010 at 3:15 PM, Dave Mitchell <davem@iabyn.com> wrote:
That's my understanding as well. When I said only an lvalue XS sub or an sub xs :lvalue; # Returns a (probably magical) TEMP (Note that no lvalue builtins currently return TEMPs, and that I want to |
From @obraOn Tue 23.Feb'10 at 20:15:05 +0000, Dave Mitchell wrote:
Oh good :) Thanks for digging into it. Best, |
From chm@cpan.orgThe breakage for perl -d with programs using PDL Thanks much, |
From @doughuntChris: Regarding this perl bug, I've just been applying this patch Inline Patchdiff -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 @@
For a year now to both perl 5.10 and 5.12. I've been using this in a I would just apply this patch to your local perl distribution, even if Regards, Doug On Fri Oct 15 17:40:36 2010, chm wrote:
|
From @cpansproutThis has hopefully now been fixed by commit b724cc1, which takes away Now there is a warning (introduced shortly before in commit 8fe85e3), So the error has basically been downgraded. |
@cpansprout - Status changed from 'open' to 'resolved' |
From @jmdhOn Mon May 30 21:35:27 2011, sprout wrote:
And this was re-fixed in fd6c41c in a There has been a request for the earlier patch for 5.10.1 on this ticket |
From @cpansproutOn Sun Jan 08 14:49:34 2012, dom wrote:
The earlier patch is no good. It allows existing bugs (see The later patch is probably OK for 5.10.1, but I have not checked -- Father Chrysostomos |
From @cpansproutOn Sun Jan 08 14:56:04 2012, sprout wrote:
Clarification: If, by the earlier patch, you mean b724cc1, that is no good. The earlier patch in this ticket
-- Father Chrysostomos |
From 654387@bugs.debian.orgThank you for the additional information you have supplied regarding This is an automatically generated reply to let you know your message Your message is being forwarded to the package maintainers and other Your message has been sent to the package maintainer(s): If you wish to submit further information on this problem, please Please do not send mail to owner@bugs.debian.org unless you wish -- |
1 similar comment
From 654387@bugs.debian.orgThank you for the additional information you have supplied regarding This is an automatically generated reply to let you know your message Your message is being forwarded to the package maintainers and other Your message has been sent to the package maintainer(s): If you wish to submit further information on this problem, please Please do not send mail to owner@bugs.debian.org unless you wish -- |
From @jmdhOn Sun, Jan 08, 2012 at 03:51:32PM -0800, Father Chrysostomos via RT wrote:
I meant the earlier patch in this ticket; sorry, and thanks for the -- |
Migrated from rt.perl.org#71172 (status was 'resolved')
Searchable as RT71172$
The text was updated successfully, but these errors were encountered: