GNU bug report logs - #58739
Lack of error message about number of args (?native compilation?)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58739 in the body.
You can then email your comments to 58739 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Lack of error message about number of args (?native compilation?)
Date: Sun, 23 Oct 2022 12:12:49 +0000

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.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #8 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Andrea Corallo <akrl <at> sdf.org>
Cc: 58739 <at> debbugs.gnu.org, acm <at> muc.de
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Mon, 24 Oct 2022 16:41:25 +0000

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).

Message #11 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Andrea Corallo <akrl <at> sdf.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 58739 <at> debbugs.gnu.org
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Tue, 25 Oct 2022 20:15:38 +0000

Alan Mackenzie <acm <at> muc.de> writes:

> 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?

Hi Alan,

to me this fix looks like a good idea (assuming changing the definition
of MANY is acceptable).

I think also that throwing the error in 'declare_imported_func' is okay
at this point.

Just two small nits (forgive me please :) :

> Thanks!
>
> Here's my proposed patch:

[...]

>      }
> +  else if (nargs > 8)
> +      /* The calling convention for a function with a fixed number of
> +	 arguments greater than eight is different between a native
         ^^^
         I think this should be aligned with the initial 'The'
         
> +	 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",

I think we should break this string to stay within 78 characters (see CONTRIBUTE).

> +		  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)

Likewise

>    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)

Likewise

>  	      return subr->function.a8 (a[0], a[1], a[2], a[3], a[4], a[5],
>  					a[6], a[7]);

[...]

Thanks for the patch!

  Andrea

Message #14 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Andrea Corallo <akrl <at> sdf.org>
Cc: 58739 <at> debbugs.gnu.org
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Sat, 29 Oct 2022 12:04:36 +0000

Hello, Andrea.

On Tue, Oct 25, 2022 at 20:15:38 +0000, Andrea Corallo wrote:
> Alan Mackenzie <acm <at> muc.de> writes:

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

[ .... ]

> >> 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?

> Hi Alan,

> to me this fix looks like a good idea (assuming changing the definition
> of MANY is acceptable).

We'll see what Eli says.

> I think also that throwing the error in 'declare_imported_func' is okay
> at this point.

Apologies at this point.  I should have produced an error from this
before bothering you.  I was unable to produce such an error, and I've
spent the last few days understanding what happens here, with this
result:

A call from a native compiled function is always in the form nargs +
*args, regardless of whether there are more than 8 arguments or not.
More accurately, the call to an unknown type of function (.eln/.elc/.el)
puts the function (in some form, I don't know exactly what) at element 0
of *args, and the arguments themselves starting at element 1 of *args.
It then calls Ffuncall (or something like it).

When the called function is a byte-code function, further down the call
stack exec_byte_code gets called with this (nargs + 1) + *args.
exec_byte_code then pushes the arguments onto the stack before
interpreting the byte code.

So this "problem" with the native compiler not knowing what call sequence
to generate isn't a problem at all.  It's all dealt with at run time.  No
doubt this slows down Emacs quite a bit, but it's safe.

So I'll take that bit out of my patch, and commit the rest of it to
master, and see what happens from there.

> Just two small nits (forgive me please :) :

Thanks for these, I've incorporated them into my amended source code.

[ .... ]

> Thanks for the patch!

>   Andrea

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #17 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 58739 <at> debbugs.gnu.org, akrl <at> sdf.org
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Sat, 29 Oct 2022 15:48:35 +0300

> Cc: 58739 <at> debbugs.gnu.org
> Date: Sat, 29 Oct 2022 12:04:36 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > to me this fix looks like a good idea (assuming changing the definition
> > of MANY is acceptable).
> 
> We'll see what Eli says.

About what, may I ask?

Message #22 received at 58739-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: 58739-done <at> debbugs.gnu.org
Subject: Re: bug#58739: Fixed
Date: Sat, 29 Oct 2022 13:32:13 +0000

Bug fixed.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #25 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 58739 <at> debbugs.gnu.org, akrl <at> sdf.org
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Sat, 29 Oct 2022 16:04:22 +0000

On Sat, Oct 29, 2022 at 15:48:35 +0300, Eli Zaretskii wrote:
> > Cc: 58739 <at> debbugs.gnu.org
> > Date: Sat, 29 Oct 2022 12:04:36 +0000
> > From: Alan Mackenzie <acm <at> muc.de>

> > > to me this fix looks like a good idea (assuming changing the definition
> > > of MANY is acceptable).

> > We'll see what Eli says.

> About what, may I ask?

My patch for this bug.

As a quick summary, the cause of spurious warning messages in the byte
compiler was subr-arity returning (12 . many) rather than the correct
(12 . 12) for a native compiled function with exactly 12 parameters.

I've corrected this, by making subr-arity return (12 . 12), and I
committed the patch earlier on this afternoon.

In the process, MANY came to be defined to mean "has &rest parameters".

I think you'll be interested in the patch.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #28 received at 58739 <at> debbugs.gnu.org (full text, mbox):

From: Andrea Corallo <akrl <at> sdf.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 58739 <at> debbugs.gnu.org
Subject: Re: bug#58739: Lack of error message about number of args (?native
 compilation?)
Date: Mon, 31 Oct 2022 13:15:18 +0000

Alan Mackenzie <acm <at> muc.de> writes:

> Hello, Andrea.
>
> On Tue, Oct 25, 2022 at 20:15:38 +0000, Andrea Corallo wrote:
>> Alan Mackenzie <acm <at> muc.de> writes:
>
>> > On Sun, Oct 23, 2022 at 12:12:49 +0000, Alan Mackenzie wrote:
>
> [ .... ]
>
>> >> 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?
>
>> Hi Alan,
>
>> to me this fix looks like a good idea (assuming changing the definition
>> of MANY is acceptable).
>
> We'll see what Eli says.
>
>> I think also that throwing the error in 'declare_imported_func' is okay
>> at this point.
>
> Apologies at this point.  I should have produced an error from this
> before bothering you.  I was unable to produce such an error, and I've
> spent the last few days understanding what happens here, with this
> result:
>
> A call from a native compiled function is always in the form nargs +
> *args, regardless of whether there are more than 8 arguments or not.
> More accurately, the call to an unknown type of function (.eln/.elc/.el)
> puts the function (in some form, I don't know exactly what) at element 0
> of *args, and the arguments themselves starting at element 1 of *args.
> It then calls Ffuncall (or something like it).
>
> When the called function is a byte-code function, further down the call
> stack exec_byte_code gets called with this (nargs + 1) + *args.
> exec_byte_code then pushes the arguments onto the stack before
> interpreting the byte code.
>
> So this "problem" with the native compiler not knowing what call sequence
> to generate isn't a problem at all.  It's all dealt with at run time.  No
> doubt this slows down Emacs quite a bit, but it's safe.
>
> So I'll take that bit out of my patch, and commit the rest of it to
> master, and see what happens from there.

Hi Alan,

That's correct.  As you prefer, we could also keep it as assertion.

Thanks

  Andrea

Previous Next

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