Re: pdumper on Solaris 10

[Pip Cet via "Emacs development discussions." <emacs-devel@gnu.org>]

"Po Lu" <luangruo@yahoo.com> writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>>> From: Po Lu <luangruo@yahoo.com>
>>> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,  Eli
>>>  Zaretskii <eliz@gnu.org>,
>>>   ali_gnu2@emvision.com,  emacs-devel@gnu.org
>>> Date: Tue, 10 Dec 2024 08:04:03 +0800
>>>
>>> Pip Cet <pipcet@protonmail.com> writes:
>>>
>>> > I was talking about the non-mps branch, yes.  We should drop !USE_LSB,
>>> > which doesn't work in its original use case today and hasn't for a
>>> > while.  It does happen to work in the WIDE_EMACS_INT case, but that's a
>>> > fortuitous accident at best.
>>>
>>> I propose to make it work again.  It ought to be a simple matter of
>>> scanning stack slots twice, with and without tag bits.
>>
>> Patches to that effect will be welcome, thanks.
>
> Yes, like I said at the beginning of this (burgeoning) thread, I intend
> to return to active Emacs development after the release of Emacs 30.

That's great to hear, but I'd like to make a final (promise!) attempt to
dissuade you from making this particular change ("fixing" the code to
support !USE_LSB_TAG more often).

The changes that are necessary concern the most delicate part of the
garbage collector: ambiguous scanning needs to remove the tag (the easy
part), and live_cons_p etc. have to be changed to allow for more offsets
(we need to recognize pointers to &Lisp_Object + 4 as well as pointers
to &Lisp_Object itself; I think this bug is already present on
big-endian 32-bit builds utilizing WIDE_EMACS_INT, but no one's using
that).  I suspect other changes will be necessary (in particular, I
expect breakage on systems that use the high byte of 64-bit pointers, as
some Android systems do; I also expect there will be sign extension /
zero extension problems). The pdumper code also needs to be studied
carefully, and most likely changed. (Pure space and unexec will likely
have gone away by then, but they would be affected, too).  This is not a
quick fix.

What makes this code delicate is that it's very rare for a stack
reference, particularly an unusual one, to be the last reference that
keeps another object alive; even if we fail to recognize an ambiguous
reference and free the object it refers to, the most likely outcome is
an invisible UAF error, because we happen to use-after-free memory right
after the garbage collection, and it'll still have the expected
contents.

This part of the garbage collector has long been in need of some work
(we currently search the RB tree twice for every word, even though the
second pass is usually unnecessary). Obviously, that will be harder if
we change the code in other ways.

The very best outcome of making the changes you propose is that no one
will ever use the changed code; in that case, all that will be achieved
is to add unused code to a function that's already hard to understand,
and to make future changes that much harder.

But that's not what I think will hapen. What I think will happen is that
users will start or continue using !USE_LSB_TAG, try to switch to MPS,
run into a problem, (hopefully) report a bug, and we won't be able to
deal with that bug report because we're comparing a USE_LSB_TAG + MPS
build to a !USE_LSB_TAG + !MPS one, and it'll be impossible to tell
which of the two major changes are causing the problem.

In other words, every person affected by your proposed changes will be
unable to usefully test MPS. I think that's bad.

If you insist on making the changes, please make sure there is a visible
"feature" in the corresponding MPS build which will let us know that bug
reports are useless and should be disregarded. I personally won't ask
anyone to test MPS in a setting where they cannot usefully report bugs.

Obviously, reducing the number of people who can usefully test MPS will
make it slightly less likely it'll ever land.

Pip