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

Previous Next

Full log

View this message in rfc822 format

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24751 <at> debbugs.gnu.org
Subject: bug#24751: 26.0.50; Regex stack overflow not detected properly (gets "Variable binding depth exceeds max-specpdl-size")
Date: Sun, 13 Nov 2016 00:39:39 -0500

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>
> I think the patch can be simplified, where we now multiply by the size
> of fail_stack_elt_t and then divide by it: simply remove both the
> multiplication and the division.  That will make the code easier to
> read, and will make the units of each variable clear, something that I
> think is at the heart of this issue.

Ah, right.

[v2-0001-Fix-computation-of-regex-stack-limit.patch (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

>
>> 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).
>
> Which IMO is exactly TRT, since re_max_failures was computed given the
> runtime stack size of 8MB, so having it bail out after merely 800KB
> doesn't sound right to me, don't you agree?

Yes, I suppose we should also try to make use of the stack, rather than
calling malloc, right?  Something like this:

diff --git i/src/regex.c w/src/regex.c
index d23ba01..dcabde5 100644
--- i/src/regex.c
+++ w/src/regex.c
@@ -447,7 +447,11 @@ init_syntax_once (void)
 #else /* not REGEX_MALLOC  */
 
 # ifdef emacs
-#  define REGEX_USE_SAFE_ALLOCA USE_SAFE_ALLOCA
+#  define REGEX_USE_SAFE_ALLOCA                                         \
+  ptrdiff_t sa_avail = re_max_failures                                  \
+    * TYPICAL_FAILURE_SIZE * sizeof (fail_stack_elt_t);                 \
+  ptrdiff_t sa_count = SPECPDL_INDEX (); bool sa_must_free = false
+
 #  define REGEX_SAFE_FREE() SAFE_FREE ()
 #  define REGEX_ALLOCATE SAFE_ALLOCA
 # else



>
>> 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.
>
> I don't think I follow.  Can you tell what you tried to change, and
> where did you see the lack of any effect?

Modifying the initial value of re_max_failures as in the below patch,
has no effect on the size of the file at which stack regex overflow
happens (I confirmed it gets compiled by adding a #warning on the
previous line).  Printing re_max_failures in gdb still shows 40000.

diff --git i/src/regex.c w/src/regex.c
index d23ba01..d9170c0 100644
--- i/src/regex.c
+++ w/src/regex.c
@@ -1249,7 +1249,7 @@ WEAK_ALIAS (__re_set_syntax, re_set_syntax)
    whose default stack limit is 2mb.  In order for a larger
    value to work reliably, you have to try to make it accord
    with the process stack limit.  */
-size_t re_max_failures = 40000;
+size_t re_max_failures = 20;
 # else
 size_t re_max_failures = 4000;
 # endif


Actually I find Emacs still compiles if I removed that line completely,
there's just a compile warning saying

    regex.o: In function `re_match_2_internal':
    /home/npostavs/src/emacs/emacs-master/lib-src/../src/regex.c:5529: warning: the 're_max_failures' variable is obsolete and will go away.

I guess there's some kind of definition of it in libc?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.