Skip Menu |
Report information
Id: 128238
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: dcollinsn [at] gmail.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



Subject: Assert fail in gv.c without other symptoms: use re%:=0
Download (untitled) / with headers
text/plain 15.4k

Message body is not shown because it is too large.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Wed May 25 15:58:12 2016, dcollinsn@gmail.com wrote: Show quoted text
> use re%:=0
Less noisy version: use less %: = 0 Show quoted text
> A git bisect was performed and > reported the following, which is the commit in which the assert was > initially added. > > 9075437773fb626926ef91a510090f595c08c653 is the first bad commit > commit 9075437773fb626926ef91a510090f595c08c653 > Author: David Mitchell <davem@iabyn.com> > Date: Sat Feb 15 16:38:31 2014 +0000 > > gv_check(): use aux flag rather than IsCOW
The assertion seems to be wrong. In fact, it seems that a BEGIN-time require *and* %: assignment are sufficient to trigger it. I don’t understand why ‘require’ is affecting it. It happens with re.pm and less.pm, but not utf8.pm, so presumably a line of code common to the first two is helping to trigger this. $ ./miniperl -Ilib -e 'BEGIN { require re; %: = 0}' Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417. Abort trap: 6 $ ./miniperl -Ilib -e 'BEGIN { require utf8; %: = 0}' $ ./miniperl -Ilib -e 'BEGIN { require less; %: = 0}' Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417. Abort trap: 6 -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Wed May 25 18:11:15 2016, sprout wrote: Show quoted text
> On Wed May 25 15:58:12 2016, dcollinsn@gmail.com wrote:
> > use re%:=0
> > Less noisy version: > > use less %: = 0 >
> > A git bisect was performed and > > reported the following, which is the commit in which the assert was > > initially added. > > > > 9075437773fb626926ef91a510090f595c08c653 is the first bad commit > > commit 9075437773fb626926ef91a510090f595c08c653 > > Author: David Mitchell <davem@iabyn.com> > > Date: Sat Feb 15 16:38:31 2014 +0000 > > > > gv_check(): use aux flag rather than IsCOW
> > The assertion seems to be wrong. > > In fact, it seems that a BEGIN-time require *and* %: assignment are > sufficient to trigger it. > > I don’t understand why ‘require’ is affecting it. It happens with > re.pm and less.pm, but not utf8.pm, so presumably a line of code > common to the first two is helping to trigger this. > > $ ./miniperl -Ilib -e 'BEGIN { require re; %: = 0}' > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, > line 2417. > Abort trap: 6 > $ ./miniperl -Ilib -e 'BEGIN { require utf8; %: = 0}' > $ ./miniperl -Ilib -e 'BEGIN { require less; %: = 0}' > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, > line 2417. > Abort trap: 6
Anything that enables warnings (of course, since without warnings enabled at least somewhere gv_check doesn’t get called). $ ./miniperl -Ilib -e 'BEGIN { %: = 0; $^W=1}' Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417. Abort trap: 6 -- Father Chrysostomos
Date: Tue, 21 Jun 2016 17:12:17 +0100
CC: perl5-porters [...] perl.org
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #128238] Assert fail in gv.c without other symptoms: use re%:=0
Download (untitled) / with headers
text/plain 986b
On Wed, May 25, 2016 at 08:12:32PM -0700, Father Chrysostomos via RT wrote: Show quoted text
> Anything that enables warnings (of course, since without warnings enabled at least somewhere gv_check doesn’t get called). > > $ ./miniperl -Ilib -e 'BEGIN { %: = 0; $^W=1}' > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417.
Fixed with this: commit e7acdfe976f01ee0d1ba31b3b1db61454a72d6c9 Author: David Mitchell <davem@iabyn.com> AuthorDate: Tue Jun 21 17:06:52 2016 +0100 Commit: David Mitchell <davem@iabyn.com> CommitDate: Tue Jun 21 17:06:52 2016 +0100 only treat stash entries with .*:: as sub-stashes RT #128238 %: = 0 would cause an assertion failure in Perl_gv_check(), since when it searched a stash for substashes, it assumed anything ending in ':' was a substash, whereas substashes end in '::'. So check for a double colon before recursing. -- Never work with children, animals, or actors.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Tue Jun 21 09:12:49 2016, davem wrote: Show quoted text
> On Wed, May 25, 2016 at 08:12:32PM -0700, Father Chrysostomos via RT > wrote:
> > Anything that enables warnings (of course, since without warnings > > enabled at least somewhere gv_check doesn’t get called). > > > > $ ./miniperl -Ilib -e 'BEGIN { %: = 0; $^W=1}' > > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, > > line 2417.
> > Fixed with this: > > commit e7acdfe976f01ee0d1ba31b3b1db61454a72d6c9 > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Tue Jun 21 17:06:52 2016 +0100 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Tue Jun 21 17:06:52 2016 +0100 > > only treat stash entries with .*:: as sub-stashes > > RT #128238 > > %: = 0 would cause an assertion failure in Perl_gv_check(), since when > it searched a stash for substashes, it assumed anything ending in ':' > was > a substash, whereas substashes end in '::'. So check for a double > colon > before recursing.
Apologies if this sounds rude, but that is not a very robust fix. This code is naughty, but it still should not crash: $ ./perl -Ilib -MDevel::Peek -e 'BEGIN { $::{"foo::"} = *ENV; $^W=1}' Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417. Abort trap: 6 -- Father Chrysostomos
Date: Thu, 23 Jun 2016 13:56:54 +0100
Subject: Re: [perl #128238] Assert fail in gv.c without other symptoms: use re%:=0
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 749b
On Wed, Jun 22, 2016 at 01:23:47PM -0700, Father Chrysostomos via RT wrote: Show quoted text
> Apologies if this sounds rude, but that is not a very robust fix. > > This code is naughty, but it still should not crash: > > $ ./perl -Ilib -MDevel::Peek -e 'BEGIN { $::{"foo::"} = *ENV; $^W=1}' > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, line 2417. > Abort trap: 6
Ah, so its still possible to put substash-like entries in stashes that aren't actually stashes. I don't know whether the correct fix is stashify values put into stashes whose keys end in '::', or make core code robust against non-stashiness. This isn't an area I'm very conversant with. Do you have any opinions? -- Never do today what you can put off till tomorrow.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Thu Jun 23 05:57:27 2016, davem wrote: Show quoted text
> On Wed, Jun 22, 2016 at 01:23:47PM -0700, Father Chrysostomos via RT > wrote:
> > Apologies if this sounds rude, but that is not a very robust fix. > > > > This code is naughty, but it still should not crash: > > > > $ ./perl -Ilib -MDevel::Peek -e 'BEGIN { $::{"foo::"} = *ENV; > > $^W=1}' > > Assertion failed: (SvOOK(stash)), function Perl_gv_check, file gv.c, > > line 2417. > > Abort trap: 6
> > Ah, so its still possible to put substash-like entries in stashes that > aren't actually stashes. > > I don't know whether the correct fix is stashify values put into > stashes > whose keys end in '::', or make core code robust against non- > stashiness. > > This isn't an area I'm very conversant with. Do you have any opinions?
Since this code is just for a warning, I would suggest skipping sub-stashes that are not SvOOK. In fact, if we do that at the beginning of gv_check, we don’t need to check HvARRAY (we can switch the conditions in the if() and the assert() around). In fact, I might go ahead and write the patch within the next day or two. :-) -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 834b
On Thu Jun 23 06:36:48 2016, sprout wrote: Show quoted text
> On Thu Jun 23 05:57:27 2016, davem wrote:
> > Ah, so its still possible to put substash-like entries in stashes > > that > > aren't actually stashes. > > > > I don't know whether the correct fix is stashify values put into > > stashes > > whose keys end in '::', or make core code robust against non- > > stashiness. > > > > This isn't an area I'm very conversant with. Do you have any > > opinions?
> > Since this code is just for a warning, I would suggest skipping sub- > stashes that are not SvOOK. In fact, if we do that at the beginning > of gv_check, we don’t need to check HvARRAY (we can switch the > conditions in the if() and the assert() around). In fact, I might go > ahead and write the patch within the next day or two. :-)
Fixed in 9e5cda6. -- Father Chrysostomos
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org