Assertion failed: S_finalize_op (op.c:2562)

p5pRT commented 9 years ago

Migrated from rt.perl.org#126170 (status was 'open')

Searchable as RT126170$

p5pRT commented 9 years ago

From @geeknik

Fuzzing perl v5.23.4 (v5.23.3-7-ge120c24) with AFL found the following assertion failure:

~/perl/perl -e 'BEGIN():o'

Prototype mismatch: sub main::BEGIN () vs none at -e line 1. perl: op.c:2562: S_finalize_op: Assertion `has_last || family == (1 \<\< 8) || family == (15 \<\< 8) || family == (3 \<\< 8) || family == (11 \<\< 8) || family == (12 \<\< 8) || family == (13 \<\< 8) || family == (14 \<\< 8) || type == OP_SASSIGN || type == OP_CUSTOM || type == OP_NULL' failed. Aborted

gdb output: Prototype mismatch: sub main::BEGIN () vs none at test00 line 1. perl: op.c:2562: S_finalize_op: Assertion `has_last || family == (1 \<\< 8) || family == (15 \<\< 8) || family == (3 \<\< 8) || family == (11 \<\< 8) || family == (12 \<\< 8) || family == (13 \<\< 8) || family == (14 \<\< 8) || type == OP_SASSIGN || type == OP_CUSTOM || type == OP_NULL' failed.

Program received signal SIGABRT\, Aborted. 0x00007ffff6d90165 in *__GI_raise (sig=\) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory. (gdb) bt #0 0x00007ffff6d90165 in *__GI_raise (sig=\) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x00007ffff6d933e0 in *__GI_abort () at abort.c:92 #2 0x00007ffff6d89311 in *__GI___assert_fail ( assertion=0xef41c8 "has_last || family == (1 \<\< 8) || family == (15 \<\< 8) || family == (3 \<\< 8) || family == (11 \<\< 8) || family == (12 \<\< 8) || family == (13 \<\< 8) || family == (14 \<\< 8) || type == OP_SASSIGN || type =="...\, file=\\, line=2562\, function=0xf08387 "S_finalize_op") at assert.c:81 #3 0x0000000000443cd1 in S_finalize_op () at op.c:2549 #4 0x0000000000443f90 in S_finalize_op () at op.c:2579 #5 0x0000000000443f90 in S_finalize_op () at op.c:2579 #6 0x0000000000443f90 in S_finalize_op () at op.c:2579 #7 0x00000000004e0432 in Perl_newATTRSUB_x () at op.c:2381 #8 0x00000000004f5459 in Perl_utilize () at op.c:6041 #9 0x00000000004f7a14 in Perl_load_module.constprop.43 () at op.c:6188 #10 0x00000000004db764 in Perl_newATTRSUB_x () at op.c:8631 #11 0x000000000067332c in Perl_yyparse () #12 0x000000000053c005 in S_parse_body () at perl.c:2304 #13 0x0000000000543d83 in perl_parse () at perl.c:1634 #14 0x000000000042c6f8 in main () at perlmain.c:114

p5pRT commented 9 years ago

From @iabyn

On Thu\, Sep 24\, 2015 at 07:41:54PM -0700\, Brian Carpenter wrote:

Fuzzing perl v5.23.4 (v5.23.3-7-ge120c24) with AFL found the following assertion failure:

~/perl/perl -e 'BEGIN():o'

