Skip to content

Commit

Permalink
[perl #123452] Fix crash with s/${<>{})//
Browse files Browse the repository at this point in the history
s/foo/bar/ tokenizes as something akin to subst("foo","bar") and the
resulting list op representing the contents of the parentheses is
passed to pmruntime as its expr argument.

If we have invalid code like s/${<>{})//, the bison parser will dis-
card invalid tokens until it finds something it can fall back to, in
an attempt to keep parsing (to report as many errors as possible).

In the process of discarding tokens, it may convert s/${<>{})//, which
the lexer emits like this:

    PMFUNC ( $ { THING(readline) { } ) , "" )

into this:

    PMFUNC ( $ { THING(readline) } ) , "" )

(or something similar).  So when the parser sees the first closing
parentheses, it decides it has a complete PMFUNC(...), and the expr
argument to pmruntime ends up being an rv2sv op (the ${...}), not
a list op.

pmruntime assumes it is a list op, and tries to access its op_last
field, to find the replacement part; but rv2sv has no op_last field,
so this reads past the end of the op struct, usually into the first
pointer in the next op slot, which itself is an opslot pointer, not an
op pointer, so things really screw up.

If we check that the arguments to subst are indeed a list op first
before trying to extract the replacement part, everything works.  We
get the syntax errors reported as expected, but no crash.
  • Loading branch information
Father Chrysostomos committed Jan 8, 2015
1 parent e736822 commit 08b999a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion op.c
Original file line number Diff line number Diff line change
Expand Up @@ -5473,7 +5473,10 @@ Perl_pmruntime(pTHX_ OP *o, OP *expr, bool isreg, I32 floor)

/* for s/// and tr///, last element in list is the replacement; pop it */

if (is_trans || o->op_type == OP_SUBST) {
/* If we have a syntax error causing tokens to be popped and the parser
to see PMFUNC '(' expr ')' with no commas in it; e.g., s/${<>{})//,
then expr will not be of type OP_LIST, there being no repl. */
if ((is_trans || o->op_type == OP_SUBST) && expr->op_type == OP_LIST) {
OP* kid;
repl = cLISTOPx(expr)->op_last;
kid = cLISTOPx(expr)->op_first;
Expand Down
5 changes: 4 additions & 1 deletion t/comp/parser.t
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ BEGIN {
chdir 't' if -d 't';
}

print "1..172\n";
print "1..173\n";

sub failed {
my ($got, $expected, $name) = @_;
Expand Down Expand Up @@ -524,6 +524,9 @@ $x[($_)];
# OPf_SPECIAL on OP_GV in subscript
$x[FILE1->[0]];
# Used to crash [perl #123452]
eval 's /${<>{}) //';
# Add new tests HERE (above this line)
# bug #74022: Loop on characters in \p{OtherIDContinue}
Expand Down

0 comments on commit 08b999a

Please sign in to comment.