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

Message #23 received at 24751 <at> debbugs.gnu.org (full text, mbox):

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

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

>> 
>> 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
>> +
>
> Yes.  And please also add a comment there saying that this replaces
> USE_SAFE_ALLOCA.

Actually, we should avoid increasing this limit if the stack wasn't
increased, right?  Here's what I came up with, I think it doesn't cover
Cygwin/Windows though.

diff --git c/src/emacs.c i/src/emacs.c
index b74df21..d4655c8 100644
--- c/src/emacs.c
+++ i/src/emacs.c
@@ -831,8 +831,8 @@ main (int argc, char **argv)
 	 re_max_failures, then add 33% to cover the size of the
 	 smaller stacks that regex.c successively allocates and
 	 discards on its way to the maximum.  */
-      int ratio = 20 * sizeof (char *);
-      ratio += ratio / 3;
+      int min_ratio = 20 * sizeof (char *);
+      int ratio = min_ratio + min_ratio / 3;
 
       /* Extra space to cover what we're likely to use for other reasons.  */
       int extra = 200000;
@@ -869,6 +869,7 @@ main (int argc, char **argv)
 
       /* Don't let regex.c overflow the stack.  */
       re_max_failures = lim < extra ? 0 : min (lim - extra, SIZE_MAX) / ratio;
+      emacs_re_safe_alloca = re_max_failures * min_ratio;
     }
 #endif /* HAVE_SETRLIMIT and RLIMIT_STACK and not CYGWIN */
 
diff --git c/src/regex.c i/src/regex.c
index d23ba01..56cffa1 100644
--- c/src/regex.c
+++ i/src/regex.c
@@ -447,7 +447,13 @@ init_syntax_once (void)
 #else /* not REGEX_MALLOC  */
 
 # ifdef emacs
-#  define REGEX_USE_SAFE_ALLOCA USE_SAFE_ALLOCA
+/* This may be adjusted in main(), if the stack is successfully grown.  */
+ptrdiff_t emacs_re_safe_alloca = MAX_ALLOCA;
+/* Like USE_SAFE_ALLOCA, but use emacs_re_safe_alloca.  */
+#  define REGEX_USE_SAFE_ALLOCA                                        \
+  ptrdiff_t sa_avail = emacs_re_safe_alloca;                           \
+  ptrdiff_t sa_count = SPECPDL_INDEX (); bool sa_must_free = false
+
 #  define REGEX_SAFE_FREE() SAFE_FREE ()
 #  define REGEX_ALLOCATE SAFE_ALLOCA
 # else
diff --git c/src/regex.h i/src/regex.h
index 4922440..45cbe0a 100644
--- c/src/regex.h
+++ i/src/regex.h
@@ -187,6 +187,11 @@
 /* Roughly the maximum number of failure points on the stack.  */
 extern size_t re_max_failures;
 
+#ifdef emacs
+/* Amount of memory that we can safely stack allocate.  */
+extern ptrdiff_t emacs_re_safe_alloca;
+#endif
+
 
 /* Define combinations of the above bits for the standard possibilities.
    (The [[[ comments delimit what gets put into the Texinfo file, so


>> 
>> 
>> 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?
>
> Most probably.  You should be able to see that using "nm -A".  If
> that's indeed so, I think we should rename that variable to something
> like emacs_re_max_failures, to avoid stomping on the libc variable..

Ah, right:

    $ nm -A /usr/lib/libc.so.6 | grep re_max_failures
    /usr/lib/libc.so.6:0000000000000000 n __evoke_link_warning_re_max_failures
    /usr/lib/libc.so.6:00000000003981d8 D re_max_failures

Previous Next

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