On Thu, Mar 23, 2017 at 03:51:45PM +0100, Balint Reczey wrote:
> On Wed, Mar 22, 2017 at 8:53 PM, Niko Tyni <nt...@debian.org> wrote:
> > On Tue, Mar 21, 2017 at 04:05:43PM +0100, Balint Reczey wrote:
> >
> >> It looks like not all regression are fixed after the re-implementation
> >> and the one affecting nama is one of them.
> >
> > Some effort on a reduced test case would have been appreciated; the nama
> > code base is rather big.
>
> I worked on reducing the test, but I could not reach reasonable size and
> thought someone familiar with Perl internals could potentially spot the
> problem easily from the segfault/assert.
>
> I just found a perl test reducer, I'll give it a try:
> https://github.com/blekko/perl-reduce
Cool, thanks & sorry for being a bit cranky. FWIW I got maybe halfway
there reducing the thing manually before I spotted the Perl version check.
> > if ($minor_version <= 14)
> > { $track->{ops} = [ @new_list ] }
> > else { @{ $track->{ops} } = @new_list }
> >
> > and if I patch that to take the <= 14 code path, the segfault goes away
> > here and all tests pass. What's the history behind this version check?
>
> Funny story, it was added to work around a segfault in older perl:
> https://github.com/bolangi/nama/commit/8ecdbd5ad0b8ac67b7e4adab2ee1100fbf29aa1f
Heh. Thanks for digging that up.
> The line you found looks very much like a corner case for the assignment
> reimplementation.
Yeah. I tried to devise a test case from scratch involving similar
assignments but couldn't get it to happen so it doesn't seem to be
anything trivial.
> Should I forward it to Perl upstream?
That would be nice, thanks. I'm sure they'd also prefer a minimal test case,
but maybe the assert details are enough for them.
I'll also work a bit on reducing the test further when I find the time.
--
Niko
