bug#38407: 27.0.50; infinite loop with display of large file without newlines

[Robert Pluim <rpluim@gmail.com>]

>>>>> On Tue, 03 Dec 2019 18:05:42 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> Yes, I was wondering about that myself.  But we need more details to
    Eli> understand better what, if anything, can be done about this.

    Eli> First, which part of SAVE_IT causes this?  I'm guessing it's this
    Eli> part:

    Eli>   #define SAVE_IT(ITCOPY, ITORIG, CACHE)	\
    Eli>     do {					\
    Eli>       if (CACHE)				\
    Eli> 	bidi_unshelve_cache (CACHE, true);	\
    Eli>       ITCOPY = ITORIG;				\
    Eli>       CACHE = bidi_shelve_cache ();		\  <<<<<<<<<<<<
    Eli>     } while (false)

Yes, itʼs bidi_shelve_cache

    Eli> If that is true, then I think the offending part of bidi_shelve_cache
    Eli> is this:

    Eli>   alloc = (bidi_shelve_header_size
    Eli> 	   + bidi_cache_idx * sizeof (struct bidi_it));
    Eli>   databuf = xmalloc (alloc);
    Eli>   bidi_cache_total_alloc += alloc;

    Eli>   memcpy (databuf, &bidi_cache_idx, sizeof (bidi_cache_idx));
    Eli>   memcpy (databuf + sizeof (bidi_cache_idx),                      <<<<<<<
    Eli> 	  bidi_cache, bidi_cache_idx * sizeof (struct bidi_it));  <<<<<<<
    Eli>   memcpy (databuf + sizeof (bidi_cache_idx)
    Eli> 	  + bidi_cache_idx * sizeof (struct bidi_it),
    Eli> 	  bidi_cache_start_stack, sizeof (bidi_cache_start_stack));

    Eli> And if this guess is also true, then I think the problem is that
    Eli> databuf + sizeof (bidi_cache_idx) is unaligned on 64-bit systems,
    Eli> since bidi_cache_idx is an int.

The '_unaligned_' bit of that memmove function name does not mean
thatʼs itʼs doing unoptimized unaligned copies: it means it accepts
unaligned pointers, and aligns them as necessary to enable fast
copying. Anyway, I made bidi_cache_idx an intptr_t, and it made no
difference.

I think that misalignment is vastly dwarfed by the following:

    (gdb) l bidi_shelve_cache
    973	{
    974	  unsigned char *databuf;
    975	  ptrdiff_t alloc;
    976
    977	  /* Empty cache.  */
    978	  if (bidi_cache_idx == 0)
    979	    return NULL;
    980
    981	  alloc = (bidi_shelve_header_size
    982		   + bidi_cache_idx * sizeof (struct bidi_it));
    (gdb) b 980
    Breakpoint 3 at 0x4b66aa: file bidi.c, line 981.
    (gdb) commands
    Type commands for breakpoint(s) 3, one per line.
    End with a line saying just "end".
    >p bidi_cache_idx
    >p bidi_cache_idx * sizeof(struct bidi_it)
    >end

    Thread 1 "emacs" hit Breakpoint 3, bidi_shelve_cache () at bidi.c:981
    981	  alloc = (bidi_shelve_header_size
    $25 = 30860
    $26 = 71842080

which means Emacs is copying 70MB of data every time bidi_shelve_cache
is called, and itʼs called *a lot* in this scenario. Could we not do
this shelving by pointer-swapping or similar rather than copying?

Robert