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

From: Eli Zaretskii <[email-masked]>
Date: Sun, 02 Jul 2023 08:52:18 +0300

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

Thanks, but how do you explain that this code works as-is when the
BLOCK_ALIGN change is not used?