On Tue Dec 12 16:13:15 2017, zefram@fysh.org wrote:
Show quoted text> List-MoreUtils-XS has started failing its tests at core commit
> 16ada235c332e017667585e1a5a00ce43a31c529. This is due to preexisting
> bugs in L-MU-XS getting noticed due to a core bugfix.
>
> The underlying bugs in L-MU-XS are in the treatment of
> GvSV(PL_{first,second,def}gv), that is, in the aliasing of $a, $b, and
> $_ that is done by several of its functions. The GvSV slot of a glob
> is
> refcounted, but L-MU-XS isn't diddling reference counts. This is
> actually
> the same bug that the core pp_sort had that the breaking commit was
> fixing. L-MU-XS has probably cribbed the logic from the buggy
> pp_sort.
>
> When assigning to GvSV, you need to increment the refcount of the
> new value first, and then decrement the refcount of the old value
> afterwards. When you want to localise GvSV around a function, the
> way pp_sort does, SAVESPTR() is the wrong way to do it. You need to
> SAVEGENERICSV() and then increment the reference count. See core
> commit
> 16ada235c332e017667585e1a5a00ce43a31c529 for an example of this type
> of bugfix.
>
> I think the actual failure mode that's causing test failure arises
> from
> this refcounting error, which can be seen on any perl version:
>
> $ perl -MDevel::Peek -MList::MoreUtils::XS=reduce_0 -lwe 'print
> reduce_0 { $a + $b } 2,3,4; Dump $a'
> 9
> SV = UNKNOWN(0xff) (0x2478928) at 0x2455110
> REFCNT = 0
> FLAGS = ()
>
> The issue there is that $a was locally set to the partial sum,
> repeatedly,
> but each partial sum scalar was mortal. When the reduction was
> complete,
> $a was left aliased to a scalar that was now dead. (The bugfixed
> pp_sort,
> performing refcounting on the $a that was in place when it was
> invoked,
> then barfed on this scalar already being dead.) Properly refcounting
> GvSV would fix this, but there's another issue too. I think
> reduce_0()
> et al should not be side-effecting $a and $b. I think reduction
> should
> localise $a and $b, just as some of your other functions do.
>
> This breakage is being tracked on the core side as [perl #132578].
>
> -zefram
Without the intension to disagree - cause I see the missing refcnt()
and believe this is a long undiscovered bug, the only segfault I can
reproduce is in:
(lldb) target create "/Users/sno/perl5/perlbrew/perls/perl-bisect/bin/perl"
Current executable set to '/Users/sno/perl5/perlbrew/perls/perl-bisect/bin/perl' (x86_64).
(lldb) settings set -- target.run-args "-Mblib" "t/xs/occurrences.t"
(lldb) process launch
Process 64124 launched: '/Users/sno/perl5/perlbrew/perls/perl-bisect/bin/perl' (x86_64)
ok 1 - Each word is counted
ok 2 - Text to long, each word is there at least twice
ok 3 - 11 comma
ok 4 - 7 dots
ok 5 - 9 words 'et'
ok 6 - Words are as many as requested at www.loremipsum.de
ok 7 - probes untouched
ok 8 - occurrences of integer probes
Process 64124 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xff00000008)
frame #0: 0x00000001000bd176 perl`S_sv_unmagicext_flags + 102
perl`S_sv_unmagicext_flags:
-> 0x1000bd176 <+102>: movq 0x8(%rbx), %rax
0x1000bd17a <+106>: movsbl 0x12(%rbx), %esi
0x1000bd17e <+110>: cmpq %r12, %rax
0x1000bd181 <+113>: sete %dl
Target 0: (perl) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xff00000008)
* frame #0: 0x00000001000bd176 perl`S_sv_unmagicext_flags + 102
frame #1: 0x00000001000be067 perl`Perl_sv_clear + 263
frame #2: 0x00000001000bee1e perl`Perl_sv_free2 + 94
frame #3: 0x00000001000df425 perl`Perl_leave_scope + 357
frame #4: 0x000000010014c86d perl`Perl_pp_sort + 3181
frame #5: 0x00000001002f8826 LeakTrace.bundle`leaktrace_runops at LeakTrace.xs:184 [opt]
frame #6: 0x0000000100028f7c perl`perl_run + 876
frame #7: 0x0000000100000d15 perl`main + 149
frame #8: 0x00007fffded32235 libdyld.dylib`start + 1
Which is caused by following perl code:
{
my @probes = ((1) x 3, undef, (2) x 4, undef, (3) x 2, undef, (4) x 7, undef, (5) x 2, undef, (6) x 4);
my $fp = freeze(\@probes);
my @o = map {
ref $_
? [sort { (defined $a <=> defined $b) or $a <=> $b } @$_]
: $_
} occurrences @probes;
}
And - to be honest - I can't see any failing refcnt in occurrences (...)
Best regards,
Jens
