bug#34910: 27.0.50; Too much matching reentrancy

["Mattias Engdegård" <mattiase@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

tis 2019-03-19 klockan 12:01 +0200 skrev Eli Zaretskii:
> 
> I don't know yet.  I asked you to elaborate on the reasons for your
> expectations, and I still hope you will humor me.  Because just
> taking the problem's manifestations at face value, one could argue
> that the precise error message is immaterial, as long as it points to
> the right direction, and "stack overflow in regexp matcher" is as
> obscure as the other message.  But maybe I'm missing something, which
> is why I asked you to elaborate on your reasoning.

Sorry, I didn't mean to sound touchy! Being the bug reporter, I deserve
every probing question. I'm most grateful for your quick reply!

It turns out the regexp cache logic is broken: it always uses the last
(LRU) entry, and fails if that entry is busy. It should really use the
last free entry.

What happened was that a regexp failed because of stack overflow, which
caused lots of code to run and many regexps to be compiled, but the
original (failing) regexp was still marked busy in the cache.
Eventually, it had been pushed down to the last position in the cache,
where it caused the next request for a free cache entry to fail with
the reentrancy error.

This patch fixes it. Please review.


[-- Attachment #2: 0001-Fix-spurious-regexp-reentrancy-error.patch --]
[-- Type: text/x-patch, Size: 2034 bytes --]

From 6ec7c354513a058fb0b396150ed3a7c1655e2a18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 19 Mar 2019 13:06:20 +0100
Subject: [PATCH] Fix spurious regexp reentrancy error

src/search.c (compile_pattern): Don't give up if the last regexp cache
entry is busy.  Instead, use the last (least recently used) non-busy
entry, and only report the reentrancy error if there is no free entry
at all (Bug#34910).
---
 src/search.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/search.c b/src/search.c
index a1e0b0976e..2c00e6e93f 100644
--- a/src/search.c
+++ b/src/search.c
@@ -223,11 +223,13 @@ static struct regexp_cache *
 compile_pattern (Lisp_Object pattern, struct re_registers *regp,
 		 Lisp_Object translate, bool posix, bool multibyte)
 {
-  struct regexp_cache *cp, **cpp;
+  struct regexp_cache *cp, **cpp, **lru_nonbusy;
 
-  for (cpp = &searchbuf_head; ; cpp = &cp->next)
+  for (cpp = &searchbuf_head, lru_nonbusy = NULL; ; cpp = &cp->next)
     {
       cp = *cpp;
+      if (!cp->busy)
+        lru_nonbusy = cpp;
       /* Entries are initialized to nil, and may be set to nil by
 	 compile_pattern_1 if the pattern isn't valid.  Don't apply
 	 string accessors in those cases.  However, compile_pattern_1
@@ -247,13 +249,14 @@ compile_pattern (Lisp_Object pattern, struct re_registers *regp,
 	  && cp->buf.charset_unibyte == charset_unibyte)
 	break;
 
-      /* If we're at the end of the cache, compile into the nil cell
-	 we found, or the last (least recently used) cell with a
-	 string value.  */
+      /* If we're at the end of the cache, compile into the last
+	 (least recently used) non-busy cell in the cache.  */
       if (cp->next == 0)
 	{
-          if (cp->busy)
+          if (!lru_nonbusy)
             error ("Too much matching reentrancy");
+          cpp = lru_nonbusy;
+          cp = *cpp;
 	compile_it:
           eassert (!cp->busy);
 	  compile_pattern_1 (cp, pattern, translate, posix);
-- 
2.20.1