bug#41321: 27.0.91; Emacs aborts due to invalid pseudovector objects

[Pip Cet <pipcet@gmail.com>]

On Sun, May 24, 2020 at 2:24 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Sat, 23 May 2020 23:54:17 +0000
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 41321@debbugs.gnu.org
> >
> > I think I've worked it out: it's this mingw bug:
> > https://sourceforge.net/p/mingw-w64/bugs/778/
>
> Thank you for working on this tricky problem.
>
> FTR, I don't use that flavor of MinGW.

So your flavor is even more broken than what Debian ships? That's
interesting, which flavor is it?

> > On mingw, if <stdint.h> is included before/instead of stddef.h,
> > alignof (max_align_t) == 16.
>
> The problem with the order of inclusion doesn't exist in my header
> files, so alignof (max_align_t) is always 16.

Okay, so that is our bug.

> > However, as can be seen by the backtrace
> > above, Eli's malloc only returned an 8-byte-aligned block.
>
> Isn't that strange?  Lisp data is allocated via lmalloc, AFAIK, and
> lmalloc is supposed to guarantee LISP_ALIGNMENT alignment.  Or am I
> missing something?

No, it relies on the compile-time constants and never checks.

The relevant code is:

enum { MALLOC_IS_LISP_ALIGNED = alignof (max_align_t) % LISP_ALIGNMENT == 0 };

static bool
laligned (void *p, size_t size)
{
  return (MALLOC_IS_LISP_ALIGNED || (intptr_t) p % LISP_ALIGNMENT == 0
      || size % LISP_ALIGNMENT != 0);
}

... so laligned is a constant "true" function on your machine, since
alignof (max_align_t) is 16 and LISP_ALIGNMENT is 16.

static void *
lmalloc (size_t size, bool clearit)
{
#ifdef USE_ALIGNED_ALLOC
  if (! MALLOC_IS_LISP_ALIGNED && size % LISP_ALIGNMENT == 0)
    {
      void *p = aligned_alloc (LISP_ALIGNMENT, size);
      if (clearit && p)
    memclear (p, size);
      return p;
    }
#endif

  while (true)
    {
      void *p = clearit ? calloc (1, size) : malloc (size);
      if (laligned (p, size))
    return p;
      free (p);
      size_t bigger = size + LISP_ALIGNMENT;
      if (size < bigger)
    size = bigger;
    }
}

That optimizes down to returning the malloc/calloc return value directly.

IOW, alloc.c relies on malloc() being max_align_t-aligned, and never
checks, not even in debug builds. That's something that needs to be
fixed, since broken-malloc environments such as yours exist.

> > That's not normally a problem, because mark_maybe_object doesn't
> > care about alignment; but in conjunction with the gcc behavior
> > change, we rely or mark_maybe_pointer to mark the pointer, and it
> > doesn't, because the pointer is not aligned to a LISP_ALIGNMENT =
> > 16-byte boundary.
>
> I still very much doubt that this has anything to do with stack
> marking during GC, since I've shown in my backtrace that
> current_buffer->overlays_before points to an overlay with invalid
> markers.

You haven't.