fix COW match capture optimisation
authorDavid Mitchell <davem@iabyn.com>
Sat, 20 Jul 2013 15:16:10 +0000 (16:16 +0100)
committerDavid Mitchell <davem@iabyn.com>
Sun, 28 Jul 2013 09:33:40 +0000 (10:33 +0100)
When an SV matches, a new SV is created which is a COW copy of the original
SV, and stored in prog->saved_copy, then prog->subbeg is set to point to
the (shared) PVX buffer.

Earlier in this branch I introduced an optimisation that skipped freeing
the old saved_copy and creating a new COW SV each time, if the old
saved_copy SV was already a shared copy of the SV being matched.

So far so good, except that I implemented it wrongly: if non-COW
matches (which malloc() subbeg) are interspersed with COW matches,
then the subbeg of the COW and the malloced subbeg get mixed up and
AddressSanitizer throws a wobbly.

The fix is simple: in the optimised branch, we still need to free subbeg
if RXp_MATCH_COPIED is true, then reassign it.

regexec.c
t/re/pat.t

index 35851f6..9015f7d 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -2068,19 +2068,29 @@ S_reg_set_capture_string(pTHX_ REGEXP * const rx,
                               "Copy on write: regexp capture, type %d\n",
                               (int) SvTYPE(sv));
             }
-            /* skip creating new COW SV if a valid one already exists */
-            if (! (    prog->saved_copy
-                    && SvIsCOW(sv)
-                    && SvPOKp(sv)
-                    && SvIsCOW(prog->saved_copy)
-                    && SvPOKp(prog->saved_copy)
-                    && SvPVX(sv) == SvPVX(prog->saved_copy)))
+            /* Create a new COW SV to share the match string and store
+             * in saved_copy, unless the current COW SV in saved_copy
+             * is valid and suitable for our purpose */
+            if ((   prog->saved_copy
+                 && SvIsCOW(prog->saved_copy)
+                 && SvPOKp(prog->saved_copy)
+                 && SvIsCOW(sv)
+                 && SvPOKp(sv)
+                 && SvPVX(sv) == SvPVX(prog->saved_copy)))
             {
+                /* just reuse saved_copy SV */
+                if (RXp_MATCH_COPIED(prog)) {
+                    Safefree(prog->subbeg);
+                    RXp_MATCH_COPIED_off(prog);
+                }
+            }
+            else {
+                /* create new COW SV to share string */
                 RX_MATCH_COPY_FREE(rx);
                 prog->saved_copy = sv_setsv_cow(prog->saved_copy, sv);
-                prog->subbeg = (char *)SvPVX_const(prog->saved_copy);
-                assert (SvPOKp(prog->saved_copy));
             }
+            prog->subbeg = (char *)SvPVX_const(prog->saved_copy);
+            assert (SvPOKp(prog->saved_copy));
             prog->sublen  = strend - strbeg;
             prog->suboffset = 0;
             prog->subcoffset = 0;
index a892490..020068c 100644 (file)
@@ -20,7 +20,7 @@ BEGIN {
     require './test.pl';
 }
 
-plan tests => 689;  # Update this when adding/deleting tests.
+plan tests => 694;  # Update this when adding/deleting tests.
 
 run_tests() unless caller;
 
@@ -1435,6 +1435,20 @@ EOP
         }
     }
 
+    # this mixture of readonly (not COWable) and COWable strings
+    # messed up the capture buffers under COW. The actual test results
+    # are incidental; the issue is was an AddressSanitizer failure
+    {
+       my $c ='AB';
+       my $res = '';
+       for ($c, 'C', $c, 'DE') {
+           ok(/(.)/, "COWable match");
+           $res .= $1;
+       }
+       is($res, "ACAD");
+    }
+
+
 } # End of sub run_tests
 
 1;