bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode

[Pip Cet <pipcet@gmail.com>]

On Sat, Feb 27, 2021 at 7:49 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Sat, 27 Feb 2021 05:06:43 +0000
> > Cc: Andrea Corallo <akrl@sdf.org>, 46670@debbugs.gnu.org, mauricio@collares.org
> >
> > > AFAICT, the principles proposed by Andrea are just common sense, and
> > > definitely not a drastic change from our existing practices.
> >
> > Let me try to explain a situation in which I don't think they work
> > very well, and which may or may not be similar to the situation we're
> > actually in:
[...]
> > How, assuming for the moment that the "strange" in (1) actually means
> > "buggy", are we supposed to fix this?
>
> I don't see any evidence yet that this needs to be fixed.
> Without
> such evidence, the whole discussion is about a moot point.

Quite the reverse: if we make rules saying such bug reports are to be
ignored, as Andrea suggested, actually reporting the bug is moot. It's
the rules I objected to in the previous mail, not the legitimate
requirement for further elaboration on my part before anyone else is
convinced there's a bug.

> Maybe I don't understand the issue well enough?

I'm certainly in no position to say I understand it perfectly and I
can explain it to you.

The problem is that "assume" insns do not have semantics yet: they
don't behave as you would expect "assume" to behave; they aren't
documented to behave differently; and there is no code (yet) which
uses them in ways that would make clear what they are supposed to
mean.

There is, I am convinced, no consistent way of _giving_ them semantics
without changing the assume insns we emit, first. For example, we're
emitting

<#(mvar Y :slot 1) is live>
(assume #(mvar X :slot 1) (not #(mvar Y :slot 1)))
<#(mvar X :slot 1) is live>

That's paradoxical on the face of it, as the two mvars refer to the
same variable. If there is a consistent nontrivial interpretation of
"assume" that would work in this case, I'm unaware of it.

Note that "assume" as we know and love it has very simple semantics:
it expresses a condition which, at runtime, is known to be satisfied.
You can convert an assume into a runtime test, and if it fails, the
assume was wrong in the first place.

That's not the case here, since the assume above would translate into

(assert (not (eq #(mvar X :slot 1) #(mvar Y :slot 1))))

which, obviously, never succeeds.

The fix is as trivial as not saying that the mvar X lives in slot 1.
There's actually a different slot that it does live in, and my patch
would have used that slot instead. It would have been equally valid
simply not to give it a slot at all, by whichever mechanism is
appropriate for that (I would have left the slot slot nil, but if
Andrea prefers assigning a negative "virtual" slot number to the mvar,
that's a valid choice as well).

There's a very similar issue with the other branch of the "if", as it turns out.

All of this was sufficient for me to write Andrea asking whether there
was an issue (leaving out what I thought would be, to him, the tedious
trivialities of the lines above), or whether I was confused. I could
certainly have handled the ensuing exchange better, and I'll try to do
so in the future. However, the categorical instalment of new rules
disallowing the presentation of patches or bug reports for all bugs
outside a very narrow class would prevent that entirely.

Pip