Subject: bug#74966: 31.0.50; Crash report (using igc on macOS)

From: Gerd Möllmann <[email-masked]>
Date: Fri, 20 Dec 2024 09:57:39 +0100

Gerd Möllmann <[email-masked]> writes:

> Eli Zaretskii <[email-masked]> writes:
>
>>> From: Gerd Möllmann <[email-masked]>
>>> Cc: Andrea Corallo <[email-masked]>,  [email-masked],
>>>   [email-masked],  [email-masked]
>>> Date: Fri, 20 Dec 2024 09:21:38 +0100
>>> 
>>> Eli Zaretskii <[email-masked]> writes:
>>> 
>>> >> Cc: [email-masked], [email-masked]
>>> >> From: Gerd Möllmann <[email-masked]>
>>> >> Date: Fri, 20 Dec 2024 08:30:55 +0100
>>> >> 
>>> >> Gerd Möllmann <[email-masked]> writes:
>>> >> 
>>> >> > I think I'll read the code a bit now.
>>> >> 
>>> >> BTW, does C-h f x-file-dialog work on platforms != macOS?
>>> >
>>> > In what version of Emacs? and with or without native compilation?
>>> 
>>> master with native compilation, if possible with --enable-checking.
>>> 
>>> >
>>> > It works here as follows:
>>> >
>>> >   . On MS-Windows:
>>> >     - emacs 31 without native compilation
>>> >     - emacs 30 with and without native compilation
>>> >   . On GNU/Linux:
>>> >     - emacs 31 with and without native compilation
>>> >     - emacs 30 with and without native compilation
>>> >
>>> >> It aborts here for the same reason.
>>> >> 
>>> >> And I'm asking myself now why the heck it lands in native_function_doc in
>>> >> the first place. Isn't x-file-dialog a C function everywhere?
>>> >
>>> > It is a C function, yes.
>>> 
>>> My current theory is that one needs to C-h f a function with a doc
>>> string lie this one:
>>> 
>>> (defun x-file-dialog (prompt dir &optional default-filename
>>>                              mustmatch only-dir-p)
>>>   "SKIP: real doc in xfns.c."
>>>   (ns-read-file-name prompt dir mustmatch default-filename only-dir-p))
>>> 
>>> And the function must be native-comp-function-p.
>>> 
>>> I can't see so far how/where such "redirections" are handled.
>>
>> AFAIR, the doc string is in etc/DOC, and is collected by
>> lib-src/make-docfile.  That program scans the C source according to
>> what src/Makefile tells is (see the $(etc)/DOC rule there), so it
>> doesn't care whether xfns.c is or isn't compiled into the binary.
>
> Yes, that matches what I remember from the old times.
>
>> But we had many changes lately in how etc/DOC is handled, and maybe my
>> memory is no long accurate.  So I added Stefan to this discussion, who
>> made at least some of those DOC-related changes.
>
> Maybe the problem is in store_doc_string (doc.c:469 here), not sure.
> That function does
>
>   if (SUBRP (fun))
>     XSUBR (fun)->doc = offset;
>
> without checking if it's a native compiled function. That does look
> like a place where an invalid offset could land in the subr.
>
> I have no idea what's the right thing to do to fix this. Just not doing
> anything for native compiled function certainly isn't right.

That seems to be the cause, indeed. When I 

1 file changed, 4 insertions(+), 1 deletion(-)
src/doc.c | 5 ++++-

modified   src/doc.c
@@ -479,7 +479,10 @@ store_function_docstring (Lisp_Object obj, EMACS_INT offset)
     fun = XCDR (fun);
   /* Lisp_Subrs have a slot for it.  */
   if (SUBRP (fun))
-    XSUBR (fun)->doc = offset;
+    {
+      if (!NATIVE_COMP_FUNCTIONP (fun))
+	XSUBR (fun)->doc = offset;
+    }
   else if (CLOSUREP (fun))
     {
       /* This bytecode object must have a slot for the docstring, since

I don't get the assert anymore.

C-h f x-file-dialog shows the "redirection doc string" though, so it's not
a correct solution. I think I'll leave to the experts :-).