bug#58739: Lack of error message about number of args (?native compilation?)

[Alan Mackenzie]

Hello, Andrea.

On Sun, Oct 23, 2022 at 12:12:49 +0000, Alan Mackenzie wrote:
> Hello, Emacs.

> Firstly, note that the function desktop-buffer has exactly 12
> parameters.

> With an up to date Emacs 29:

> (i) emacs -Q
> (ii) M-x load-library RET desktop RET.
> (iii) M-x disassemble RET desktop-buffer.

> Note that this is native code.

> (iv) M-: (desktop-buffer 1 2 3 4 5 6 7 8 9 10 11 12 13) RET

> This gives an error message about 4 not being a list.  What it ought to
> do is instead throw an error about the number of arguments.  This is a
> bug.

> (v) M-x load-file RET /path/to/emacs/lisp/desktop.elc.
> (vi) M-x disassemble RET desktop-buffer.

> Note that we now have byte compiled code.

> (vii) M-: (desktop-buffer 1 2 3 4 5 6 7 8 9 10 11 12 13) RET

> We now get a correct error message about the numbere of arguments.

> As a matter of interest, I noticed this bug while byte-compiling
> desktop.el inside Emacs.  It gave a warning message about the number of
> parameters to desktop-buffer having changed from 12+ to 12.

> Here, I suspect there's a bug in the native compilation of
> desktop-buffer.

The problem here is that (func-arity 'desktop-buffer) returns (12 . 12) on a
byte compiled desktop.elc, but (12 . many) on the corresponding .eln file.
This (12 . many) must be regarded as a bug, since there are no &rest
parameters in desktop-buffer.

I propose a minor amendment to the definition of MANY, such that it will
mean "there are &rest parameters", rather than "the calling convention
is with nargs + *args".  I have implemented this, and my patch is below.

What I want to ask you to check is that in the native compiler, when
declare_imported_func encounters a function with, say, exactly 12
arguments, it will throw an error.  I think this is actually correct,
since the compiler cannot know whether this function uses the subr
calling convention of nargs + *args, or the byte coded convention of
pushing the 12 arguments individually onto the stack.  Is throwing this
error a good idea?

Thanks!

Here's my proposed patch:


Amend the meaning of "MANY"; this solves bug #58739

Make "MANY" mean there are &rest parameters.  The previous definition left
ambiguous whether a subr with over 8 parameters also had &rest parameters.

* lisp/emacs-lisp/comp.el (comp-prepare-args-for-top-level): Return the number
of parameters rather than `many' when this is a fixed number over 8.

* src/comp.c (declare_imported_func): Throw an error when an imported function
has a fixed number over 8 of parameters, since we cannot compile a call to
this which will work for both byte code and native code.

* src/eval.c (eval_sub, funcall_subr): Where we test for MANY, also test the
number of arguments against 8.
(funcall_subr): Resolve the previous inability to call subr's with a fixed
number over 8 of arguments.


diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 5a05fe4854..f7e118616d 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -2057,9 +2057,10 @@ comp-prepare-args-for-top-level
   "Lexically-scoped FUNCTION."
   (let ((args (comp-func-l-args function)))
     (cons (make-comp-mvar :constant (comp-args-base-min args))
-          (make-comp-mvar :constant (if (comp-args-p args)
-                                        (comp-args-max args)
-                                      'many)))))
+          (make-comp-mvar :constant (cond
+                                     ((comp-args-p args) (comp-args-max args))
+                                     ((comp-nargs-rest args) 'many)
+                                     (t (comp-nargs-nonrest args)))))))
 
 (cl-defmethod comp-prepare-args-for-top-level ((function comp-func-d))
   "Dynamically scoped FUNCTION."
diff --git a/src/comp.c b/src/comp.c
index 14012634cc..8c9db20e72 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -999,6 +999,13 @@ declare_imported_func (Lisp_Object subr_sym, gcc_jit_type 
*ret_type,
       types = SAFE_ALLOCA (nargs * sizeof (* types));
       types[0] = comp.lisp_obj_type;
     }
+  else if (nargs > 8)
+      /* The calling convention for a function with a fixed number of
+        arguments greater than eight is different between a native
+        compiled function and a byte compiled function.  Thus we
+        cannot safely compile it with the native compiler.  */
+    signal_error ("Imported function has too many arguments safely to compile 
natively",
+                 subr_sym);
   else if (!types)
     {
       types = SAFE_ALLOCA (nargs * sizeof (* types));
diff --git a/src/eval.c b/src/eval.c
index 8810136c04..495dbb53b5 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2433,7 +2433,9 @@ eval_sub (Lisp_Object form)
 
       else if (XSUBR (fun)->max_args == UNEVALLED)
        val = (XSUBR (fun)->function.aUNEVALLED) (args_left);
-      else if (XSUBR (fun)->max_args == MANY)
+      else if (XSUBR (fun)->max_args == MANY
+              || XSUBR (fun)->max_args > 8)
+
        {
          /* Pass a vector of evaluated arguments.  */
          Lisp_Object *vals;
@@ -2996,7 +2998,8 @@ funcall_subr (struct Lisp_Subr *subr, ptrdiff_t numargs, 
Lisp_Object *args)
   if (numargs >= subr->min_args)
     {
       /* Conforming call to finite-arity subr.  */
-      if (numargs <= subr->max_args)
+      if (numargs <= subr->max_args
+         && subr->max_args <= 8)
        {
          Lisp_Object argbuf[8];
          Lisp_Object *a;
@@ -3032,15 +3035,13 @@ funcall_subr (struct Lisp_Subr *subr, ptrdiff_t 
numargs, Lisp_Object *args)
              return subr->function.a8 (a[0], a[1], a[2], a[3], a[4], a[5],
                                        a[6], a[7]);
            default:
-             /* If a subr takes more than 8 arguments without using MANY
-                or UNEVALLED, we need to extend this function to support it.
-                Until this is done, there is no way to call the function.  */
-             emacs_abort ();
+             emacs_abort ();   /* Can't happen. */
            }
        }
 
       /* Call to n-adic subr.  */
-      if (subr->max_args == MANY)
+      if (subr->max_args == MANY
+         || subr->max_args > 8)
        return subr->function.aMANY (numargs, args);
     }
 
diff --git a/src/lisp.h b/src/lisp.h
index 5f6721595c..881548ead4 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3187,10 +3187,11 @@ CHECK_SUBR (Lisp_Object x)
  `minargs' should be a number, the minimum number of arguments allowed.
  `maxargs' should be a number, the maximum number of arguments allowed,
     or else MANY or UNEVALLED.
-    MANY means pass a vector of evaluated arguments,
-        in the form of an integer number-of-arguments
-        followed by the address of a vector of Lisp_Objects
-        which contains the argument values.
+    MANY means there are &rest arguments.  Here we pass a vector
+        of evaluated arguments in the form of an integer
+        number-of-arguments followed by the address of a vector of
+        Lisp_Objects which contains the argument values.  (We also use
+        this convention when calling a subr with more than 8 parameters.)
     UNEVALLED means pass the list of unevaluated arguments
  `intspec' says how interactive arguments are to be fetched.
     If the string starts with a `(', `intspec' is evaluated and the resulting


-- 
Alan Mackenzie (Nuremberg, Germany).