bug#24751: 26.0.50; Regex stack overflow not detected properly (gets "Variable binding depth exceeds max-specpdl-size")

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Date: Thu, 20 Oct 2016 23:54:05 -0400
>> 
>> So we we might want to fix the re_max_failures setting in main, but it
>> doesn't quite make sense to me that GROW_FAIL_STACK relies on
>> re_max_failures being a multiple of (sizeof (fail_stack_elt_t)).  At the
>> definition of TYPICAL_FAILURE_SIZE we have
>> 
>> /* Estimate the size of data pushed by a typical failure stack entry.
>>    An estimate is all we need, because all we use this for
>>    is to choose a limit for how big to make the failure stack.  */
>> /* BEWARE, the value `20' is hard-coded in emacs.c:main().  */
>> #define TYPICAL_FAILURE_SIZE 20
>> 
>> Why do we use an "estimate" here?  What's wrong with just using
>> (re_max_failures * sizeof (fail_stack_elt_t)) as the limit?  Or should
>> the limit actually be (re_max_failures * TYPICAL_FAILURE_SIZE * sizeof
>> (fail_stack_elt_t))?
>
> I think it should be the latter, indeed.
>
> Can you propose a patch along those lines that would remove the
> infloop in ENSURE_FAIL_STACK?
>
> Thanks.

The below seems to work, but effectively increases the size of the
failure stack (so the sample file size has to be increased 8-fold to get
a regex stack overflow).  Strangely, changing the value in the
definition of re_max_failures doesn't seem to have any effect, it stays
40000 regardless.  I am quite confused.

diff --git i/src/regex.c w/src/regex.c
index 1c6c9e5..163c5b4 100644
--- i/src/regex.c
+++ w/src/regex.c
@@ -1320,19 +1320,22 @@ WEAK_ALIAS (__re_set_syntax, re_set_syntax)
 
 #define GROW_FAIL_STACK(fail_stack)                                    \
   (((fail_stack).size * sizeof (fail_stack_elt_t)                      \
-    >= re_max_failures * TYPICAL_FAILURE_SIZE)                         \
+    >= re_max_failures * sizeof (fail_stack_elt_t)                      \
+    * TYPICAL_FAILURE_SIZE)                                             \
    ? 0                                                                 \
    : ((fail_stack).stack                                               \
       = REGEX_REALLOCATE_STACK ((fail_stack).stack,                    \
          (fail_stack).size * sizeof (fail_stack_elt_t),                \
-         min (re_max_failures * TYPICAL_FAILURE_SIZE,                  \
+         min (re_max_failures * sizeof (fail_stack_elt_t)              \
+               * TYPICAL_FAILURE_SIZE,                                  \
               ((fail_stack).size * sizeof (fail_stack_elt_t)           \
                * FAIL_STACK_GROWTH_FACTOR))),                          \
                                                                        \
       (fail_stack).stack == NULL                                       \
       ? 0                                                              \
       : ((fail_stack).size                                             \
-        = (min (re_max_failures * TYPICAL_FAILURE_SIZE,                \
+         = (min (re_max_failures * sizeof (fail_stack_elt_t)            \
+                 * TYPICAL_FAILURE_SIZE,                                \
                 ((fail_stack).size * sizeof (fail_stack_elt_t)         \
                  * FAIL_STACK_GROWTH_FACTOR))                          \
            / sizeof (fail_stack_elt_t)),                               \