-
Notifications
You must be signed in to change notification settings - Fork 571
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
Panic in pure-Perl code with vanilla perl-5.16.0 from perlbrew #12315
Comments
From @shlomifCreated by @shlomifThis Bash script causes a panic with vanilla perl-5.16.0 (installed <<<< The code does not make a lot of sense, but it was a starting point for working http://golf.shinh.org/p.rb?MaxMinSwap When running it I get: shlomif@telaviv1:~$ bash reproduce.bash Further lines are not processed. Please look into fixing it. Perl Info
|
From @nwc10On Wed Aug 01 12:00:05 2012, shlomif@shlomifish.org wrote:
Interesting test case. It's not a new bug: $ echo -e (earlier versions seem to SEGV instead of panicing) I suspect that the old contents of @x are getting freed whilst they Nicholas Clark |
The RT System itself - Status changed from 'new' to 'open' |
From dennis.kaarsemaker@booking.comOn wo, 2012-08-01 at 12:09 -0700, Nicholas Clark via RT wrote:
Actually, the reverse seems unrelated (or I've uncovered another bug). I ./perl -e '@x = (1,2); @h{@x=sort}=@x' -- |
From @iabynOn Wed, Aug 01, 2012 at 09:57:06PM +0200, Dennis Kaarsemaker wrote:
The sort's unrelated too. It can be reduced further to: @x = (1,2); @h{@x=()}=@x; Looks like a classic stack-not-refcounted bug. The elements of @x are -- |
From @cpansproutOn Wed Aug 01 12:00:05 2012, shlomif@shlomifish.org wrote:
I plan to eventually, but it will require rewriting thousands of lines -- Father Chrysostomos |
From @shlomifOn Mon, 03 Sep 2012 21:49:34 -0700
well, thanks for your willingness, and it sucks that it will require so much Regards, Shlomi Fish -- Shlomi Fish http://www.shlomifish.org/ And the top story for today: wives live longer than husbands because they are Please reply to list if it's a mailing list post - http://shlom.in/reply . |
From @cpansproutOn Wed Sep 05 05:13:40 2012, shlomif@shlomifish.org wrote:
The stack, used to pass arguments between operators, does not hold a @x = (1,2); @{ @x = (); \@y } = @x; The RHS of = is evaluated first, so the second statement puts the Simply making the stack ref-counted would break XS modules. So we would All Perl operators take care of stack manipulation themselves. So they -- Father Chrysostomos |
From @nwc10On Wed Sep 05 09:01:14 2012, sprout wrote:
At least for XSUBs that weren't flagged as expecting the "new stack" (Yes, I can't see a simpler solution to this problem than the one you Nicholas Clark |
From @tseeOn 09/05/2012 06:01 PM, Father Chrysostomos via RT wrote:
And making the stack refcounted would be a major, major slow-down. Let's --Steffen |
From @nwc10On Wed, Sep 05, 2012 at 07:18:54PM +0200, Steffen Mueller wrote:
I wouldn't assume that without actually testing it. Certainly not All that would be abolished with a properly reference counted stack. So it's Nicholas Clark |
From @LeontOn Wed, Sep 5, 2012 at 10:38 PM, Nicholas Clark <nick@ccl4.org> wrote:
Data point: python has a refcounted stack, and that doesn't seem to Leon |
From @LeontOn Wed, Sep 5, 2012 at 6:01 PM, Father Chrysostomos via RT
I suspect we can provide a compatibility API that DWIMs on both Leon |
From @iabynOn Thu, Sep 06, 2012 at 03:09:44PM +0200, Leon Timmermans wrote:
If we have a refcounted stack, allowing existing XS subs to operate So the only extra cost to support an old-style XSUB over a new-style one -- |
From @cpansproutI have been thinking more about this lately. On Sat Sep 08 05:01:55 2012, davem wrote:
E.g., a POPs that mortalizes the value when removing it from the stack, instead of freeing it. It would still behave as expected, though it would be less efficient. But having it work only for the ‘most common usage patterns’ is not acceptable, considering that all these macros have long been part of the public API.
Yes, it can be made to work trivially for XSUBs. But what about custom pp functions? There seem to be more and more modules these days that use them, so we do need some way to have things work under both systems. What is particularly difficult is that the same .xs file may have pp functions and XSUBs. If POPs in an XSUB is old-style, it has to be defined as (*sp--). pp functions have to use whichever stack perl itself uses, so that would be sv_2mortal(*sp--) on a newer perl. How can we have POPs defined both ways in the same file? Alternatively, if we use new macro names, then the pp function would have to use rPOPs on newer perls and POPs on older perls, which is a major API breakage. Another thought I had is that a final pass when compiling a sub could wrap custom pp functions with a wrapper that copies things to a new stack and copies them back, just as we would do for XSUBs. I don’t like it, but it may be the only way. Though I still don’t think it would work for custom pp functions that wrap perl’s own pp functions. I’m stuck. -- Father Chrysostomos |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
There aren't too many at the moment. If an incompatible change is
Wrong question. pp functions require more update than just different Though if you're concerned about mixing pp styles in one file, the
The simpler pp functions and xsub bodies, which work equivalently on #ifdef rPOPs and xsub source can likewise have a flag state saying that the code
No, you can't do that, at least not the way you would for an xsub. The -zefram |
From @cpansproutOn Fri Jul 15 20:54:22 2016, zefram@fysh.org wrote:
Thank you for your response. For op_sibling, we provided a transitional period in which defining PERL_OP_PARENT would flag which modules needed updated, without them breaking just yet. A simple define will not be practical, because perl’s own pp functions would be littered with #ifdefs. The only way I can think of providing a transitional phase is to #define op_ppaddr op_rppaddr /* or whatever we call it */ and provide a define that disables that. Affected modules will leak memory, but it may not be the end of the world. We will have a define to catch them. dSP would have to be redefined to use the refcounted stack. eupxs uses something other than dSP for the old stack. It’s ugly. It probably breaks some XS modules that do dSP on their own. -- Father Chrysostomos |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
Like this... Core headers define macros to operate both ways based on a switch: #define POPs (PERL_STACK_LOCALLY_REFCOUNTED ? \ Core headers also define a macro saying how the stack really behaves in #define PERL_STACK_GLOBALLY_REFCOUNTED 1 /* or 0 */ Code that needs to be portable to older Perl versions then has a reserve #ifndef PERL_STACK_GLOBALLY_REFCOUNTED There's then some dance around redefining PERL_STACK_LOCALLY_REFCOUNTED. #define PERL_STACK_LOCALLY_REFCOUNTED PERL_STACK_GLOBALLY_REFCOUNTED When compiling an XS definition of an xsub, how to define it depends #define PERL_STACK_LOCALLY_REFCOUNTED 0 and if the core stack is refcounted then we also invoke the magic to On Perl versions predating this mechanism, with the above reserve Theoretically we could also have xsub code that will only operate with With the switch in place, the usual way to write pp functions would be -zefram |
From perl5-porters@perl.orgZefram wrote:
POPs would be defined as sv_2mortal(...), but it would be a backward-compatibility macro. I would not expect much (if any) code in core I only brought up POPs because it was the easiest macro I could rede-
It makes sense to avoid the mortals stack, because by using it we are
I think it is acceptable to have compatibility macros that use mortal-
Good point.
That was one of the ideas I had in mind.
Which is why this bug, known about for decades, has gone unfixed. :-(
As I said above, I think it is just a matter of having one stack hold- |
From zefram@fysh.orgFather Chrysostomos wrote:
Any really extensive change in stack usage like this is bound to introduce If we were to introduce more mortal refs now, that wouldn't preclude If we go with mortalisation, we can make some performance improvements -zefram |
From @cpansproutOn Sun Jul 17 11:19:22 2016, zefram@fysh.org wrote:
That seems to imply ‘Don’t do it’.
Which seems to imply ‘Do do it’. :-) So you are saying don’t do it now, do it later. But that does not avoid the fact that it is bound to introduce more bugs. Surely I am misunderstanding you.
I am not sure I understand your argument about our failure to organise it. Procrastination is not necessarily the lack of organisation. It merely indicates that nobody competent enough cared enough actually to do anything about it. If we could fix the bugs easily and the optimise, that would be nice. But it may be entail a lot of work twice over. See below.
We would also need to do it for rv2sv, rv2gv, the refaliasing ops (or padsv), and maybe a whole list of other ops. A careful review of the various cases of failure reported over the years may help. We currently use the mortals stack in many places where it is not needed most of the time, but we do it for the sake of edge cases. It may be worth making the stack track reference counts just for that reason. Benchmarking will tell. My arguments above may sound as though I want to make the stack keep reference counts come hell or high water, but I am actually ambivalent. -- Father Chrysostomos |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
It's not really a normative statement. It's more an assessment of
True, but my plan would change the context in which we do it later.
Yes, an awful lot of ops would need this added. But if we miss some,
I'm likewise open-minded regarding how we keep things on the stack alive; -zefram |
From zefram@fysh.orgI wrote:
Let's flesh this out a bit more. The safe way is that the normal state is I'll illustrate the process with a simple case of a custom pp function static OP *THX_pp_int_add(pTHX) When the refcounted-stack-capable Perl version comes out, this function #define PERL_STACK_LOCALLY_REFCOUNTED PERL_STACK_GLOBALLY_REFCOUNTED Note that if we did not have the rather cautious PUTBACK/SPAGAIN around This version using the compatibility macros is relatively inefficient #define PERL_STACK_LOCALLY_REFCOUNTED PERL_STACK_GLOBALLY_REFCOUNTED Safety for non-local exits is maintained by leaving the operands on the I've written out the stack pointer and refcount manipulation explicitly, #define POPdiscard ((void) (PERL_STACK_LOCALLY_REFCOUNTED ? \ PUSHowned() is quite close to the compatibility version of the existing We'd also want a parameterised version of TOPm1s, because we're going With these macros, our pp function becomes: #define PERL_STACK_LOCALLY_REFCOUNTED PERL_STACK_GLOBALLY_REFCOUNTED This is almost as readable as the original. Although to maintain In theory one could also write pp functions for one or other stack style STATIC_ASSERT_GLOBAL(PERL_STACK_LOCALLY_REFCOUNTED); For regular XS code we're going to have a compatibility mechanism so A problem arises with the stack manipulation code generated by the XS -zefram |
From @cpansproutOn Sat Jul 16 15:31:33 2016, zefram@fysh.org wrote:
How would you suggest we handle call_sv and similar functions? Should we stop the macro wrapper from being generated by regen/embed.pl and define something like this manually: #define call_sv(a,b) \ where the _rstack version takes a bool parameter? But what do we do with calls that explicitly use a Perl_ prefix? Should they just be assumed to be using the non-rc stack? Should Perl_call_sv be deprecated in favour of the macro, so that any code using it will warn at compile time? I can see several ways of doing this. I would like the opinion of others. -- Father Chrysostomos |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
That's tricky. I reckon we can't get away with defining call_sv() -zefram |
Migrated from rt.perl.org#114372 (status was 'open')
Searchable as RT114372$
The text was updated successfully, but these errors were encountered: