From: Pip Cet <pipcet@gmail.com>
To: emacs-devel@gnu.org, Paul Eggert <eggert@cs.ucla.edu>
Cc: emacs-diffs@gnu.org
Subject: Re: master 9227864: Further fix for aborts due to GC losing pseudovectors
Date: Tue, 26 May 2020 06:40:49 +0000 [thread overview]
Message-ID: <CAOqdjBddeH2iHW2m2T+U+FALyKEwbP_aZrLYDnjUoX8=Ukc9qw@mail.gmail.com> (raw)
In-Reply-To: <20200526060646.662E120A2C@vcs0.savannah.gnu.org>
On Tue, May 26, 2020 at 6:06 AM Paul Eggert <eggert@cs.ucla.edu> 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.
next parent reply other threads:[~2020-05-26 6:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200526060645.22243.34109@vcs0.savannah.gnu.org>
[not found] ` <20200526060646.662E120A2C@vcs0.savannah.gnu.org>
2020-05-26 6:40 ` Pip Cet [this message]
2020-05-26 7:09 ` master 9227864: Further fix for aborts due to GC losing pseudovectors Paul Eggert
2020-05-26 7:25 ` Pip Cet
2020-05-26 7:40 ` Paul Eggert
2020-05-26 8:02 ` Pip Cet
2020-05-26 14:51 ` Stefan Monnier
2020-05-26 17:25 ` Pip Cet
2020-05-26 17:46 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOqdjBddeH2iHW2m2T+U+FALyKEwbP_aZrLYDnjUoX8=Ukc9qw@mail.gmail.com' \
--to=pipcet@gmail.com \
--cc=eggert@cs.ucla.edu \
--cc=emacs-devel@gnu.org \
--cc=emacs-diffs@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).