Subject: bug#61960: 30.0.50; Unexec build reliably crashes during loadup

From: Konstantin Kharlamov <[email-masked]>
Date: Sun, 02 Jul 2023 04:50:26 +0300

I've found a diff that fixes the build, but whether it's okay is worth discussion:

    diff --git a/src/gmalloc.c b/src/gmalloc.c
    index e655d69f660..f49bb01e08b 100644
    --- a/src/gmalloc.c
    +++ b/src/gmalloc.c
    @@ -1704,7 +1704,7 @@ allocated_via_gmalloc (void *ptr)
         return false;
       size_t block = BLOCK (ptr);
       size_t blockmax = _heaplimit - 1;
    -  return block <= blockmax && _heapinfo[block].busy.type != 0;
    +  return block <= blockmax;
     }

     /* See the comments near the beginning of this file for explanations

Here's what happens: Emacs uses internal stack-based allocator (apparently allocating
with sbrk(), but I'm not sure) along with the system allocator. Whenever a memory is
allocated from the internal allocator, you can't call `free()` on it.

When Emacs wants to free memory, it calls `hybrid_free_1()`, which internally
determines whether the `ptr` passed belongs to system heap or to Emacs
stack. Determining in turn is done by `allocated_via_gmalloc()`.

Emacs also keeps the lowest and highest boundary of this stack in variables
`_heapbase` and `_heaplimit` accordingly (except the latter is measured in
"blocks"). The code in diff `block <= blockmax` simply makes sure that the `ptr`
passed is within the stack-allocated memory, which implies it can't be deallocated
with `free()`

There's a question though of the right-hand side that I remove, the
`_heapinfo[block].busy.type != 0;`. Apparently the `type` should keep some memory
info, and apparently there's a bug somewhere that screws it up. It is a bug worth
fixing, although for some reason `rr replay` doesn't work for me with `temacs`
(probably a bug in rr), and without reverse-execution tracking that down would be
very hard.

But I would argue that the right-hand side check has no value in this function,
because to determine the source of allocation it's enough to just check whether `ptr`
is in _heapbase .. _heaplimit range (barring the fact they're different units).