scope.c - rework SSGROW() and SSCHECK() macros and undelying functions

regexec.c

@@ -262,7 +262,8 @@ S_regcppush(pTHX_ const regexp *rex, I32 parenfloor, U32 maxopenparen _pDEPTH)
             );
     );
 
-    SSCHECK(total_elems + REGCP_FRAME_ELEMS);
+    SSGROW(total_elems + REGCP_FRAME_ELEMS);
+    assert((IV)PL_savestack_max > (IV)(total_elems + REGCP_FRAME_ELEMS));
 
     /* memcpy the offs inside the stack - it's faster than for loop */
     memcpy(&PL_savestack[PL_savestack_ix], rex->offs + parenfloor + 1, paren_bytes_to_push);

scope.c

@@ -63,10 +63,10 @@ Perl_stack_grow(pTHX_ SV **sp, SV **p, SSize_t n)
     return PL_stack_sp;
 }
 
-#ifndef STRESS_REALLOC
-#define GROW(old) ((old) * 3 / 2)
-#else
+#ifdef STRESS_REALLOC
 #define GROW(old) ((old) + 1)
+#else
+#define GROW(old) ((old) * 3 / 2)
 #endif
 
 PERL_SI *
@@ -166,25 +166,44 @@ Perl_markstack_grow(pTHX)
 void
 Perl_savestack_grow(pTHX)
 {
-    IV new_max;
-#ifdef STRESS_REALLOC
-    new_max = PL_savestack_max + SS_MAXPUSH;
-#else
-    new_max = GROW(PL_savestack_max);
-#endif
-    /* Note that we allocate SS_MAXPUSH slots higher than ss_max
-     * so that SS_ADD_END(), SSGROW() etc can do a simper check */
-    Renew(PL_savestack, new_max + SS_MAXPUSH, ANY);
-    PL_savestack_max = new_max;
+    const I32 by = PL_savestack_max - PL_savestack_ix;
+    Perl_savestack_grow_cnt(aTHX_ by);
 }
 
 void
 Perl_savestack_grow_cnt(pTHX_ I32 need)
 {
-    const IV new_max = PL_savestack_ix + need;
-    /* Note that we allocate SS_MAXPUSH slots higher than ss_max
-     * so that SS_ADD_END(), SSGROW() etc can do a simper check */
-    Renew(PL_savestack, new_max + SS_MAXPUSH, ANY);
+    /* NOTE: PL_savestack_max and PL_savestack_ix are I32.
+     *
+     * This makes sense when you consider that having I32_MAX items on
+     * the stack would be quite large.
+     *
+     * However, we use IV here so that we can detect if the new requested
+     * amount is larger than I32_MAX.
+     */
+    const IV new_floor = PL_savestack_max + need; /* what we need */
+    /* the GROW() macro normally does scales by 1.5 but under
+     * STRESS_REALLOC it simply adds 1 */
+    IV new_max         = GROW(new_floor); /* and some extra */
+
+    /* the new_max < PL_savestack_max is for cases where IV is I32
+     * and we have rolled over from I32_MAX to a small value */
+    if (new_max > I32_MAX || new_max < PL_savestack_max) {
+        if (new_floor > I32_MAX || new_floor < PL_savestack_max) {
+            Perl_croak(aTHX_ "panic: savestack overflows I32_MAX");
+        }
+        new_max = new_floor;
+    }
+
+    /* Note that we add an additional SS_MAXPUSH slots on top of
+     * PL_savestack_max so that SS_ADD_END(), SSGROW() etc can do
+     * a simper check and if necessary realloc *after* apparently
+     * overwriting the current PL_savestack_max. See scope.h.
+     *
+     * The +1 is because new_max/PL_savestack_max is the highest
+     * index, by Renew needs the number of items, which is one
+     * larger than the highest index. */
+    Renew(PL_savestack, new_max + SS_MAXPUSH + 1, ANY);
     PL_savestack_max = new_max;
 }
 
@@ -633,7 +652,7 @@ Perl_save_iv(pTHX_ IV *ivp)
 {
     PERL_ARGS_ASSERT_SAVE_IV;
 
-    SSCHECK(3);
+    SSGROW(3);
     SSPUSHIV(*ivp);
     SSPUSHPTR(ivp);
     SSPUSHUV(SAVEt_IV);

scope.h

@@ -27,8 +27,8 @@
  * macros */
 #define SS_MAXPUSH 4
 
-#define SSCHECK(need) if (UNLIKELY(PL_savestack_ix + (I32)(need) > PL_savestack_max)) savestack_grow()
 #define SSGROW(need) if (UNLIKELY(PL_savestack_ix + (I32)(need) > PL_savestack_max)) savestack_grow_cnt(need)
+#define SSCHECK(need) SSGROW(need) /* legacy */
 #define SSPUSHINT(i) (PL_savestack[PL_savestack_ix++].any_i32 = (I32)(i))
 #define SSPUSHLONG(i) (PL_savestack[PL_savestack_ix++].any_long = (long)(i))
 #define SSPUSHBOOL(p) (PL_savestack[PL_savestack_ix++].any_bool = (p))
@@ -47,7 +47,7 @@
  * like save_pushptrptr() to half its former size.
  * Of course, doing the size check *after* pushing means we must always
  * ensure there are SS_MAXPUSH free slots on the savestack. This is ensured by
- * savestack_grow() and savestack_grow_cnt always allocating SS_MAXPUSH slots
+ * savestack_grow_cnt always allocating SS_MAXPUSH slots
  * more than asked for, or that it sets PL_savestack_max to
  *
  * These are for internal core use only and are subject to change */
@@ -61,7 +61,7 @@
     ix += (need);                                               \
     PL_savestack_ix = ix;                                       \
     assert(ix <= PL_savestack_max + SS_MAXPUSH);                \
-    if (UNLIKELY(ix > PL_savestack_max)) savestack_grow();      \
+    if (UNLIKELY(ix > PL_savestack_max)) savestack_grow_cnt(ix - PL_savestack_max);      \
     assert(PL_savestack_ix <= PL_savestack_max);
 
 #define SS_ADD_INT(i)   ((ssp++)->any_i32 = (I32)(i))

t/re/pat.t

@@ -27,7 +27,7 @@ skip_all_without_unicode_tables();
 
 my $has_locales = locales_enabled('LC_CTYPE');
 
-plan tests => 1230;  # Update this when adding/deleting tests.
+plan tests => 1231;  # Update this when adding/deleting tests.
 
 run_tests() unless caller;
 
@@ -2413,6 +2413,19 @@ SKIP:
         is($y,"b","Branch reset in list context check 11 (b)");
         is($z,"z","Branch reset in list context check 12 (z)");
     }
+    {
+        # Test for GH Issue #20826. Save stack overflow introduced in
+        # 92373dea9d7bcc0a017f20cb37192c1d8400767f PR #20530.
+        # Note this test depends on an assert so it will only fail
+        # under DEBUGGING.
+        fresh_perl_is(q{
+            $_ = "x" x 1000;
+            my $pat = '(.)' x 200;
+            $pat = qr/($pat)+/;
+            m/$pat/;
+            print "ok";
+        }, 'ok', {}, 'gh20826: test regex save stack overflow');
+    }
 } # End of sub run_tests
 
 1;