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

Perl_ck_refassign: Assertion `left->op_type == OP_SREFGEN' failed (op.c:10498) #15285

Closed
p5pRT opened this issue Apr 21, 2016 · 7 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Apr 21, 2016

Migrated from rt.perl.org#127952 (status was 'resolved')

Searchable as RT127952$

@p5pRT
Copy link
Author

p5pRT commented Apr 21, 2016

From @geeknik

perl -e '0,!n||!\r=0' triggers an assertion failure in Perl v5.24.0-RC1-2-gde1d2c7. This bug was found with American Fuzzy Lop.

Program received signal SIGABRT, Aborted.
0x00007ffff6d8e125 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 0x00007ffff6d8e125 in *__GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c​:64
#1 0x00007ffff6d913a0 in *__GI_abort () at abort.c​:92
#2 0x00007ffff6d87311 in *__GI___assert_fail (
  assertion=assertion@​entry=0xebfb70 "left->op_type == OP_SREFGEN", file=<optimized out>,
  file@​entry=0xf91722 "op.c", line=line@​entry=10498,
  function=function@​entry=0xed4720 "Perl_ck_refassign") at assert.c​:81
#3 0x00000000004a5869 in Perl_ck_refassign (o=0x120b060) at op.c​:10498
#4 0x000000000047c944 in Perl_newBINOP (type=type@​entry=388, flags=flags@​entry=64, first=0x120aa08,
  last=last@​entry=0x120a988) at op.c​:5016
#5 0x00000000004996ef in Perl_newASSIGNOP (flags=flags@​entry=64, left=0x120a988, optype=0,
  right=0x120aa08) at op.c​:6518
#6 0x00000000006608b5 in Perl_yyparse (gramtype=gramtype@​entry=258) at perly.y​:781
#7 0x0000000000531035 in S_parse_body (env=env@​entry=0x0, xsinit=xsinit@​entry=0x42ec20 <xs_init>)
  at perl.c​:2331
#8 0x0000000000537147 in perl_parse (my_perl=<optimized out>, xsinit=xsinit@​entry=0x42ec20 <xs_init>,
  argc=<optimized out>, argv=<optimized out>, env=env@​entry=0x0) at perl.c​:1650
#9 0x000000000042e838 in main (argc=3, argv=0x7fffffffe658, env=0x7fffffffe678) at perlmain.c​:114

Perl v5.14.2 fails with this message​:
Can't modify single ref constructor in scalar assignment at -e line 1, at EOF
Execution of -e aborted due to compilation errors.

@p5pRT
Copy link
Author

p5pRT commented Apr 21, 2016

From zefram@fysh.org

Brian Carpenter wrote​:

perl -e '0,!n||!\r=0' triggers an assertion failure

The initial "0," is not required, but all of the rest is. The assertion
is that the lhs of the assignment is an srefgen op, and it's found to
actually be an ex-not op whose child is an srefgen op. Something goes
awry in optimising that lhs​: that not op can't be validly optimised out.
The same bug can be seen when using the same expression in rvalue context​:

$ perl -le '$a = !n||!\r; print $a || "false"'
SCALAR(0xc8eb08)

This clearly should have printed "false". Nearby expressions behave
as expected​:

$ perl -le '$a = !"n"||!\r; print $a || "false"'
false
$ perl -le '$a = !n||!\"r"; print $a || "false"'
false
$ perl -le '$a = !!!n||!\r; print $a || "false"'
false
$ perl -le '$a = !n||\r; print $a || "false"'
SCALAR(0x246fb08)
$ perl -le '$a = n||!\r; print $a || "false"'
n
$ perl -le '$a = n||\r; print $a || "false"'
n

-zefram

@p5pRT
Copy link
Author

p5pRT commented Apr 21, 2016

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

@p5pRT
Copy link
Author

p5pRT commented May 15, 2016

From @arc

Zefram <zefram@​fysh.org> wrote​:

Brian Carpenter wrote​:

perl -e '0,!n||!\r=0' triggers an assertion failure

The initial "0," is not required, but all of the rest is. The assertion
is that the lhs of the assignment is an srefgen op, and it's found to
actually be an ex-not op whose child is an srefgen op. Something goes
awry in optimising that lhs​: that not op can't be validly optimised out.
The same bug can be seen when using the same expression in rvalue context​:

$ perl -le '$a = !n||!\r; print $a || "false"'
SCALAR(0xc8eb08)

This clearly should have printed "false". Nearby expressions behave
as expected​:

$ perl -le '$a = !"n"||!\r; print $a || "false"'
false
$ perl -le '$a = !n||!\"r"; print $a || "false"'
false
$ perl -le '$a = !!!n||!\r; print $a || "false"'
false
$ perl -le '$a = !n||\r; print $a || "false"'
SCALAR(0x246fb08)
$ perl -le '$a = n||!\r; print $a || "false"'
n
$ perl -le '$a = n||\r; print $a || "false"'
n

Thanks — your analysis there let me work out what was going on here. Other incorrect cases include​:

$ perl -le 'print scalar( !r || !n )'
n
$ perl -le 'print scalar( !r && !n )'
r
$ perl -le 'print scalar( !do { "r" } || !n )'
n
$ perl -le 'print scalar( !do { "r" } && !n )'
r

This bug is fixed in blead by f15d058​:

[perl #127952] misoptimization for negated constant-ish on lhs of logop

Negations were being incorrectly deleted from the op tree for an OP_AND or
OP_OR (corresponding to Perl code with any of `&& || and or`, or postfix
"if" or "unless") whose left-hand side looks like "!BAREWORD" or "!do {
'const' }" and whose right-hand side is a non-constant-foldable negation.

The symptom in the reported case was an assertion failure in ck_refassign
for an srefgen op, caused by such an OP_NOT having been nulled. But other
cases exist that instead yielded incorrect results.

The underlying cause is that two optimisations in S_new_logop() were
partially interfering with each other. One of those attempts to optimise
code like "!$x && !$y" to the equivalent of "!($x || $y)", saving a
negation op; this is valid by De Morgan's laws. If it detects code of
this form, it nulls out the negations on each side of the "&&", and makes
a note to wrap the op it generates inside a new OP_NOT.

The other optimisation looks at the left-hand arm, and if it's a constant at
compile time, avoids the entire logop in favour of directly evaluating the
lhs or rhs as appropriate, and eliding whichever arm is no longer needed.
This optimisation is important for code like this​:

  use constant DEBUG => …;
  print_debug_output() if DEBUG;

because it allows the entire statement to be eliminated when DEBUG is false.

When both conditions were true simultaneously, the De Morgan optimisation
was applied before the constant-based arm elision. But the arm elision
involved returning early from S_new_logop(), so the code later in that
function that wraps the generated op in a new OP_NOT never had a chance to
run. This meant that "!X && !Y" when X is constant was being compiled as if
it were in fact "X || Y", which is clearly incorrect.

This is, however, a very rare situation​: it requires the lhs to be an OP_NOT
that dominates an OP_CONST (possibly with some intervening OP_LINESEQ or
similar). But OP_NOT is constant-foldable, so that doesn't normally happen.
The two ways for it to happen are​:

- The constant is a bareword (since even though barewords are constants,
  they don't currently participate in constant folding)

- The constant is hidden inside one or more layers of do{} (since that
  serves as a barrier to constant folding, but the arm-elision optimisation
  is smart enough to search recursively through the optree for the real
  constant)

The fix is much simpler than the explanation​: apply the optimisations in the
opposite order, so that when arm elision returns early, the negation ops
haven't yet been nulled.

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

@p5pRT
Copy link
Author

p5pRT commented May 15, 2016

@arc - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

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

1 participant