This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Fix regex seqfault 5.18 regression
authorKarl Williamson <public@khwilliamson.com>
Mon, 1 Jul 2013 16:26:14 +0000 (10:26 -0600)
committerKarl Williamson <public@khwilliamson.com>
Mon, 1 Jul 2013 17:02:39 +0000 (11:02 -0600)
This segfault is a result of an optimization that can leave the
compilation in an inconsistent state.

  /f{0}/

doesn't match anything, and hence should be removable from the regex for
all f.  However,

  qr{(?&foo){0}(?<foo>)}

caused a segfault.  What was happening prior to this commit is that
(?&foo) refers to a named capture group further along in the regex.
The "{0}" caused the "(?&foo)" to be discarded prior to setting up the
pointers between the two related subexpressions; a segfault follows.

This commit removes the optimization, and should be suitable for a
maintenance release.

One might think that no one would be writing code like this, but this
example was distilled from machine-generated code in Regexp::Grammars.

Perhaps this optimization can be done, but the location I chose for
checking it was during parsing, which turns out to be premature.  It
would be better to do it in the optimization phase of regex compilation.
Another option would be to retain it where it was, but for it to operate
only on a limited set of nodes, such as EXACTish, which would have no
unintended consequences.  But that is for looking at in the future; the
important thing is to have a simple patch suitable for fixing this
regression in a maintenance release.

For the record, the code being reverted was mistakenly added by me in
commit 3018b823898645e44b8c37c70ac5c6302b031381, and wasn't even
mentioned in that commit message.  It should have had its own commit.

regcomp.c
t/re/pat_advanced.t

index d01f62a..4885c0b 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -9696,24 +9696,6 @@ S_regpiece(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth)
                 ret = reg_node(pRExC_state, OPFAIL);
                 return ret;
             }
-            else if (max == 0) {    /* replace {0} with a nothing node */
-                if (SIZE_ONLY) {
-                    RExC_size = PREVOPER(RExC_size) - regarglen[(U8)NOTHING];
-                }
-                else {
-                    RExC_emit = orig_emit;
-                }
-                ret = reg_node(pRExC_state, NOTHING);
-
-                /* But the quantifier includes any '?', the non-greedy
-                 * modifier, after the {}, [perl #118375]
-                 * Likewise the '+', the possessive modifier. They are mutually exclusive.
-                 */
-                if (RExC_parse < RExC_end && (*RExC_parse == '?' || *RExC_parse == '+') ) {
-                    nextchar(pRExC_state);
-                }
-                return ret;
-            }
 
        do_curly:
            if ((flags&SIMPLE)) {
index 4f72a9e..e6c5f39 100644 (file)
@@ -2317,6 +2317,14 @@ EOP
                             "Overlapping ranges in user-defined properties");
     }
 
+    { # Regexp:Grammars was broken:
+  # http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2013-06/msg01290.html
+        fresh_perl_like('use warnings; "abc" =~ qr{(?&foo){0}abc(?<foo>)}',
+                        'Quantifier unexpected on zero-length expression',
+                        "",
+                        'No segfault on qr{(?&foo){0}abc(?<foo>)}');
+    }
+
     # !!! NOTE that tests that aren't at all likely to crash perl should go
     # a ways above, above these last ones.