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

[Paul Eggert]

On 5/25/20 11:40 PM, Pip Cet wrote:

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

It's true that we haven't run into these systems yet. However, I worry that it
won't be all that hypothetical in the not-so-distant future, given that so many
SIMD instructions require multiple-of-16 alignment and Emacs pseudovectors use
system types that may require such alignment.

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

It can also happen with GCC 7 + glibc 2.25. Some platforms are fitfully moving
to alignment-of-16 malloc and there are mismatches between system pieces. I'm
not sure we can catalog all the affected systems.

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

In current master I've fixed this so that there is zero wasted memory; the type
is used only to calculate alignment, not to allocate memory.

It is a maintenance hassle, though not bad compared to the hassle of remembering
to add GCALIGNED_STRUCT and GCALIGNED_UNION_MEMBER all over the place, which we
have to do anyway.

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

That's not going to work if some platform uses an alignment-of-16 type in
pthread_cond_t (or some other system type that Emacs uses).

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

I assume you meant 'verify' rather than 'eassert'? That'd catch the bug at
compile time.

But instead, how about an alignment argument to allocate_pseudovector, or a
variant of allocate_pseudovector that takes such an argument? Then Emacs could
support any alignment-greater-than-16 types that turn up.