multiconcat: /$a(?{ $b . "c" })/ could crash
authorDavid Mitchell <davem@iabyn.com>
Mon, 19 Feb 2018 21:32:36 +0000 (21:32 +0000)
committerDavid Mitchell <davem@iabyn.com>
Mon, 19 Feb 2018 22:06:49 +0000 (22:06 +0000)
commit8327fe931a244c33965f30d2ba4bbe7248016951
tree4be98d26838e5f349e32dcb885052f6400547372
parent789a38b60994f42b44e183fe51a1c5e78fe2ac9e
multiconcat: /$a(?{  $b . "c" })/ could crash

RT #132772

Due to the weird order in which codeblocks within patterns are optimised,
it was possible for the OP_CONCAT -> OP_MULTICONCAT optimisation to leave
freed ops in the op execution chain, leading to assertion failures or
crashes.

In particular, optimize_optree() needs to always be called before
CALL_PEEP(), otherwise the individual subtrees which make up the two
children of a concat op may not have the head of subtree as the last op in
the subtree's op_next chain: in the subtree

    RV2SV
      |
     GV

this subtree gets peephole-optimised to

    ex-RV2SV
      |
     GVSV

and GVSV->op_next no longer points to the ex-RV2SV but rather directly
to RV2SV->op_next. But S_maybe_multiconcat() assumes that the head of each
subtree is the last op to be executed: It updates RV2SV->op_next when
reorganising the optree, but leaves GVSV->op_next possibly pointing at a
freed op.

This commit provides a minimal fix by unconditionally calling
optimize_optree() on each code block in Perl_pmruntime(). This may
mean that optimize_optree() may be run against the same code block again
later, but apart from the slight inefficiency, this should be harmless.

A more general fix will be applied post 5.28.0 release.
op.c
t/re/pat_re_eval.t