From: npostavs@users.sourceforge.net
To: Eli Zaretskii <eliz@gnu.org>
Cc: 24751@debbugs.gnu.org
Subject: 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 [thread overview]
Message-ID: <87wpg5l9st.fsf@users.sourceforge.net> (raw)
In-Reply-To: <83a8d3cq9s.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 13 Nov 2016 18:12:47 +0200")
Eli Zaretskii <eliz@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
+
\f
/* 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
next prev parent reply other threads:[~2016-11-15 3:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-21 3:54 bug#24751: 26.0.50; Regex stack overflow not detected properly (gets "Variable binding depth exceeds max-specpdl-size") npostavs
2016-11-04 8:22 ` Eli Zaretskii
2016-11-05 19:34 ` npostavs
2016-11-06 15:45 ` Eli Zaretskii
2016-11-13 5:39 ` npostavs
2016-11-13 16:12 ` Eli Zaretskii
2016-11-15 3:08 ` npostavs [this message]
2016-11-15 16:12 ` Eli Zaretskii
2016-11-16 1:06 ` npostavs
2016-11-16 16:25 ` Eli Zaretskii
2016-11-16 23:25 ` npostavs
2016-11-17 16:21 ` Eli Zaretskii
2016-11-19 10:02 ` Eli Zaretskii
2017-01-01 18:33 ` npostavs
2017-01-01 18:41 ` Eli Zaretskii
2017-01-01 18:57 ` npostavs
2017-01-01 20:06 ` Eli Zaretskii
2017-01-02 4:49 ` npostavs
2017-01-02 15:24 ` Eli Zaretskii
2017-01-02 18:30 ` npostavs
2017-01-02 19:22 ` Eli Zaretskii
2017-01-08 23:49 ` npostavs
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wpg5l9st.fsf@users.sourceforge.net \
--to=npostavs@users.sourceforge.net \
--cc=24751@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).