Re: git: f6ac79fb12f3 - main - Introduce the PROC_SIGCODE() macro

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Mon, 18 Jul 2022 17:06:21 UTC
On 7/18/22 7:29 AM, Kornel Dulęba wrote:
> The branch main has been updated by kd:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=f6ac79fb12f3c7ad891849d6721a20a100f6a9a9
> 
> commit f6ac79fb12f3c7ad891849d6721a20a100f6a9a9
> Author:     Kornel Dulęba <kd@FreeBSD.org>
> AuthorDate: 2022-06-02 08:45:54 +0000
> Commit:     Kornel Dulęba <kd@FreeBSD.org>
> CommitDate: 2022-07-18 14:27:26 +0000
> 
>      Introduce the PROC_SIGCODE() macro
>      
>      Use a getter macro instead of fetching the sigcode address directly
>      from a sysent of a given process. It assumes that the sigcode is stored
>      in the shared page, which is true in all cases, except for a.out
>      binaries. This will be later useful when the shared page address
>      randomization is introduced.
>      No functional change intended.

In CheriBSD we have a local 'p_md.md_sigcode' (which could really just
be a 'p_sigcode') that is a user pointer directly to the sigcode.  This
means, e.g. that exec_copyout_strings could set it to the address it
copies a trampoline out to directly rather than duplicating the same
expression in all the various sendsig functions.  That is likely still
a cleaner approach even for FreeBSD as it means the various places you
just changed to all be duplicates in followup commits could instead just
set p_sigcode directly in a single place for all arches (probably I would
have exec_copyout_strings set it to the right per-process location for
the shared page in an 'else' branch opposite the copyout of the sigcode).

> diff --git a/sys/riscv/riscv/exec_machdep.c b/sys/riscv/riscv/exec_machdep.c
> index 2d30ba9cb01c..d45e8b808f74 100644
> --- a/sys/riscv/riscv/exec_machdep.c
> +++ b/sys/riscv/riscv/exec_machdep.c
> @@ -416,7 +416,7 @@ sendsig(sig_t catcher, ksiginfo_t *ksi, sigset_t *mask)
>   
>   	sysent = p->p_sysent;
>   	if (sysent->sv_sigcode_base != 0)
> -		tf->tf_ra = (register_t)sysent->sv_sigcode_base;
> +		tf->tf_ra = (register_t)PROC_SIGCODE(p);
>   	else
>   		tf->tf_ra = (register_t)(PROC_PS_STRINGS(p) -
>   		    *(sysent->sv_szsigcode));

In particular, (to pick an example) this would mean that all these sendsig
functions would instead just become:

    tf->tf_ra = (register_t)p->p_sigcode;

And the logic for choosing between these two options would instead be in a
single place in exec_copyout_strings() to control setting p_sigcode.

-- 
John Baldwin