Subject: Re: bug#27761: Crash while using proof-general/company-coq on OS X

From: Eli Zaretskii <[email-masked]>
Date: Fri, 04 Aug 2017 11:24:59 +0300

> Date: Fri, 04 Aug 2017 14:03:25 +0900
> From: YAMAMOTO Mitsuharu <[email-masked]>
> Cc: [email-masked],	[email-masked],	[email-masked],
> 	Денис Редозубов
>  <[email-masked]>
> 
> I tried setting a breakpoint at the only recursive call to
> get_next_display_element and print *it.  The result is shown at the
> end of this mail (the line number is slightly different because I used
> the Mac port).
> 
> Notably, it->what == IT_EOB, it->method == GET_FROM_STRING,
> it->stop_charpos == 4, it->current.string_pos.charpos (aka
> IT_STRING_CHARPOS (*it)) == 6, it->string is a Lisp string " 164", and
> it->face_box_p == false.  Thus, GET_NEXT_DISPLAY_ELEMENT (it) at the
> entry of get_next_display_element sets it->what to IT_EOB again in
> next_element_from_string, all the top-level conditions in
> get_next_display_element become false except the last one, and goes
> directly to the recursive call again.

Yes, the problem is obviously that set_iterator_to_next doesn't pop
the iterator stack for some reason.  And that seems to happen because
it->end_charpos is 4, which corresponds to " 163", but does NOT
correspond to the "forall" string.  I'd like to understand how did
this happen, I think some code is missing somewhere to support this
complicated use case.

In any case, just stabbing in the dark, one possible band-aid could be
this:

diff --git a/src/xdisp.c b/src/xdisp.c
index c6f8566..95cbbe9 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -7655,7 +7655,8 @@ set_iterator_to_next (struct it *it, bool reseat_p)
 	     its end, and there is something on IT->stack, proceed
 	     with what is on the stack.  This can be either another
 	     string, this time an overlay string, or a buffer.  */
-	  if (IT_STRING_CHARPOS (*it) == SCHARS (it->string)
+	  if ((IT_STRING_CHARPOS (*it) == SCHARS (it->string)
+	       || IT_STRING_CHARPOS (*it) >= it->end_charpos)
 	      && it->sp > 0)
 	    {
 	      pop_it (it);

Another possible band-aid is this:

diff --git a/src/xdisp.c b/src/xdisp.c
index c6f8566..bc6d51c 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -7320,8 +7320,10 @@ get_next_display_element (struct it *it)
      because otherwise that stuff will never be displayed.  */
   if (!success_p && it->sp > 0)
     {
+      int old_sp = it->sp;
       set_iterator_to_next (it, false);
-      success_p = get_next_display_element (it);
+      if (it->sp != old_sp)
+	success_p = get_next_display_element (it);
     }
 
   /* Value is false if end of buffer or string reached.  */

But even if these band-aids do succeed in preventing the infinite
recursion, I'd like to understand how did we arrive at the situation
where the string position is 6 while the string being displayed is "
163", a 4-character string.  If you can step through the code and
describe what you see, I think it will help to find the real culprit.

Thanks.