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

Previous Next

Package: emacs;

Reported by: Mauricio Collares <mauricio <at> collares.org>

Date: Sun, 21 Feb 2021 00:14:02 UTC

Severity: normal

Found in version 28.0.50

Done: Andrea Corallo <akrl <at> sdf.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Pip Cet <pipcet <at> gmail.com>
To: Andrea Corallo <akrl <at> sdf.org>
Cc: 46670 <at> debbugs.gnu.org, Mauricio Collares <mauricio <at> collares.org>
Subject: bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode
Date: Sun, 21 Feb 2021 22:46:29 +0000
[Message part 1 (text/plain, inline)]
On Sun, Feb 21, 2021 at 9:03 PM Andrea Corallo <akrl <at> sdf.org> wrote:
> Pip Cet <pipcet <at> gmail.com> writes:
>
> > On Sun, Feb 21, 2021 at 11:51 AM Pip Cet <pipcet <at> gmail.com> wrote:
> >> Does the attached patch help? Andrea, is my analysis correct?
> >
> > CCing Andrea.
> >
> > In more detail, some debugging showed we were trying to intersect a
> > "nil or t" constraint with a "sequence" constraint, the result being a
> > null constraint (t is not a sequence). So if (assume (and a b)) was
> > meant to imply the intersection of a and b, we're emitting it
> > incorrectly.
>
> Hi Pip,
>
> thanks for looking into this.

Thanks for your explanation!

> 'and' is meant to imply intersection, so yeah... as you guess there's
> some problem in the LIMPLE we generate :)

Thanks, I was on the wrong track there.

> The correct fix is to have `comp-add-cond-cstrs' rewrite the comparison
> sequence as something like:
>
> (set #(mvar nil X t) #(mvar 42082358 1 t))
> (set #(mvar 41121566 1 boolean) (call equal #(mvar 42082358 1 t) #(mvar 41665638 2 sequence)))
> (cond-jump #(mvar 41121566 1 boolean) #(mvar nil nil null) bb_2_cstrs_0 bb_1)
>
> Where X is a new slot we add to the frame.  We will reference this slot
> number in the assume instead of 1 so it does not get clobbered.

Okay, I think I understand the problem (we don't do classical SSA and
throw away the slot numbers), and your solution would avoid it, but it
seems needlessly complicated to me.

Why create a new slot that isn't used anywhere? The value of the stack
slot is clobbered by the (set ...), so we cannot emit any assumptions
about that stack slot based on previous values it held.

In fact, all we need to do is tell comp-cond-cstrs-target-mvar to
return nil more often, isn't it?

Pip
[0001-Don-t-lie-about-who-set-the-variable.patch (text/x-patch, attachment)]

This bug report was last modified 4 years and 168 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.