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

[Eli Zaretskii <eliz@gnu.org>]

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

> The static-storage global-scope
> Lisp_Object variables are probably almost all gcproed already.

Maybe.  But someone needs to verify that, right?