-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perl_op_free: Assertion `!(o->op_private & ~PL_op_private_valid[type])' failed (op.c:721) #14483
Comments
From @geeknikBuilt v5.21.9 (v5.21.8-237-g3c47da3) with the following command line: /Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j6 test-prep Bug found with AFL (http://lcamtuf.coredump.cx/afl) GDB output: Hello World. Program received signal SIGABRT, Aborted. Debian 7, kernel v3.2.63-2+deb7u2 x86_64, libc6 v2.13-38+deb7u7, GCC 4.9.2 |
From @geeknik |
From @geeknikBuilt v5.21.9 (v5.21.8-200-ga57d3d4) using the following command line: ./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j6 test-prep Bug found with AFL (http://lcamtuf.coredump.cx/afl) GDB output: Program received signal SIGABRT, Aborted. Test case: my($_);0=split Debian 7, kernel v3.2.63-2+deb7u2 x86_64, libc6 v2.13-38+deb7u7, GCC 4.9.2 |
From @hvdsOn Sun Feb 08 12:52:53 2015, brian.carpenter@gmail.com wrote:
The op is OP_PUSHRE, and the flag in question is OPpTARGET_MY being set in Perl_ck_match. If this is ok, I think the fix is as simple as the patch below followed by regen/opcode.pl. I can't tell if it is, though: pp_pushre supplies its return value via XPUSHs which is in neither the safe nor the unsafe lists in the docs for the flag in regen/op_private. Hugo --- a/regen/op_private |
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Tue Feb 10 16:05:15 2015, hv wrote:
Yes, that fix is right. The flag does nothing on pushre, but turning it off would be a waste of CPU cycles. It gets turned on because the first argument to split (whether explicit or not) is a match op (which gets the flag if a lexical $_ is in scope), but it gets converted into a pushre op. Ultimately, whether lexical $_ was seen becomes irrelevant.
That would only be a problem if it caused PL_opargs[OP_PUSHRE] & OA_TARGLEX to be true, which it does not. (Even then, I suspect the flag would still be harmless, because a lone pushre would never be on the rhs of an assignment.)
-- Father Chrysostomos |
From @hvds"Father Chrysostomos via RT" <perlbug-followup@perl.org> wrote: Thanks for the confirmation. I'll aim to add a test case and push Hugo |
From @hvdsThis case reduces to 'my($_); @x = split'; I'm going to merge it into [perl #123763] since fixes for both issues will need to take account of each other. Hugo |
The RT System itself - Status changed from 'new' to 'open' |
From @hvdsAh, it might be more complicated than that: rt123762 reduces to the near-identical 'my $_; @x = split', and is not fixed by this change. At the point OPpTARGET_MY is set in ck_match, we also set o->op_targ; later in pp_split if op_targ is set it is taken to be the pad offset for the array. I think this may have been introduced by: commit fd017c0 Optimise @lexarray = split... Hugo |
From @cpansproutOn Wed Feb 11 03:02:11 2015, hv wrote:
Yes, that is likely. We probably need to free the target, if any, in ck_split. -- Father Chrysostomos |
From @cpansproutOn Tue Feb 10 18:37:31 2015, hv wrote:
I have applied your patch as 26f4cc1 (thank you) and added a test in 179b3fa. -- Father Chrysostomos |
From @cpansproutOn Wed Feb 11 09:13:38 2015, sprout wrote:
And this I have done in 55b3980. -- Father Chrysostomos |
@cpansprout - Status changed from 'open' to 'pending release' |
From @khwilliamsonThanks for submitting this ticket The issue should be resolved with the release today of Perl v5.22, available at http://www.perl.org/get.html -- |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#123763 (status was 'resolved')
Searchable as RT123763$
The text was updated successfully, but these errors were encountered: