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

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 20 Dec 2024 20:50:56 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: gerd.moellmann@gmail.com, spd@toadstyle.org, acorallo@gnu.org, 
>> monnier@iro.umontreal.ca, 74966@debbugs.gnu.org
>> 
>> "Eli Zaretskii" <eliz@gnu.org> writes:
>> 
>> > TBH, I'd be much happier without splitting 'doc' into two members.
>> > Isn't it possible to avoid overwriting 'doc', thus avoiding the crash,
>> > without splitting the member in two?
>> 
>> Absolutely.  That'd cause wrong docstrings for some symbols
>
> Can you elaborate on that?  When and how could that happen?
>
>> but reduce
>> the risk of code not compiling anymore.  There is no risk of unvetted
>> code compiling and accessing the doc field, because it's been renamed,
>> but that does require, of course, to consider the changes in the patch
>> as "vetted"; furthermore, if there is a compilation error in some code
>> I've missed, we need to be careful not to fix it without checking that
>> code, too.
>
> I didn't have in mind problems that will be caught by the compiler --
> these are easy to find and fix, even if it takes some time because
> some code is only compiled on a certain rarely-used platform or
> configuration.
>
>> > DOC handling in Emacs is extremely complex and has many subtle
>> > aspects.  Experience teaches us that bugs in this area sometimes take
>> > years to report and fix.  Splitting a member into two risks
>> > introducing bugs because we might use the wrong member in some
>> > situation(s).
>> 
>> I don't understand this specific objection.  If we rename "doc" to
>> "doc_offset", as I've proposed, we're automatically looking at all code
>> which makes use of the new field, because code using the old name no
>> longer compiles.
>
> Yes, sure.  If we just rename a single member and all of its
> references, there's no risk whatsoever.  I had something else in mind.
> If we split the 'doc' member in two, some code which used 'doc' will
> need to use 'doc_index', and some code will need to use 'doc_offset'.
> The risk is in using the wrong one, because the intent of the original
> code (i.e., whether it interprets 'doc' as an index or as an offset)
> was not completely understood.  Moreover, it is possible that in some
> places we'd need to set the value of one of these two members from the
> value of the other, because the original code relied on the fact that
> there's just one member whose value is sometimes interpreted as this
> and sometimes as that.
>
>> > The test suite's coverage of doc.c and features related
>> > to doc strings is still rudimentary, so we cannot be sure any such
>> > bugs will be caught in time by the tests.  Which is why I hope we can
>> > avoid splitting this member in two.
>> 
>> I agree that the docfile hack in general is very hairy territory.  It
>> might be safer to simply accept the wrong docstrings for now, and Gerd's
>> patch does that, IIUC.  Can we just apply that for now and add a FIXME?
>
> Maybe, but I'd like first to understand better the "wrong docstrings"
> situations.

Maybe I can explain this. The current error's cause is that nativecomp
currently (wrongly) assumes that it never needs an offset into DOC, so
it reused Lisp_Subr::doc for its own purposes, namely as an index into a
vector contained in an .eln file. The doc string of a function is found
at that index.

Pip's patch changes nativecomp code stores the said index from above in
a new Lisp_Subr::doc_index, and makes it possible to fix the wrong
assumption from above, Nativecomp of course has to check which of the
now two members it has to use, which Pip does.

Renaming Lisp_Subr::doc to Lisp_Object::doc_offset is a nice extra, IMO,
because it makes it clearer what the member contains.