bug#56393: Actually fix the long lines display bug

[Gregory Heytings <gregory@heytings.org>]

>
> Thanks.  The crash is gone, of course,
>

That's good news.

>
> I can send you a backtrace from yesterday:
>
> [...]
>
> As you see, the cause of problem was the same one:
>
> [...]
>
> Since you have now moved the iteration to begin at the "narrowed" BEGV, 
> this crash is also gone.
>

That's good news, too.  Thanks for the other backtrace.

>
> To tell the truth, I'm not sure this kind of fix is the correct 
> solution, because basically its success is a matter of luck (or lack 
> thereof).  For example, recall the higher-level context of the first 
> segfault:
>
> [...]
>
> This is isearch.el trying to establish whether the position of the match 
> is visible in the window.  To do that, it starts from the current 
> window-start position (which happens to be 1), and simulates display of 
> the buffer text portion up until point or until the end of window, 
> whichever comes first, to determine whether point is beyond the end of 
> the window.  Your change makes the search start from some much later 
> position instead, which could very well produce incorrect results: 
> pos-visible-in-window-p could decide that point _is_ visible, when it 
> really isn't.  (It doesn't happen in this particular case because the 
> newline is far away -- at the very end of the buffer -- so it isn't 
> visible in both cases.  But that's sheer luck.)
>

Well, this happens only in buffers with long lines, and only when we are 
inside a long line, so from my point of view it works as expected, and 
moreover the risk is small.  Would the following be better from your point 
of view?

diff --git a/src/xdisp.c b/src/xdisp.c
index d69d7440bc..572ad2b854 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -8668,18 +8668,11 @@ get_visually_first_element (struct it *it)
    bool string_p = STRINGP (it->string) || it->s;
    ptrdiff_t eob = (string_p ? it->bidi_it.string.schars : ZV);
    ptrdiff_t bob;
+  ptrdiff_t obegv = BEGV;

-  SET_WITH_NARROWED_BEGV (it, bob, string_p ? 0 : BEGV);
-
-  /* Reseat again when, as a consequence of the SET_WITH_NARROWED_BEGV
-     above, the iterator is before bob.  */
-  if (!string_p && IT_CHARPOS (*it) < bob)
-    {
-      struct text_pos pos;
-      pos.charpos = bob;
-      pos.bytepos = CHAR_TO_BYTE (bob);
-      reseat (it, pos, true);
-    }
+  SET_WITH_NARROWED_BEGV (it, bob,
+                         string_p ? 0 :
+                         IT_BYTEPOS (*it) < BEGV ? obegv : BEGV);

    if (STRINGP (it->string))
      {

>
> More generally, if you look at redisplay_window, you will see that about 
> 2/3 of its code tries very hard to reuse the previous window-start 
> position, before it gives up and looks for a new starting position.  So 
> in any situation where the previous window-start is far enough before 
> point, all that code will basically work with a buffer position that is 
> at risk of being before the "narrowed" BEGV.  Thus, any code there which 
> tries stuff like start_display+move_it_to will risk hitting this kind of 
> problems -- either FETCH_BYTE will crash, or we risk producing the wrong 
> result because we force the code to jump to the "narrowed" BEGV before 
> doing anything, while its caller expects results relative to a different 
> position.
>

I understand.  But note that temporarily narrowing the buffer happens only 
at a few well-chosen places, which are situated rather low in the 
abstraction layers, so the effect on other parts of the code is nil.

>
> I think this is because the display engine assumes that BEGV stays put 
> during the entire redisplay_window lifetime, i.e. that all of the 
> subroutines it calls see the same value of BEGV.  This is no longer so 
> on the branch, and I wonder whether and how we should handle this new 
> situation to keep the display code stable and reliable.
>

I don't know.