Prototype mismatch: sub main::BEGIN () vs none at -e line 1. perl: op.c:2562: S_finalize_op: Assertion `has_last || family == (1 \<\< 8) || family == (15 \<\< 8) || family == (3 \<\< 8) || family == (11 \<\< 8) || family == (12 \<\< 8) || family == (13 \<\< 8) || family == (14 \<\< 8) || type == OP_SASSIGN || type == OP_CUSTOM || type == OP_NULL' failed.

This is occurring basically because toward the end of compiling BEGIN\, its tries to apply the attribute "o"\, which triggers an attempt to "use attributes"\, which results in faking up a BEGIN block\, which when its finished being compiled\, causes the 'old' BEGIN to be freed\, which triggers freeing any unattached ops associated with the old BEGIN's compilation\, which prematurely frees the ops used to construct the import() call\, which eventually leads to the assert failure.

This only happens because the attributue application happens at a specific point during the BEGIN's compilation\, where is hasn't yet had the root of its optree attached to CvROOT. When it's subsequently freed\, it looks to cv_undef_flags() like a sub compilation that's errored out\, and thus whose op slab needs freeing\, freeing up any orphaned ops. This is why

BEGIN { BEGIN {} } and BEGIN {} BEGIN {}

don't trigger the same issue.

The whole mess in newATTRSUB_x() is beyond my understanding. I think it might just be easier to croak if someone tries to apply attributes to a BEGIN block (or any other special BLOCK?)

Can anyone think or a reason why special blocks should be allowed attributes\, or whether such usage exists in the wild?

-- This email is confidential\, and now that you have read it you are legally obliged to shoot yourself. Or shoot a lawyer\, if you prefer. If you have received this email in error\, place it in its original wrapping and return for a full refund. By opening this email\, you accept that Elvis lives.

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From @ap

* Dave Mitchell \davem@iabyn\.com [2015-10-08 16:55]:

or whether such usage exists in the wild?

Fiddling around with grep.cpan.me I found no use on CPAN whatsoever.

There is some very minor use of `sub BEGIN` and some even more minor use of the empty prototype on BEGIN blocks\, but I did not find even so much as an empty attribute list on any of them\, let alone actual attributes.

Can anyone think or a reason why special blocks should be allowed attributes

I think it should be an error. But that’s in a vacuum. It has been possible to write them\, for a long time\, so the question is whether we gain anything but outlawing them now. The obvious answer is we fix the assert crash. But wait\, there’s an assert crash\, so *can* this syntax even work at all? If not\, then we can trivially outlaw it because it doesn’t work and can’t be used anyway. If yes\, the question is again\, given the circumstances in which it works\, their likelihood\, etc\, what do we gain from outlawing it?

My expectation is that “just kill it” is not particularly questionable\, but I don’t feel I am firm enough on the answers to actually take that stance.

Regards\, -- Aristotle Pagaltzis // \<http://plasmasturm.org/>

p5pRT commented 8 years ago

From @arc

Aristotle Pagaltzis \pagaltzis@gmx\.de wrote:

* Dave Mitchell \davem@iabyn\.com [2015-10-08 16:55]:

Can anyone think or a reason why special blocks should be allowed attributes

I think it should be an error. But that’s in a vacuum. It has been possible to write them\, for a long time\, so the question is whether we gain anything but outlawing them now. The obvious answer is we fix the assert crash. But wait\, there’s an assert crash\, so *can* this syntax even work at all?

This attempt:

sub MODIFY_CODE_ATTRIBUTES { () } sub BEGIN :Foo {}

does nothing (while emitting no warnings) under 5.6 through 5.14. As of 5.16\, it says:

Subroutine BEGIN redefined at -e line 1. Attempt to free unreferenced scalar: SV 0xdeadbeef at -e line 1.

but the code still executes. (I haven't tried to work out how much of a problem the "unreferenced scalar" error is.)

Removing the "sub" keyword from the attribute-attached BEGIN:

sub MODIFY_CODE_ATTRIBUTES { () } BEGIN :Foo {}

produces this error (or a reworded equivalent as of 5.18) on all Perls from 5.6 onwards:

Can't call method "Foo" on an undefined value at -e line 1.

because it's actually being parsed as the rough equivalent of

BEGIN: (do {})->Foo

— that is\, a statement with the label "BEGIN"\, in which the "Foo" method is invoked (using indirect-object notation) on the result of an empty block.

To summarise: in the past we've had the ability to attach attributes to "sub BEGIN"\, but nobody noticed when we apparently-accidentally broke some aspects of it in 5.16; and we've never had the ability to attach attributes to "BEGIN" without "sub".

If not\, then we can trivially outlaw it because it doesn’t work and can’t be used anyway.

It seems to me that outlawing it would indeed be safe. But we presumably need tests that "BEGIN : stuff" treats the stuff as a labelled statement.

-- Aaron Crane ** http://aaroncrane.co.uk/

Perl / perl5