Bug #133190 for Object-Pad: Object::Pad 0.32 causes compiletime SEGV when extending as-yet-unsealed base class

Wed Aug 19 11:53:34 2020 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

Object::Pad 0.32 causes compiletime SEGV in Tickit/Widget/Tabbed.pm

Starting program: /usr/bin/perl -c lib/Tickit/Widget/Tabbed.pm [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Program received signal SIGSEGV, Segmentation fault. 0x00005555555a1967 in Perl_ck_svconst (my_perl=0x5555558aa2a0, o=0x555555ec8270) at op.c:13865 13865 op.c: No such file or directory. (gdb) bt #0 0x00005555555a1967 in Perl_ck_svconst (my_perl=0x5555558aa2a0, o=0x555555ec8270) at op.c:13865 #1 0x00007ffff7fb06c3 in ?? () from /usr/lib/x86_64-linux-gnu/perl5/5.30/auto/indirect/indirect.so #2 0x00007ffff7fc6b73 in S_generate_initslots_method (my_perl=<optimized out>, meta=0x555555e3cda0) at lib/Object/Pad.xs:632 #3 S_mop_class_seal (my_perl=<optimized out>, meta=0x555555e3cda0) at lib/Object/Pad.xs:1312 #4 0x000055555569462d in Perl_leave_scope (my_perl=my_perl@entry=0x5555558aa2a0, base=base@entry=175) at scope.c:1280 #5 0x00005555555ab01b in Perl_block_end (my_perl=my_perl@entry=0x5555558aa2a0, floor=175, seq=<optimized out>) at op.c:5253 #6 0x00005555555f62a9 in Perl_yyparse (my_perl=my_perl@entry=0x5555558aa2a0, gramtype=gramtype@entry=258) at perly.y:125 #7 0x0000555555698677 in S_doeval_compile (my_perl=my_perl@entry=0x5555558aa2a0, gimme=gimme@entry=2 '\002', outside=outside@entry=0x0, seq=<optimized out>, hh=hh@entry=0x0) at pp_ctl.c:3520 #8 0x000055555569e954 in S_require_file (sv=<optimized out>, my_perl=0x5555558aa2a0) at pp_ctl.c:4340 #9 Perl_pp_require (my_perl=0x5555558aa2a0) at pp_ctl.c:4364 #10 0x0000555555652d06 in Perl_runops_standard (my_perl=0x5555558aa2a0) at run.c:42 #11 0x00005555555be964 in Perl_call_sv (my_perl=my_perl@entry=0x5555558aa2a0, sv=sv@entry=0x555555dfc958, flags=flags@entry=13) at perl.c:3043 #12 0x00005555555c1518 in Perl_call_list (my_perl=my_perl@entry=0x5555558aa2a0, oldscope=oldscope@entry=2, paramList=0x5555558da128) at perl.c:5146 #13 0x000055555559e050 in S_process_special_blocks ( my_perl=my_perl@entry=0x5555558aa2a0, floor=floor@entry=90, fullname=<optimized out>, fullname@entry=0x5555558e4090 "BEGIN", cv=cv@entry=0x555555dfc958, gv=<optimized out>) at op.c:10471 #14 0x00005555555b6f5f in Perl_newATTRSUB_x (my_perl=my_perl@entry=0x5555558aa2a0, floor=floor@entry=90, o=<optimized out>, proto=<optimized out>, proto@entry=0x0, attrs=<optimized out>, attrs@entry=0x0, block=0x555555a2cf00, block@entry=0x555555a2cf80, o_is_gv=<optimized out>) at op.c:10396 #15 0x00005555555ba778 in Perl_utilize (my_perl=my_perl@entry=0x5555558aa2a0, aver=<optimized out>, floor=90, version=<optimized out>, idop=0x555555959378, arg=<optimized out>) at op.c:7592 #16 0x00005555555f6859 in Perl_yyparse (my_perl=my_perl@entry=0x5555558aa2a0, gramtype=gramtype@entry=258) at perly.y:335 #17 0x00005555555c59f3 in S_parse_body (xsinit=0x55555559c580 <xs_init>, env=0x0, my_perl=<optimized out>) at perl.c:2531 #18 perl_parse (my_perl=<optimized out>, xsinit=0x55555559c580 <xs_init>, argc=<optimized out>, argv=<optimized out>, env=0x0) at perl.c:1822 #19 0x000055555559c3c3 in main (argc=<optimized out>, argv=<optimized out>, env=<optimized out>) at perlmain.c:126 $ perlmodversion Object::Pad 0.32 leo@shy:~/src/perl/Tickit-Widget-Tabbed [bzr] $ bzr revno 174 -- Paul Evans

Wed Aug 19 11:54:42 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Works fine on 0.31: $ perlmodversion Object::Pad 0.31 $ perl -c lib/Tickit/Widget/Tabbed.pm lib/Tickit/Widget/Tabbed.pm syntax OK -- Paul Evans

Wed Aug 19 11:54:43 2020 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.31 added

Wed Aug 19 12:41:03 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

A `bzr bisect` reveals the particular commit which breaks it is -r317: revno: 317 committer: Paul "LeoNerd" Evans <leonerd@leonerd.org.uk> branch nick: Object-Pad timestamp: Wed 2020-07-01 15:24:52 +0100 message: Store the initslots CV in ClassMeta directly, avoiding having to use a method lookup to find it -- Paul Evans

Thu Sep 03 15:39:47 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Same fix as other case, where sometimes PL_parser is NULL. -- Paul Evans

Subject:

rt133190.patch

=== modified file 'lib/Object/Pad.xs' --- old/lib/Object/Pad.xs 2020-09-03 19:27:15 +0000 +++ new/lib/Object/Pad.xs 2020-09-03 19:38:58 +0000 @@ -977,6 +977,8 @@ meta->type = type; meta->name = SvREFCNT_inc(name); + HV *stash = meta->stash = gv_stashsv(name, GV_ADD); + meta->sealed = false; meta->offset = 0; meta->slots = newAV(); @@ -989,13 +991,25 @@ meta->buildblocks = NULL; meta->initslots = NULL; + if(!PL_parser) { + /* We need to generate just enough of a PL_parser to keep newSTATEOP() + * happy, otherwise it will SIGSEGV (RT133190) + */ + SAVEVPTR(PL_parser); + Newxz(PL_parser, 1, yy_parser); + SAVEFREEPV(PL_parser); + + PL_parser->copline = NOLINE; +#if HAVE_PERL_VERSION(5, 20, 0) + PL_parser->preambling = NOLINE; +#endif + } + meta->tmpcop = (COP *)newSTATEOP(0, NULL, NULL); CopFILE_set(meta->tmpcop, __FILE__); meta->methodscope = NULL; - HV *stash = meta->stash = gv_stashsv(name, GV_ADD); - AV *isa; { SV *isaname = newSVpvf("%" SVf "::ISA", name);

Thu Sep 03 15:39:48 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'new' to 'patched'

Thu Sep 03 15:42:01 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Oops, ignore that. Wrong bug. This one is still unsolved. -- Paul Evans

Thu Sep 03 15:42:02 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'open'

Tue Sep 15 11:10:37 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Further analysis: This bug happens when a subclass is sealed before its parent, meaning that supermeta->initslots is still NULL, so the superconstructor can't be captured. Possible mitigation may be to defer seal time of subclasses whose parents are still unsealed, and come back to them later on. Would require keeping a queue of deferred ones though. -- Paul Evans

Tue Sep 15 11:16:32 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Subject:

Object::Pad 0.32 causes compiletime SEGV when extending as-yet-unsealed base class

This small test case reproduces it: +# Extend before base class is sealed (RT133190) +class BaseClass { + has $_aslot; + + class SubClass extends BaseClass { } +} + +pass( 'Did not SEGV while compiling inner derived class' ); + -- Paul Evans

Tue Sep 15 11:16:54 2020 leonerd-cpan [...] leonerd.org.uk - Subject changed from 'Object::Pad 0.32 causes compiletime SEGV in Tickit/Widget/Tabbed.pm' to 'Object::Pad 0.32 causes compiletime SEGV when extending as-yet-unsealed base class'

Tue Sep 15 12:10:44 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Fixed. -- Paul Evans

Subject:

rt133190.patch

=== modified file 'lib/Object/Pad.xs' --- old/lib/Object/Pad.xs 2020-09-15 15:28:19 +0000 +++ new/lib/Object/Pad.xs 2020-09-15 15:30:05 +0000 @@ -307,6 +307,7 @@ SV *name; HV *stash; ClassMeta *supermeta; + AV *pending_submeta; /* NULL, or AV containing raw ClassMeta pointers to subclasses pending seal */ AV *implements; /* each elem is a raw pointer directly to a ClassMeta whose type == METATYPE_ROLE */ bool sealed; SLOTOFFSET offset; /* first slot index of this partial within its instance */ @@ -993,6 +994,7 @@ meta->repr = REPR_AUTOSELECT; meta->foreign_new = NULL; meta->supermeta = NULL; + meta->pending_submeta = NULL; meta->implements = newAV(); meta->buildblocks = NULL; meta->initslots = NULL; @@ -1313,6 +1315,16 @@ #define mop_class_seal(meta) S_mop_class_seal(aTHX_ meta) static void S_mop_class_seal(pTHX_ ClassMeta *meta) { + if(meta->supermeta && !meta->supermeta->sealed) { + /* Must defer sealing until superclass is sealed first + * (RT133190) + */ + if(!meta->supermeta->pending_submeta) + meta->supermeta->pending_submeta = newAV(); + av_push(meta->supermeta->pending_submeta, (SV *)meta); + return; + } + S_apply_roles(aTHX_ meta, meta); if(meta->type == METATYPE_CLASS) { @@ -1333,6 +1345,20 @@ S_generate_initslots_method(aTHX_ meta); meta->sealed = true; + + if(meta->pending_submeta) { + int i; + SV **arr = AvARRAY(meta->pending_submeta); + for(i = 0; i < av_count(meta->pending_submeta); i++) { + ClassMeta *submeta = (ClassMeta *)arr[i]; + arr[i] = &PL_sv_undef; + + mop_class_seal(submeta); + } + + SvREFCNT_dec(meta->pending_submeta); + meta->pending_submeta = NULL; + } } static XS(xsub_mop_class_seal) === modified file 't/05extends.t' --- old/t/05extends.t 2020-08-19 15:46:02 +0000 +++ new/t/05extends.t 2020-09-15 15:30:05 +0000 @@ -46,4 +46,18 @@ 'message from insufficient version' ); } +# Extend before base class is sealed (RT133190) +{ + class BaseClass { + has $_aslot; + + class SubClass extends BaseClass { + method one { 1 } + } + } + + pass( 'Did not SEGV while compiling inner derived class' ); + is( SubClass->new->one, 1, 'Inner derived subclass instances can be constructed' ); +} + done_testing;

Tue Sep 15 12:10:44 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'open' to 'patched'

Thu Sep 17 09:47:21 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Released in 0.33 -- Paul Evans

Thu Sep 17 09:47:21 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'resolved'

Thu Sep 17 09:47:21 2020 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.33 added

Thu Sep 17 09:47:21 2020 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.31 deleted