Bug #132814 for Object-Pad: t/73mop-create-slot.t SEGV on perls 5.18 to 5.22

Fri Jun 12 19:56:38 2020 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

t/73mop-create-slot.t SEGV on perls 5.18 to 5.22

Often seen in cpantesters matrix. 5.16, and 5.22 onwards seems to be fine. Example: http://www.cpantesters.org/cpan/report/b6ad6476-8847-11ea-ab2c-73181f24ea8f An initial gdb attempt showed: ok 1 - $slotmeta->name ok 2 - Failure from ->add_slot undef ok 3 - Failure from ->add_slot on empty string ok 4 - Failure from ->add_slot without sigil ok 5 - Failure from ->add_slot duplicate Program received signal SIGSEGV, Segmentation fault. 0x0000000000460538 in S_cv_clone () (gdb) bt #0 0x0000000000460538 in S_cv_clone () #1 0x00000000004bbf18 in Perl_pp_anoncode () #2 0x000000000049eb73 in Perl_runops_standard () #3 0x00000000004356d7 in Perl_call_sv () #4 0x000000000043750e in Perl_call_list () #5 0x000000000041e90b in S_process_special_blocks () #6 0x000000000042fb24 in Perl_newATTRSUB_x () #7 0x000000000045fda2 in Perl_yyparse () #8 0x00000000004431dd in S_parse_recdescent_for_op () #9 0x000000000045cad4 in Perl_parse_stmtseq () #10 0x00007ffff7fca036 in my_keyword_plugin () from /home/leo/src/perl/Object-Pad/blib/arch/auto/Object/Pad/Pad.so #11 0x000000000044cfd4 in Perl_yylex () #12 0x000000000045da25 in Perl_yyparse () #13 0x000000000043af3d in perl_parse () #14 0x000000000041dde0 in main () -- Paul Evans

Sat Jun 13 10:13:36 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Probably some uninitialised data somewhere, because if it fails simply repeating the test without changes may well make it pass. Probably a good candidate for valgrind -- Paul Evans

