Re: master 9227864: Further fix for aborts due to GC losing pseudovectors

[Pip Cet]

On Tue, May 26, 2020 at 6:06 AM Paul Eggert <address@hidden> wrote:
>     Further fix for aborts due to GC losing pseudovectors

I don't understand this change. It makes the code much more
complicated, all for hypothetical systems that require alignment > 8
for a hypothetical 16-byte data type in a Lisp structure.

> +   multiple of LISP_ALIGNMENT.  This works even for buggy platforms
> +   like MinGW circa 2020, where alignof (max_align_t) is 16 even though
> +   the malloc alignment is only 8, and where Emacs still works because
> +   it never does anything that requires an alignment of 16.  */

I think we should be specific here and say it's the mingw.org 32-bit
version (or whatever Eli's using) only that has problems. mingw64 is
fine, I believe, and the normal mingw32 version is buggy but our
header inclusion order happens to get things right, so there are no
crashes.

>  static bool
>  maybe_lisp_pointer (void *p)
>  {
> -  return (uintptr_t) p % LISP_ALIGNMENT == 0;
> +  return (uintptr_t) p % GCALIGNMENT == 0;
>  }

This is the only change that I understand is necessary.

> -   generate better code.
> +   generate better code.  Also, such structs should be added to the
> +   emacs_align_type union.

That's going to be a maintenance nightmare, since failures to do so
won't actually show up on real machines, and a lot of wasted memory if
someone does add an AVX512 type.

I'd prefer a simple warning not to use long double or similarly
unusual types in pseudovectors, and an eassert (see below) to catch it
if people do that.

> +/* A type with alignment at least as large as any object that Emacs
> +   allocates.  This is not max_align_t because some platforms (e.g.,
> +   mingw) have buggy malloc implementations that do not align for
> +   max_align_t.  This union contains types of all GCALIGNED_STRUCT

I think a simple eassert (GCALIGNMENT % alignof (type) == 0) in an
(inlined, obviously) version of allocate_pseudovector should suffice
to catch this hypothetical problem.