This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
PATCH: [perl #123539] regcomp.c node overrun/segfault
authorKarl Williamson <khw@cpan.org>
Tue, 6 Jan 2015 20:07:51 +0000 (13:07 -0700)
committerKarl Williamson <khw@cpan.org>
Tue, 6 Jan 2015 22:05:26 +0000 (15:05 -0700)
This is a minimal patch suitable for a maintenance release.  It extracts
the guts of reguni and REGC without the conditional they have.  The next
commit will do some refactoring to avoid branching and to make things
clearer.

This bug is due to the current two pass structure of the Perl regular
expression compiler.  The first pass attempts to do just enough work to
figure out how much space to malloc for the compiled pattern; and the
2nd pass actually fills in the details.  One problem with this design is
that in many cases quite a bit of work is required to figure out the
size, and this work is thrown away and redone in the second pass.
Another problem is that it is easy to forget to do enough work in the
sizing pass, and that is what happened with the blamed commit.  I
understand that there are plans (God speed) to change the compiler
design.

When not under /i matching, the size of a node that will match a
sequence of characters is just the number of bytes those characters take
up.  We have an easy way to calculate the number of bytes any code point
will occupy in UTF-8, and it's just 1 byte per code point for non-UTF-8.
So in the sizing pass, we don't actually have to figure out the
representation of the characters.  However under /i matching, we do.
First of all, matching of UTF-8 strings is done by replacing each
character of each string by its fold-case (function fc()) and then
comparing.  This is required by the nature of full Unicode matching
which is not 1-1.  If we do that replacement for the pattern at compile
time, we avoid having to do it over-and-over as pattern matching
backtracks at execution.  And because fc(x) may not occupy the same
number of bytes as x, and there is no easy way to know that size without
actually doing the fc(), we have to do the fold in the sizing pass.
Now, there are relatively few folds where sizeof(fc(x)) != sizeof(x), so
we could construct an exception table for those few cases where it is,
and look up through that.

But there is another reason that we have to fold in the sizing pass.
And that is because of the potential for multi-character folds being
split across regnodes.  The regular expression compiler generates
EXACTish regnodes for matching sequences of characters exactly or via
/i.  The limit for how many bytes in a sequence such a node can match is
255 because the length is stored in a U8.  If the pattern has a sequence
longer than that, it is split into two or more EXACTish nodes in a row.
(Actually, the compiler splits at a size much lower than that; I'm not
sure why, but then two adjoining nodes whose total sum length is at most
255 get joined later in the third, optimizing pass.)  Now consider,
matching the character U+FB03 LATIN SMALL LIGATURE FFI.  It matches the
sequence of the three characters "f f i".  Because of the design of the
regex pattern matching code, if these characters are such that the first
one or two are at the end of one EXACTish node, and the final two or one
are in another EXACTish node, then U+FB03 wrongly would not match them.
Matches can't cross node boundaries.  If the pattern were tweaked so all
three characters were in either the first or second node, then the match
would succeed.  And that is what the compiler does.  When it reaches the
node's size limit, and the final character is one that is a non-terminal
character in a multi-char fold, what's in the node is backed-off until
it ends with a character without this characteristic.  This has to be
done in the sizing pass, as we are repacking the nodes, which can affect
the size of the pattern, and we have to know what the folds are in order
to determine all this.

(We don't fold non-UTF-8 patterns.  This is for two reasons.  One is
that one character, the U+00B5 MICRO SIGN, folds to above-Latin1, and if
we folded it, we would have to change the pattern into UTF-8, and that
would slow everything down.  I've thought about adding a regnode type
for the much more common case of a sequence that doesn't have this
character in it, and which could hence be folded at compile time.  But
I've not been able to justify this because of the 2nd reason, which is
folds in this range are simple enough to be handled by an array lookup,
so folding is fast at runtime.)

Then there is the complication of matching under locale rules.  This bug
manifested itself only under /l matching.  We can't fold at pattern
compile time, because the folding rules won't be known until runtime.
This isn't a problem for non-UTF-8 locales, as all folds are 1-1, and so
there never will be a multi-char fold.  But there could be such folds in
a UTF-8 locale, so the regnodes have to be packed to work for that
eventuality.  The blamed commit did not do that, and because this issue
doesn't arise unless there is a string long enough to trigger the
problem, this wasn't found until now.  What is needed, and what this
commit does, is for the unfolded characters to be accumulated in both
passes.  The code that looks for potential multi-char fold issues
handles both folded and unfolded-inputs, so will work.

regcomp.c
t/re/pat.t

index 78c614d..82f23d3 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -12444,7 +12444,17 @@ tryagain:
                         && is_PROBLEMATIC_LOCALE_FOLD_cp(ender)))
                 {
                     if (UTF) {
-                        const STRLEN unilen = reguni(pRExC_state, ender, s);
+
+                        /* Normally, we don't need the representation of the
+                         * character in the sizing pass--just its size, but if
+                         * folding, we have to actually put the character out
+                         * even in the sizing pass, because the size could
+                         * change as we juggle things at the end of this loop
+                         * to avoid splitting a too-full node in the middle of
+                         * a potential multi-char fold [perl #123539] */
+                        const STRLEN unilen = (SIZE_ONLY && ! FOLD)
+                                               ? UNISKIP(ender)
+                                               : (uvchr_to_utf8((U8*)s, ender) - (U8*)s);
                         if (unilen > 0) {
                            s   += unilen;
                            len += unilen;
@@ -12457,6 +12467,10 @@ tryagain:
                          * cancel out the increment that follows */
                         len--;
                     }
+                    else if (FOLD) {
+                        /* See comment above for [perl #123539] */
+                        *(s++) = (char) ender;
+                    }
                     else {
                         REGC((char)ender, s++);
                     }
index e532054..ec68e6b 100644 (file)
@@ -22,7 +22,7 @@ BEGIN {
     skip_all_without_unicode_tables();
 }
 
-plan tests => 755;  # Update this when adding/deleting tests.
+plan tests => 757;  # Update this when adding/deleting tests.
 
 run_tests() unless caller;
 
@@ -1620,6 +1620,12 @@ EOP
         use utf8;
         like("ÿ", qr/[ÿ-ÿ]/, "\"ÿ\" should match [ÿ-ÿ]");
     }
+
+    {  # [perl #123539]
+        like("TffffffffffffTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT5TTTTTTTTTTTTTTTTTTTTTTTTT3TTgTTTTTTTTTTTTTTTTTTTTT2TTTTTTTTTTTTTTTTTTTTTTTHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHiHHHHHHHfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff&ffff", qr/TffffffffffffTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT5TTTTTTTTTTTTTTTTTTTTTTTTT3TTgTTTTTTTTTTTTTTTTTTTTT2TTTTTTTTTTTTTTTTTTTTTTTHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHiHHHHHHHfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff&ffff/il, "");
+        like("TffffffffffffT\x{100}TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT5TTTTTTTTTTTTTTTTTTTTTTTTT3TTgTTTTTTTTTTTTTTTTTTTTT2TTTTTTTTTTTTTTTTTTTTTTTHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHiHHHHHHHfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff&ffff", qr/TffffffffffffT\x{100}TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT5TTTTTTTTTTTTTTTTTTTTTTTTT3TTgTTTTTTTTTTTTTTTTTTTTT2TTTTTTTTTTTTTTTTTTTTTTTHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHiHHHHHHHfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff&ffff/il, "");
+    }
+
 } # End of sub run_tests
 
 1;