bug#75322: SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string)

[Daniel Colascione <dancol@dancol.org>]

On January 6, 2025 10:12:53 AM EST, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Daniel Colascione <dancol@dancol.org>
>> Cc: gerd.moellmann@gmail.com,  pipcet@protonmail.com,  75322@debbugs.gnu.org
>> Date: Mon, 06 Jan 2025 09:48:09 -0500
>> 
>> I wouldn't call it a "rewrite".  If auditing the codebase for memory
>> safety is a "rewrite", I'm a "duck".  We're talking about a few hundred
>> lines of changes at the most.  Most of the work is just auditing the
>> code for problems.  We should be grateful Gerd has done this work
>> already, not "run away from MPS, fast".
>
>I _am_ grateful to Gerd (and Helmut, and Pip, and others who work on
>this).  I also invested a significant, albeit smaller, effort on my
>part into this branch.  However, the potential amount of changes still
>bothers me.  I understand it doesn't bother you, so I guess we
>disagree in our estimations.
>
>> > 	    SAFE_NALLOCA (args2, 1, nargs + 1);
>> > 	    args2[0] = Qcall_process;
>> > 	    for (i = 0; i < nargs; i++) args2[i + 1] = args[i];
>> > 	    coding_systems = Ffind_operation_coding_system (nargs + 1, args2);
>> > 	    val = CONSP (coding_systems) ? XCDR (coding_systems) : Qnil;
>> >
>> > "Look, ma: no pointers!"
>> 
>>       Lisp_Object val, *args2;
>> 
>> In the C programming language, "*" means "pointer".
>
>Are we going to argue about pointers and arrays?
>
>> > So this code needs to be changed.
>> 
>> The snippet you quoted above can be fixed with a one-liner --- replace
>> SAFE_NALLOCA with SAFE_ALLOCA_LISP.
>
>It's just one example, and there are many like it.  So that one-liner
>is multiplied many times.
>
>And then we have variations, where args[] gets text of strings or some
>other similar stuff.  Etc. etc.
>
>> > And if you look around, we have quite a lot of these in many places.
>> 
>> Sounds like Gerd's spent some time hunting them down.
>
>Sure, but I'm afraid there are many more.
>
>> > We have almost 200 static
>> > Lisp_Object variables, probably not all of them staticpro'd (8 of them
>> > inside functions, like the above example, so definitely not
>> > staticpro'd).  So now we need to examine the uses of all of them and
>> > either staticpro them or do something else (like move the assignment
>> > to 'last_coding' to after call_some_function).
>> 
>> Changing eight variables from function statics to file statics hardly
>> seems like a monumental effort.
>
>After you found them, and after you know they should be changed, yes.
>It's easy to account for the knowns; the problem is always the
>unknowns.  That's why most effort estimations are inaccurate.  I
>wonder what are our unknowns here, and how many of them are there.

I'm a lot less worried than you are about the unknown unknowns. I have a few specific reasons why:

1) we caught most of the movement-unsafe global references when we did pdumper, which, like MPS, moves things around in memory and so has to worry about the kind of unsafe references we're discussing here.

2) when I did my own moving GC a few years ago, I didn't run into serious problems with movement-unsafe references, although, like Gerd and others have done on the MPS branch, I had to fix a few

3) it should be possible (I don't know whether MPC implements this or not, but it could) to arrange for a debugging mode moving GC that moves *everything* not pinned from the from-space to a non-overlapping to-space, then applies PROT_NONE to any part of the from-space not used for a pinned object. This way, any GC-invisible Lisp_Obiects (or other pointers) "left behind" because we forgot to tell the GC about them will produce an immediate SIGSEGV when used. We could even conservatively scan for them. 

#3 is probably overkill, but it's something we could try if we attempted to merge MPS and found chronic problems 

>> The static-storage global-scope
>> Lisp_Object variables are probably almost all gcproed already.
>
>Maybe.  But someone needs to verify that, right?

A few people have already done things like this over the years.