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: previous op failed to extend arg stack #16091
Comments
From @craigberryOn VMS, blead as of: $ type .patch has several new test failures that have appeared in the last few days, all with the panic in my subject line. Here's the shortest, simplest one run under the VMS debugger: $ dbgperl -T dist/Locale-Maketext/t/09_compile.t OpenVMS I64 Debug64 Version V8.4-001 %DEBUG-I-INITIAL, Language: C, Module: PERLMAIN DBG> go The simplest reproducer I could come up with is: $ perl -MTest::More -e "my @ENV_values = sort keys %ENV;" Removing Test::More makes the panic go away. I'm not really sure where to begin debugging this. There is a fair amount of VMS-only code in hv.c (delineated by DYNAMIC_ENV_FETCH), so my first hunch would be that the magic there disagrees with something new. Here's my configuration: $ perl -V Characteristics of this PERLSHR image: ________________________________________ "... getting out of a sonnet is much more |
From @iabynOn Fri, Jul 28, 2017 at 07:13:35AM -0700, Craig A.Berry wrote:
This panic was added by me a month ago with 87058c3, and has had a couple 978b185 2017-07-16 PL_curstackinfo->si_stack_hwm: gently restore Its quite possible that its a false positive, given that its a new Its mechanism is as follows: In DEBUGGING builds, in Perl_runops_debug, just before calling the pp The complicated bit comes when the pp function triggers runops loop To debug this, I would set a breakpoint on the croak("panic...") call, Eventually you'll either find some code which is pushing things on the stack -- |
The RT System itself - Status changed from 'new' to 'open' |
From @craigberry
Thanks. I tried to follow this advice and found it difficult to identify which instance of S_cx_popblock (where the panic check lives) was firing. Since it's a static function from inline.h, each module has its own copy. By the time the panic hits, the C stack is hosed and there are no active call frames. It appears to be the one from pp_ctl.c, but when I set a breakpoint on that I get stuck in a loop of endless "Out of memory!" messages until I kill it. I also noticed that there are often (but not always) access violations in non-debugging builds, so I don't think this panic is a false positive. I eventually bisected the problem to: commit 8dc9003 hv_pushkv(): handle keys() and values() too The newish function hv_pushkv() currently just pushes all key/value pairs on This is basically moving the remaining list-context functionality out of The rationale for this is that hv_pushkv() is a pure HV-related function, The main difference I can see is that before the commit above, we always extended the stack by 2 regardless of whether we were iterating over keys, values, or both. Now we only extend by 2 when iterating both keys and values, and otherwise only extend by 1. I've further simplified the reproducer to: $ perl -"Dst" -e "$ENV{foo}='bar'; my @E = keys %ENV;" EXECUTING... => When I run the same thing on macOS, the line => PV("bar"\0) PVMG("bar"\0) looks like: => PV("bar"\0) PVMG() but otherwise (except for the panic) looks the same. My debugging time for the weekend is just about up, but it strikes me as plausible that the panic is telling the truth and the multideref($ENV{"foo"}) only extended the stack by one while placing two items on it and the "keys" op is an innocent victim that used to paper things over by reserving two slots on the stack where it really only needs one for its own purposes. But I'm not even sure I have read the debugging output correctly or know what I'm talking about and will have to revisit this later. ________________________________________ "... getting out of a sonnet is much more |
From @iabynOn Sun, Jul 30, 2017 at 05:44:18PM -0500, Craig A. Berry wrote:
I think you're getting confused. The panic lives in Perl_runops_debug().
That's distinctly odd. Since the string 'bar' at that point is on the
The panic means exactly that in Perl_runops_debug(), on return from I would suggest: set a breakpoint at the start of Perl_pp_keys, then set -- |
From @craigberry[at long last getting back to this]
Yup. For some reason I was thinking the assert and the panic happened in the same place.
We use a bunch more stack than we have allocated in Perl_hv_pushkv. The basic problem is that Perl_hv_pushkv extends stack space once based on how many keys are in the HV. But on VMS, the keys don't exist yet until the first call to Perl_hv_iternext_flags triggers a call to Perl_prime_env_iter in vms/vms.c. The following patch (not meant to be applied) shows how far off we are: Inline Patch--- hv.c;-0 2017-07-27 05:49:23 -0500
+++ hv.c 2017-08-05 07:04:59 -0500
@@ -1008,6 +1008,7 @@ Perl_hv_pushkv(pTHX_ HV *hv, U32 flags)
}
else {
Size_t nkeys = HvUSEDKEYS(hv);
+ Size_t actual_nkeys = 0;
SSize_t ext;
if (!nkeys)
@@ -1021,6 +1022,7 @@ Perl_hv_pushkv(pTHX_ HV *hv, U32 flags)
EXTEND(SP, ext);
while ((entry = hv_iternext(hv))) {
+ actual_nkeys++;
if (flags & 1) {
SV *keysv = newSVhek(HeKEY_hek(entry));
SvTEMP_on(keysv);
@@ -1030,6 +1032,9 @@ Perl_hv_pushkv(pTHX_ HV *hv, U32 flags)
if (flags & 2)
PUSHs(HeVAL(entry));
}
+ if (actual_nkeys != nkeys)
+ Perl_croak(aTHX_ "panic: extended stack for %" UVuf " keys but iterated over %" UVuf,
+ (UV)nkeys, (UV)actual_nkeys);
}
PUTBACK;
which dies like so: $ perl -e "$ENV{'foo'}='bar'; my @e = keys %ENV;" The safest thing to do would be to extend the stack inside the loop in Perl_hv_pushkv that iterates over keys, but we already have a code path that does that when the HV is a tied hash. So it seems to me the best solution is to piggyback PERL_MAGIC_env on the PERL_MAGIC_tied logic and consider both a form of being tied where we don't know the number of keys we'll get. The following solves the problem and I will push soonish unless someone tells me there is a better way to do it: commit 61084ee Consider magic %ENV as tied in hv_pushkv. For the DYNAMIC_ENV_FETCH case, we don't know the number of keys Inline Patchdiff --git a/hv.c b/hv.c
index 20b4eceb98..7029e28387 100644
--- a/hv.c
+++ b/hv.c
@@ -988,7 +988,11 @@ void
Perl_hv_pushkv(pTHX_ HV *hv, U32 flags)
{
HE *entry;
- bool tied = SvRMAGICAL(hv) && mg_find(MUTABLE_SV(hv), PERL_MAGIC_tied);
+ bool tied = SvRMAGICAL(hv) && (mg_find(MUTABLE_SV(hv), PERL_MAGIC_tied)
+#ifdef DYNAMIC_ENV_FETCH /* might not know number of keys yet */
+ || mg_find(MUTABLE_SV(hv), PERL_MAGIC_env)
+#endif
+ );
dSP;
PERL_ARGS_ASSERT_HV_PUSHKV;
________________________________________ "... getting out of a sonnet is much more |
From @iabynOn Sat, Aug 05, 2017 at 09:56:56AM -0500, Craig A. Berry wrote:
Looks good to me. -- |
From @craigberry
Thanks, now pushed. This ticket can be closed. ________________________________________ "... getting out of a sonnet is much more |
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#131811 (status was 'resolved')
Searchable as RT131811$
The text was updated successfully, but these errors were encountered: