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

[npostavs@users.sourceforge.net]

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)),				\