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

From: Gerd Möllmann <[email-masked]>
Date: Sat, 04 Jan 2025 13:17:50 +0100

Pip Cet <[email-masked]> writes:

>> TBH, I couldn't follow your thoughts above with the EFAULT, syscalls and
>> so on.
>
> My understanding is that if there is a memory barrier in place for a
> string that a syscall tries to access, we get an -EFAULT from Linux, an
> EFAULT from glibc, and the syscall won't work.

Ah, I think I understand now, thanks. Thing is that string data is in
our leaf pool (AMCZ). AMCZ doesn't use barriers because there are no
references in its objects. Does that make sense?

> This is what makes valid_pointer_p work, for example.  (To the extent it
> does: valid_pointer_p assumes 16 bytes after the pointer are readable; I
> don't see why that is true for small objects).
>
> What makes this more difficult is that glibc and GCC disagree about what
> to do with invalid pointers even in the simplest case: glibc documents
> printf ("%s\n", NULL) to work, but GCC will rewrite it into puts (NULL),
> which crashes.  I'm worried that glibc might wrap a syscall incorrectly
> wrt EFAULT and SIGSEGV, in this case.
>
> Worse, if the syscall is in a fork()ed process, MPS machinery to remove
> the memory barrier might not be in place after the fork.  And who knows
> about posix_spawn action descriptors?  Or vfork?
>
>>>> Or one does it as you did in b0a209e9204, that's of course also safe.
>>>> For both old and new GC. (Don't remember if you mentioned it Pip, but
>>>> old GC moves string data as well, during string compaction, should GC
>>>> run).
>>>
>>> Ouch.  Yes, I remember now.
>>>
>>> Pip
>>
>> And today I see you reverted that commit. Is there something wrong with
>> it? I couldn't see something wrong, and for me VALUE(no root) >
>> VALUE(exact) VALUE(ambig).
>
> There were two reasons for the revert:
>
> 1. Eli asked me not to push the change right after I pushed.  I thought
> it would be best to restore the "before" state so we could discuss the
> solution.
>
> 2. For the non-MPS case, I rashly assumed it would be okay to remove the
> no-GC assumption that call_process apparently establishes (even though
> there is no comment saying so).  I'm not sure what I would do now; the
> old code seems buggy to me because Fexpand_file_name can call Lisp, but
> that bug affects only argv, not envp.  It may be best to fix the argv
> code but leave the envp code in its (once again) current fragile state,
> documenting precisely which assumptions are made there.
>
>> WRT Lisp_Object allocas, please tell if I should do that.
>
> Sorry, I don't understand.  Lisp_Objects shouldn't be allocated with
> SAFE_ALLOCA, but allocating them with SAFE_ALLOCA_LISP_EXTRA is fine.
> Pointers to string data cannot currently be safely allocated with
> SAFE_ALLOCA, but I'm not sure whether SAFE_ALLOCA_AMBIGUOUS or
> SAFE_ALLOCA_EXACT_POINTER would be the right thing to do.

My fault: I meant allocas used to store Lisp_Object in them, i.e.
Lisp_Object * :-).