GNU bug report logs - #74771
Native compilation bug with struct predicates when lexical binding enabled (HEAD)

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: acorallo <at> gnu.org, pipcet <at> protonmail.com
Cc: 74771 <at> debbugs.gnu.org, eric.marsden <at> risk-engineering.org
Subject: bug#74771: Native compilation bug with struct predicates when lexical binding enabled (HEAD)
Date: Sun, 09 Mar 2025 11:00:16 +0200

Ping! Ping!  Andrea, could you please respond?

> Cc: eric.marsden <at> risk-engineering.org, 74771 <at> debbugs.gnu.org
> Date: Sat, 22 Feb 2025 11:56:25 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> Ping! Can we make progress here, please?
> 
> > Date: Wed, 08 Jan 2025 11:51:07 +0000
> > From: Pip Cet <pipcet <at> protonmail.com>
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 74771 <at> debbugs.gnu.org, eric.marsden <at> risk-engineering.org
> > 
> > "Andrea Corallo" <acorallo <at> gnu.org> writes:
> > 
> > > Eli Zaretskii <eliz <at> gnu.org> writes:
> > >
> > >>> Cc: 74771 <at> debbugs.gnu.org
> > >>> From: Andrea Corallo <acorallo <at> gnu.org>
> > >>> Date: Wed, 11 Dec 2024 17:29:34 -0500
> > >>>
> > >>> Eric Marsden <eric.marsden <at> risk-engineering.org> writes:
> > >>>
> > >>> > Hi,
> > >>> >
> > >>> > With the attached source file, Emacs miscompiles the struct predicate such
> > >>> > that a repeated call to the predicate on a non-struct object returns t.
> > >>> > This occurs with current HEAD on Linux/AMD64, but not on the Emacs 30.0.92
> > >>> > pretest. It does not occur when the lexical binding cookie is not present.
> > >>> >
> > >>> > % /opt/emacs/bin/emacs -Q --batch --eval "(load (native-compile \"/tmp/bug.el\"))" -f run
> > >>> > Loading /home/emarsden/.emacs.d/eln-cache/31.0.50-c021c983/bug-59c4b27c-c70072f9.eln (native compiled elisp)...
> > >>> > Running in GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.43, cairo version 1.18.2)
> > >>> >  of 2024-12-09
> > >>> > is? nil
> > >>> > is? t   ;; expecting nil
> > >>> > bar: 111
> > >>> >
> > >>> > ;;;   -*- lexical-binding: t -*-
> > >>> > ;;
> > >>> > ;; /opt/emacs/bin/emacs -Q --batch -L . --eval "(load (native-compile \"/tmp/bug.el\"))" -f run
> > >>> >
> > >>> > (require 'cl-lib)
> > >>> >
> > >>> > (cl-defstruct foobles bar baz)
> > >>> >
> > >>> > (defun bug (foo)
> > >>> >   (message "is? %s" (foobles-p foo))
> > >>> >   (message "is? %s" (foobles-p foo))
> > >>> >   (message "bar: %s" (foobles-bar foo)))
> > >>> >
> > >>> > (defun run ()
> > >>> >   (message "Running in %s" (version))
> > >>> >   (let ((foo "foo"))
> > >>> >     (bug foo)))
> > >>>
> > >>> Hi Eric,
> > >>>
> > >>> thanks for the report, I'll look at this in the coming days.
> > >>
> > >> Any progress here?
> > >
> > > Not so far, I'm on holiday this days so I don't have much time for
> > > coding, it's in my todo list tho.
> > 
> > No activity in a while, so I tried reproducing this: bug's still there.
> > 
> > Can you repeat for me what
> > 
> > (assume #s(comp-mvar (t) nil nil nil nil nil) (not #s(comp-mvar (foobles) nil nil nil nil nil)))
> > 
> > is supposed to mean?
> > 
> > To me, it appears to mean "the value of the first mvar is different from
> > the value of the second mvar, which is of type foobles".  This doesn't
> > say anything about the type of the first mvar, assuming there is more
> > than one value of the foobles type; if this is the correct
> > interpretation, we need to stop copying the typeset in this case:
> > 
> > diff --git a/lisp/emacs-lisp/comp-cstr.el b/lisp/emacs-lisp/comp-cstr.el
> > index 3d46cc8c6ae..5d87ff75703 100644
> > --- a/lisp/emacs-lisp/comp-cstr.el
> > +++ b/lisp/emacs-lisp/comp-cstr.el
> > @@ -1176,14 +1176,11 @@ comp-cstr-value-negation
> >    "Negate values in SRC setting the result in DST.
> >  DST is returned."
> >    (with-comp-cstr-accessors
> > -    (if (or (valset src) (range src))
> > -        (setf (typeset dst) ()
> > -              (valset dst) (valset src)
> > -              (range dst) (range src)
> > -              (neg dst) (not (neg src)))
> > -      (setf (typeset dst) (typeset src)
> > -            (valset dst) ()
> > -            (range dst) ()))
> > +    (and (or (valset src) (range src))
> > +         (setf (typeset dst) ()
> > +               (valset dst) (valset src)
> > +               (range dst) (range src)
> > +               (neg dst) (not (neg src))))
> >      dst))
> >  
> >  (defun comp-cstr-negation-make (src)
> > 
> > 
> > I see two other possibilities:
> > 
> > 1. it means the lhs mvar is of type "foobles", but not identical to the
> > second mvar.  This is what comp-cstr-negation-make currently assumes.
> > In this case, comp--add-cond-cstrs should not be emitting this
> > pseudo-insn for the case in which the lhs mvar is known not to be of
> > type "foobles", but nothing else is known about its value:
> > 
> > diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
> > index 269eae315e4..d5c512fbdc3 100644
> > --- a/lisp/emacs-lisp/comp.el
> > +++ b/lisp/emacs-lisp/comp.el
> > @@ -2033,11 +2033,7 @@ comp--add-cond-cstrs
> >         (comp--emit-assume 'and mvar-tested
> >                           (make--comp-mvar :type (comp-cstr-cl-tag mvar-tag))
> >                           (comp--add-cond-cstrs-target-block b bb2)
> > -                         nil)
> > -       (comp--emit-assume 'and mvar-tested
> > -                         (make--comp-mvar :type (comp-cstr-cl-tag mvar-tag))
> > -                         (comp--add-cond-cstrs-target-block b bb1)
> > -                         t))
> > +                         nil))
> >        (`((set ,(and (pred comp-mvar-p) cmp-res)
> >                (,(pred comp--call-op-p)
> >                 ,(and (or (pred comp--equality-fun-p)
> > 
> > 2. it means the lhs mvar is not of type "foobles".  In this case,
> > comp-cstr-value-negation should make the lhs mvar have a negated cstr
> > with type "foobles":
> > 
> > diff --git a/lisp/emacs-lisp/comp-cstr.el b/lisp/emacs-lisp/comp-cstr.el
> > index 3d46cc8c6ae..03a00123f64 100644
> > --- a/lisp/emacs-lisp/comp-cstr.el
> > +++ b/lisp/emacs-lisp/comp-cstr.el
> > @@ -1183,7 +1183,8 @@ comp-cstr-value-negation
> >                (neg dst) (not (neg src)))
> >        (setf (typeset dst) (typeset src)
> >              (valset dst) ()
> > -            (range dst) ()))
> > +            (range dst) ()
> > +            (neg dst) (not (neg src))))
> >      dst))
> >  
> >  (defun comp-cstr-negation-make (src)
> > 
> > Note that if (2) is intended, that is a really strange interpretation of
> > what "not" means: it's negating a cstr (a set of values), not an mvar (a
> > specific value), so why is the argument an mvar?  We could make this
> > change, and then try to recover whichever optimizations that disables:
> > 
> > diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
> > index 269eae315e4..bcc0628235a 100644
> > --- a/lisp/emacs-lisp/comp.el
> > +++ b/lisp/emacs-lisp/comp.el
> > @@ -2035,7 +2035,7 @@ comp--add-cond-cstrs
> >                           (comp--add-cond-cstrs-target-block b bb2)
> >                           nil)
> >         (comp--emit-assume 'and mvar-tested
> > -                         (make--comp-mvar :type (comp-cstr-cl-tag mvar-tag))
> > +                         (comp--type-to-cstr (comp-cstr-cl-tag mvar-tag))
> >                           (comp--add-cond-cstrs-target-block b bb1)
> >                           t))
> >        (`((set ,(and (pred comp-mvar-p) cmp-res)
> > 
> > In any case, please consider documenting the pseudo-insns.
> > not-eq-to-mvar and not-matching-cstr are two very different things!
> > 
> > Pip
> > 
> > 
> 
> 
> 
> 

Previous Next

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