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

[Pip Cet]

On Sun, Feb 21, 2021 at 9:03 PM Andrea Corallo <akrl@sdf.org> wrote:
> Pip Cet <pipcet@gmail.com> writes:
>
> > On Sun, Feb 21, 2021 at 11:51 AM Pip Cet <pipcet@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
Description: Text Data