Wed Jun 17 11:55:02 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Tut tut Perl... $ perl5.20.3 --valgrind -Mblib t/01method.t ==8178== Memcheck, a memory error detector ==8178== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==8178== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==8178== Command: /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3 -Mblib t/01method.t ==8178== ok 1 - method compiles OK ok 2 - warning from redeclared $self comes from correct line ok 3 - $p has refcount 1 initially ok 4 - $p->where ok 5 - $p has refcount 1 after method ok 6 - anon method ==8178== Invalid read of size 8 ==8178== at 0x46052B: S_cv_clone (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x4BBF17: Perl_pp_anoncode (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x49EB72: Perl_runops_standard (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x4354B0: Perl_call_sv (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x65CE752: injected_constructor (in /home/leo/src/perl/Object-Pad/blib/arch/auto/Object/Pad/Pad.so) ==8178== by 0x4A5D6F: Perl_pp_entersub (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x49EB72: Perl_runops_standard (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x43B9F8: perl_run (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x41DE64: main (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== Address 0x615dc48 is 8 bytes before a block of size 40 alloc'd ==8178== at 0x483677F: malloc (vg_replace_malloc.c:309) ==8178== by 0x4847D4: Perl_safesysmalloc (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x65DF981: indirect_map_store (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/lib/site_perl/5.20.3/x86_64-linux/auto/indirect/indirect.so) ==8178== by 0x65DFD8D: indirect_ck_method (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/lib/site_perl/5.20.3/x86_64-linux/auto/indirect/indirect.so) ==8178== by 0x424D06: Perl_newUNOP (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x45E498: Perl_yyparse (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x43AF3C: perl_parse (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== by 0x41DDDF: main (in /home/leo/perl5/perlbrew/perls/perl-5.20.3/bin/perl5.20.3) ==8178== Variable "$_genvalue" is not available at t/01method.t line 49. ok 7 - $obj->value from BUILD-generated anon method 1..7 At first glance this isn't even within any code I have written. -- Paul Evans

Wed Jun 17 11:55:03 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'new' to 'open'

Tue Nov 03 12:51:16 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

I've decided to look into this a bit to at least understand where it comes from. Even though it seems only to affect perl 5.20 and earlier, it would be good to work out in case it's indicative of a more serious underlying problem that's still there. -- Paul Evans

Tue Nov 03 16:46:31 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

By starting at: ok 1 - $slotmeta->name ok 2 - Failure from ->add_slot undef ok 3 - Failure from ->add_slot on empty string ok 4 - Failure from ->add_slot without sigil ok 5 - Failure from ->add_slot duplicate Variable "$slot" is not available at (eval 10) line 1. ... That message is generated by: pad.c: static void S_unavailable() I haven't been able to make the script crash with gdb running it, but valgrind helps as well: ok 5 - Failure from ->add_slot duplicate ==4097345== Invalid read of size 8 ==4097345== at 0x1F4E6E: S_cv_clone_pad (pad.c:2087) ==4097345== by 0x1F5D28: S_cv_clone (pad.c:2194) ==4097345== by 0x1F5F0A: Perl_cv_clone (pad.c:2231) ... ==4097345== Address 0x63ed260 is 8,272 bytes inside an unallocated block of size 1,986,000 in arena "client" ==4097345== Variable "$slot" is not available at (eval 10) line 1. Looking in 5.20.3's source at pad.c: 2087 if (!outpad || !(sv = outpad[PARENT_PAD_INDEX(namesv)]) 2088 || ( SvPADSTALE(sv) && !SvPAD_STATE(namesv) 2089 && (!outside || !CvDEPTH(outside))) ) { 2090 S_unavailable(aTHX_ namesv); 2091 sv = NULL; 2092 } 2093 else 2094 SvREFCNT_inc_simple_void_NN(sv); This would somehow seem to fit together. What I suspect is happening is that either `outpad` contains a silly junk value, or it's valid but not as big as the large index that `PARENT_PAD_INDEX(namesv)` is giving it. I have had issues with that macro before, it feels familiar... -- Paul Evans

Tue Nov 03 16:53:02 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Inserted code into pad.c just above that section: warn("D1: proto=%p cv=%p ix=%d outpad=%p PPI=%d\n", proto, cv, ix, outpad, PARENT_PAD_INDEX(namesv)); I now get output: ... ok 3 - Failure from ->add_slot on empty string D1: proto=63e6bc0 cv=623e988 ix=1 outpad=6400300 PPI=1 D1: proto=63828b8 cv=63f3828 ix=14 outpad=63dc460 PPI=1 ok 4 - Failure from ->add_slot without sigil D1: proto=63e6f20 cv=6402528 ix=1 outpad=6400300 PPI=1 D1: proto=63828b8 cv=6382870 ix=14 outpad=63dc460 PPI=1 ok 5 - Failure from ->add_slot duplicate D1: proto=6402d20 cv=6403038 ix=3 outpad=642a840 PPI=4600 ==4100688== Invalid read of size 8 ==4100688== at 0x1F502C: S_cv_clone_pad (pad.c:2089) ==4100688== by 0x1F5EE6: S_cv_clone (pad.c:2196) Looking at those values, it feels like outpad is correct, but PARENT_PAD_INDEX() is indeed giving a silly value. -- Paul Evans

Wed Nov 04 09:13:40 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

After much hunting, it turns out that the code was forgetting to clear PadnameOUTER() while patching up the PARENT_PAD_INDEX() fields, if the outside padname wasn't itself OUTER. Unclear why newer perls didn't get upset about it, but never mind... Fixed by attached patch. -- Paul Evans

Subject:

rt132814.patch

=== modified file 'hax/perl-additions.c.inc' --- old/hax/perl-additions.c.inc 2020-10-09 16:24:33 +0000 +++ new/hax/perl-additions.c.inc 2020-11-04 14:08:25 +0000 @@ -10,6 +10,13 @@ # define PadnameIsNULL(pn) (!(pn) || (pn) == &PL_sv_undef) #endif +#if HAVE_PERL_VERSION(5, 22, 0) +# define PadnameOUTER_off(pn) (PadnameFLAGS(pn) &= ~PADNAMEt_OUTER) +#else + /* PadnameOUTER is really the SvFAKE flag */ +# define PadnameOUTER_off(pn) SvFAKE_off(pn) +#endif + #define save_strndup(s, l) S_save_strndup(aTHX_ s, l) static char *S_save_strndup(pTHX_ char *s, STRLEN l) { === modified file 'lib/Object/Pad.xs' --- old/lib/Object/Pad.xs 2020-11-02 18:21:05 +0000 +++ new/lib/Object/Pad.xs 2020-11-04 14:08:25 +0000 @@ -2086,6 +2086,8 @@ PADNAME *outside_pn = PadnamelistARRAY(outside_pnl)[PARENT_PAD_INDEX(pn)]; PARENT_PAD_INDEX_set(pn, PARENT_PAD_INDEX(outside_pn)); + if(!PadnameOUTER(outside_pn)) + PadnameOUTER_off(pn); } CvOUTSIDE(PL_compcv) = CvOUTSIDE(outside);

Wed Nov 04 09:13:40 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'open' to 'patched'

Wed Nov 04 17:01:33 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Released in 0.34 -- Paul Evans

Wed Nov 04 17:01:33 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'resolved'

Wed Nov 04 17:01:33 2020 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.34 added