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

[Robert Pluim]

>>>>> 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