Fix for part of the problem mentioned in #725149 by Greg Chapman.

This problem is related to a wrong behavior from mark_save/restore(),
which don't restore the mark_stack_base before restoring the marks.
Greg's suggestion was to change the asserts, which happen to be
the only recursive ops that can continue the loop, but the problem would
happen to any operation with the same behavior. So, rather than
hardcoding this into asserts, I have changed mark_save/restore() to
always restore the stackbase before restoring the marks.

Both solutions should fix these two cases, presented by Greg:

>>> re.match('(a)(?:(?=(b)*)c)*', 'abb').groups()
('b', None)
>>> re.match('(a)((?!(b)*))*', 'abb').groups()
('b', None, None)

The rest of the bug and patch in #725149 must be discussed further.

diff --git a/Lib/test/test_re.py b/Lib/test/test_re.py
index 7ba9a1b1f92fcd7e785b1fa17083e4c4e24c2c41..6eebedf430a19ffc305281cbc70700dc88c45235 100644 (file)
--- a/Lib/test/test_re.py
+++ b/Lib/test/test_re.py
@@ -295,6 +295,13 @@ class ReTests(unittest.TestCase):
         self.assertEqual(re.match('^((a)c|[ab])*?c', 'abc').groups(),
                          ('b', None))
 
+    def test_bug_725149(self):
+        # mark_stack_base restoring before restoring marks
+        self.assertEqual(re.match('(a)(?:(?=(b)*)c)*', 'abb').groups(),
+                         ('a', None))
+        self.assertEqual(re.match('(a)((?!(b)*))*', 'abb').groups(),
+                         ('a', None, None))
+
     def test_finditer(self):
         iter = re.finditer(r":+", "a:b::c:::d")
         self.assertEqual([item.group(0) for item in iter],

diff --git a/Modules/_sre.c b/Modules/_sre.c
index b9e1827f831806d89709affdb837e06df83e1b31..4f040f1a921eef8eac74a38e7624c65b650880f7 100644 (file)
--- a/Modules/_sre.c
+++ b/Modules/_sre.c
@@ -279,7 +279,7 @@ mark_fini(SRE_STATE* state)
 }
 
 static int
-mark_save(SRE_STATE* state, int lo, int hi)
+mark_save(SRE_STATE* state, int lo, int hi, int *mark_stack_base)
 {
     void* stack;
     int size;
@@ -323,11 +323,13 @@ mark_save(SRE_STATE* state, int lo, int hi)
 
     state->mark_stack_base += size;
 
+    *mark_stack_base = state->mark_stack_base;
+
     return 0;
 }
 
 static int
-mark_restore(SRE_STATE* state, int lo, int hi)
+mark_restore(SRE_STATE* state, int lo, int hi, int *mark_stack_base)
 {
     int size;
 
@@ -336,7 +338,7 @@ mark_restore(SRE_STATE* state, int lo, int hi)
 
     size = (hi - lo) + 1;
 
-    state->mark_stack_base -= size;
+    state->mark_stack_base = *mark_stack_base - size;
 
     TRACE(("copy %d:%d from %d\n", lo, hi, state->mark_stack_base));
 
@@ -712,7 +714,7 @@ SRE_MATCH(SRE_STATE* state, SRE_CODE* pattern, int level)
     SRE_CHAR* ptr = state->ptr;
     int i, count;
     SRE_REPEAT* rp;
-    int lastmark, lastindex;
+    int lastmark, lastindex, mark_stack_base;
     SRE_CODE chr;
 
     SRE_REPEAT rep; /* FIXME: <fl> allocate in STATE instead */
@@ -948,7 +950,7 @@ SRE_MATCH(SRE_STATE* state, SRE_CODE* pattern, int level)
                     (ptr >= end || !SRE_CHARSET(pattern + 3, (SRE_CODE) *ptr)))
                     continue;
                 if (state->repeat) {
-                    i = mark_save(state, 0, lastmark);
+                    i = mark_save(state, 0, lastmark, &mark_stack_base);
                     if (i < 0)
                         return i;
                 }
@@ -957,7 +959,7 @@ SRE_MATCH(SRE_STATE* state, SRE_CODE* pattern, int level)
                 if (i)
                     return i;
                 if (state->repeat) {
-                    i = mark_restore(state, 0, lastmark);
+                    i = mark_restore(state, 0, lastmark, &mark_stack_base);
                     if (i < 0)
                         return i;
                 }
@@ -1157,14 +1159,14 @@ SRE_MATCH(SRE_STATE* state, SRE_CODE* pattern, int level)
                 /* we may have enough matches, but if we can
                    match another item, do so */
                 rp->count = count;
-                i = mark_save(state, 0, lastmark);
+                i = mark_save(state, 0, lastmark, &mark_stack_base);
                 if (i < 0)
                     return i;
                 /* RECURSIVE */
                 i = SRE_MATCH(state, rp->pattern + 3, level + 1);
                 if (i)
                     return i;
-                i = mark_restore(state, 0, lastmark);
+                i = mark_restore(state, 0, lastmark, &mark_stack_base);
                 LASTMARK_RESTORE();
                 if (i < 0)
                     return i;