[DTrace-devel] [PATCH 1/3] Allocate exit label in dt_cg_tramp_prologue rather than each provider

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>


On 09/29/2020 04:14 PM, Kris Van Hees wrote:
> Every provider implementation was allocating an id for the exit label
> that is used in the trampoline prologue and epilogue.  Now the label
> id is allocated in and returned by dt_cg_tramp_prologue() (and its
> *_act() variant).  That id is to be passed to dt_cg_tramp_epilogue()
> (or the *_advance() variant) to ensure that the same label is used
> throughout the trampoline.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c           | 15 ++++++++++-----
>   libdtrace/dt_cg.h           |  5 ++---
>   libdtrace/dt_prov_dtrace.c  |  4 ++--
>   libdtrace/dt_prov_fbt.c     |  4 ++--
>   libdtrace/dt_prov_profile.c |  4 ++--
>   libdtrace/dt_prov_sdt.c     |  4 ++--
>   libdtrace/dt_prov_syscall.c |  4 ++--
>   7 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index f5ed3f2e..d43cea41 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -38,13 +38,16 @@ static void dt_cg_node(dt_node_t *, dt_irlist_t *, dt_regset_t *);
>    *
>    * The caller should NOT depend on any register values that exist at the end of
>    * the trampoline prologue.
> + *
> + * This function returns a label id to be passed to dt_cg_tramp_epilogue().
>    */
> -void
> -dt_cg_tramp_prologue_act(dt_pcb_t *pcb, uint_t lbl_exit, dt_activity_t act)
> +uint_t
> +dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>   {
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	dt_ident_t	*mem = dt_dlib_get_map(pcb->pcb_hdl, "mem");
>   	dt_ident_t	*state = dt_dlib_get_map(pcb->pcb_hdl, "state");
> +	uint_t		lbl_exit = dt_irlist_label(dlp);
>   	struct bpf_insn	instr;
>   
>   	assert(mem != NULL);
> @@ -143,12 +146,14 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, uint_t lbl_exit, dt_activity_t act)
>   	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>   	instr = BPF_STORE(BPF_DW, BPF_REG_FP, DCTX_FP(DCTX_BUF), BPF_REG_0);
>   	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> +
> +	return lbl_exit;
>   }
>   
> -void
> -dt_cg_tramp_prologue(dt_pcb_t *pcb, uint_t lbl_exit)
> +uint_t
> +dt_cg_tramp_prologue(dt_pcb_t *pcb)
>   {
> -	dt_cg_tramp_prologue_act(pcb, lbl_exit, DT_ACTIVITY_ACTIVE);
> +	return dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
>   }
>   
>   static int
> diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
> index a4c6fb28..29678e2f 100644
> --- a/libdtrace/dt_cg.h
> +++ b/libdtrace/dt_cg.h
> @@ -19,9 +19,8 @@ extern "C" {
>   extern void dt_cg(dt_pcb_t *, dt_node_t *);
>   extern void dt_cg_xsetx(dt_irlist_t *, dt_ident_t *, uint_t, int, uint64_t);
>   extern dt_irnode_t *dt_cg_node_alloc(uint_t, struct bpf_insn);
> -extern void dt_cg_tramp_prologue_act(dt_pcb_t *pcb, uint_t lbl_exit,
> -				     dt_activity_t act);
> -extern void dt_cg_tramp_prologue(dt_pcb_t *pcb, uint_t lbl_exit);
> +extern uint_t dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act);
> +extern uint_t dt_cg_tramp_prologue(dt_pcb_t *pcb);
>   extern void dt_cg_tramp_epilogue(dt_pcb_t *pcb, uint_t lbl_exit);
>   extern void dt_cg_tramp_epilogue_advance(dt_pcb_t *pcb, uint_t lbl_exit);
>   
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index a0c8d007..1de99ab5 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -75,7 +75,7 @@ static void trampoline(dt_pcb_t *pcb)
>   	int		i;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	struct bpf_insn	instr;
> -	uint_t		lbl_exit = dt_irlist_label(dlp);
> +	uint_t		lbl_exit;
>   	dt_activity_t	act;
>   	int		adv_act;
>   	uint32_t	key = 0;
> @@ -114,7 +114,7 @@ static void trampoline(dt_pcb_t *pcb)
>   		adv_act = 0;
>   	}
>   
> -	dt_cg_tramp_prologue_act(pcb, lbl_exit, act);
> +	lbl_exit = dt_cg_tramp_prologue_act(pcb, act);
>   
>   	if (key) {
>   		/*
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 842ba53b..1ba144a5 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -164,9 +164,9 @@ static void trampoline(dt_pcb_t *pcb)
>   	int		i;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	struct bpf_insn	instr;
> -	uint_t		lbl_exit = dt_irlist_label(dlp);
> +	uint_t		lbl_exit;
>   
> -	dt_cg_tramp_prologue(pcb, lbl_exit);
> +	lbl_exit = dt_cg_tramp_prologue(pcb);
>   
>   	/*
>   	 * We cannot assume anything about the state of any registers so set up
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index 0e90351f..8541bee6 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -218,9 +218,9 @@ static void trampoline(dt_pcb_t *pcb)
>   	int		i;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	struct bpf_insn	instr;
> -	uint_t		lbl_exit = dt_irlist_label(dlp);
> +	uint_t		lbl_exit;
>   
> -	dt_cg_tramp_prologue(pcb, lbl_exit);
> +	lbl_exit = dt_cg_tramp_prologue(pcb);
>   
>   	/*
>   	 * We cannot assume anything about the state of any registers so set up
> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> index fbfdb7fe..4b2ac31b 100644
> --- a/libdtrace/dt_prov_sdt.c
> +++ b/libdtrace/dt_prov_sdt.c
> @@ -396,9 +396,9 @@ static void trampoline(dt_pcb_t *pcb)
>   	int		i;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	struct bpf_insn	instr;
> -	uint_t		lbl_exit = dt_irlist_label(dlp);
> +	uint_t		lbl_exit;
>   
> -	dt_cg_tramp_prologue(pcb, lbl_exit);
> +	lbl_exit = dt_cg_tramp_prologue(pcb);
>   
>   	/*
>   	 * We cannot assume anything about the state of any registers so set up
> diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> index b817f528..341a18c8 100644
> --- a/libdtrace/dt_prov_syscall.c
> +++ b/libdtrace/dt_prov_syscall.c
> @@ -149,9 +149,9 @@ static void trampoline(dt_pcb_t *pcb)
>   	int		i;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
>   	struct bpf_insn	instr;
> -	uint_t		lbl_exit = dt_irlist_label(dlp);
> +	uint_t		lbl_exit;
>   
> -	dt_cg_tramp_prologue(pcb, lbl_exit);
> +	lbl_exit = dt_cg_tramp_prologue(pcb);
>   
>   	/*
>   	 * We cannot assume anything about the state of any registers so set up