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

[Pip Cet]

On Sun, Feb 21, 2021 at 12:14 AM Mauricio Collares
<mauricio@collares.org> wrote:
> This was found by Anthony Cowley, who isolated the exact function in
> lsp-mode that was misbehaving. I verified that I could reproduce this
> findings, and then I removed surrounding context to obtain a minimized
> testcase. If this fails to reproduce, it's entirely my fault.
>
> Steps to reproduce:
>
> 1) Put this in minimized.el:
>
> ;;; -*- lexical-binding: t; -*-
>
> (defun minimized--look-back (s)
>   (and (equal (buffer-substring-no-properties (- (point) (length s)) (point))
>               s)
>        s))
>
> (defun minimized-go ()
>   (interactive)
>   (message (minimized--look-back ".")))
>
> (provide 'minimized)
>
> 2) Type "." in a buffer and then run minimized-go with the point after
> the period. This prints back "." in the minibuffer if the code's
> interpreted but not if it's native-compiled.
>
> Note that removing the "lexical-binding: t" line makes the bug not
> reproduce. Replacing "(- (point) (length s))" by "(1- (point))" also
> makes the bug disappear.

I can reproduce this with this code:

(funcall
 (let ((comp-verbose 3) (comp-debug 3))
   (native-compile `(lambda (s) (and (equal
(buffer-substring-no-properties (- (point) (length s)) (point)) s)
s))))
 ")")

I think there's some confusion in comp-fwprop-insn between (and) as a
logical operator and (and) as a pcase operator. The latter means a
variable's constraint must be in the intersection of all argument
types, but the former only implies that the variable constraint is
somewhere in the union of the argument constraints [1].

Does the attached patch help? Andrea, is my analysis correct?

Pip

[1] - note that we emit (assume a (and b c)) for (setq a (and c b))
under some circumstances, so it would be incorrect to use only c's
constraint.

0001-native-comp-Fix-constraint-for-assume-x-and-a-b-Bug-.patch
Description: Text Data