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

[Andrea Corallo <acorallo@gnu.org>]

Pip Cet <pipcet@protonmail.com> writes:

> "Andrea Corallo" <acorallo@gnu.org> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> From: Andrea Corallo <acorallo@gnu.org>
>>>> Cc: Pip Cet <pipcet@protonmail.com>,  monnier@iro.umontreal.ca,
>>>>   gerd.moellmann@gmail.com,  spd@toadstyle.org,  74966@debbugs.gnu.org
>>>> Date: Tue, 31 Dec 2024 05:22:11 -0500
>>>>
>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>
>>>> >> > I think a much simpler change is to use the sign bit to distinguish indices
>>>> >> > into the constant vector from indices into the DOC file.
>>>> >>
>>>> >> And use one's-complement, I assume, to guard against some future weird
>>>> >> nativecomp change resulting in the index -0?  :-)
>>>> >>
>>>> >> I really have no strong preference here.
>>>> >
>>>> > And I still want to hear from Andrea.  It's his code, so the solution
>>>> > he prefers gets my vote.
>>>>
>>>> Sorry for behing late here.
>>>>
>>>> I'm for the sign bit, it saves memory (why not), and we can sanity check
>>>> that when the bit is set the function is a non-primitve one and vice
>>>> versa.
>>>
>>> OK, can you post a patch (or even install it)?
>>
>> Will do.  I'll just need some time, I'm catching up with mails after
>> holidays while being sick at the same time.
>
> Sorry to hear it.  Get well soon!

Thanks

>> If someone is motivate to
>> jump on the task before me feel free.
>
> Here's a patch (IIRC, C doesn't require ~x = -x - 1, so I used
> the more explicit notation to avoid compiler warnings).
>
> commit 9654100f0ecc312b14ba56bf2e13691168342cf9 (HEAD -> signbit)
> Author: Pip Cet <pipcet@protonmail.com>
> Date:   Tue Jan 7 12:52:16 2025 +0000
>
>     Fix store_function_docstring for native subrs (Bug#74966)
>     
>     * src/comp.c (native_function_doc):
>     (make_subr): Use one's complement of doc index.
>     * src/doc.c (store_function_docstring): Add assertion.
>     * src/lisp.h (struct Lisp_Subr): Document 'doc' sign bit.
>
> diff --git a/src/comp.c b/src/comp.c
> index 70a9a64a714..b96fae4ae95 100644
> --- a/src/comp.c
> +++ b/src/comp.c
> @@ -5488,7 +5488,10 @@ native_function_doc (Lisp_Object function)
>    if (!VECTORP (cu->data_fdoc_v))
>      xsignal2 (Qnative_lisp_file_inconsistent, cu->file,
>  	      build_string ("missing documentation vector"));
> -  return AREF (cu->data_fdoc_v, XSUBR (function)->doc);
> +  EMACS_INT doc = XSUBR (function)->doc;
> +  if (doc < 0)
> +    return AREF (cu->data_fdoc_v, -doc - 1);
> +  return make_fixnum (doc);
>  }
>  
>  static Lisp_Object
> @@ -5529,7 +5532,8 @@ make_subr (Lisp_Object symbol_name, Lisp_Object minarg, Lisp_Object maxarg,
>    x->s.symbol_name = xstrdup (SSDATA (symbol_name));
>    x->s.intspec.native = intspec;
>    x->s.command_modes = command_modes;
> -  x->s.doc = XFIXNUM (doc_idx);
> +  x->s.doc = -XFIXNUM (doc_idx) - 1;
> +  eassert (x->s.doc < 0);
>  #ifdef HAVE_NATIVE_COMP
>    x->s.native_comp_u = comp_u;
>    x->s.native_c_name = xstrdup (SSDATA (c_name));
> diff --git a/src/doc.c b/src/doc.c
> index 88be9121dab..04afe50d3dd 100644
> --- a/src/doc.c
> +++ b/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;
> +    {
> +      XSUBR (fun)->doc = offset;
> +      eassert (XSUBR (fun)->doc >= 0);
> +    }
>    else if (CLOSUREP (fun))
>      {
>        /* This bytecode object must have a slot for the docstring, since
> diff --git a/src/lisp.h b/src/lisp.h
> index 339ff5e83b0..0979fedd846 100644
> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -2228,6 +2228,9 @@ CHAR_TABLE_SET (Lisp_Object ct, int idx, Lisp_Object val)
>        Lisp_Object native;
>      } intspec;
>      Lisp_Object command_modes;
> +    /* positive values: offset into etc/DOC.  Negative values: one's
> +       complement of index into the native comp unit's constant
> +       vector.  */
>      EMACS_INT doc;
>  #ifdef HAVE_NATIVE_COMP
>      Lisp_Object native_comp_u;

LGTM thanks