Skip to content
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

Assertion failed: S_finalize_op (op.c:2562) #14927

Open
p5pRT opened this issue Sep 25, 2015 · 5 comments
Open

Assertion failed: S_finalize_op (op.c:2562) #14927

p5pRT opened this issue Sep 25, 2015 · 5 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 25, 2015

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

Searchable as RT126170$

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2015

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=<optimized out>)
  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=<optimized out>)
  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=<optimized out>, 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
Copy link
Author

p5pRT commented Oct 8, 2015

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
Copy link
Author

p5pRT commented Oct 8, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Oct 9, 2015

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
Copy link
Author

p5pRT commented Nov 1, 2015

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/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants