GNU bug report logs - #61960
30.0.50; Unexec build reliably crashes during loadup

Previous Next

Full log

View this message in rfc822 format

From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61960 <at> debbugs.gnu.org
Subject: bug#61960: 30.0.50; Unexec build reliably crashes during loadup
Date: Sun, 02 Jul 2023 14:32:09 +0300

On Sun, 2023-07-02 at 08:52 +0300, Eli Zaretskii wrote:
> > From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> > 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?

I don't know exactly. But I've read calculations in alloc.c where BLOCK_ALIGN is
used (directly and indirectly) and the code there is very convoluted. Apparently
an offset somewhere gets calculated incorrectly, so some
`_heapinfo[block].busy.type` fields end up having 0 despite being allocated with
malloc(). As I mentioned it is worth investigating, but without reverse-
execution (as `rr` for some reason doesn't work with `temacs` in this case) it
will be hard to pinpoint.

I am planning to report the `rr` problem to them, so hopefully it will be
possible to find the real culprit in the future.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.