[DTrace-devel] [PATCH 1/3] Ensure proper probe compilation context is set for trampolines

[Kris Van Hees]

On Fri, Jul 02, 2021 at 02:33:38PM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> ... but if there is a user-visible difference with this patch alone, 
> would it make sense to add a test?  Or, would that wait until a later patch?

No user visible effect from this - but it is necessary for the following
patches (especially 3/3).  That last one could use a patch but I haven't found
a nice and stable one just yet because tracepoints are not the most stable
thing between kernel releases.  It's a bit of a challenge similar to the stack
tests and basing them on a specific function.

> On 7/2/21 2:18 PM, Kris Van Hees wrote:
> > Trampolines are generated per-probe, well after the clauses have been
> > compiled.  The code generation for the trampolines was not setting the
> > proper probe context which could lead to obscure issues like not having
> > the proper probe argument information when the trampoline is generated.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> >   libdtrace/dt_cc.c   | 9 ++++++---
> >   libdtrace/dt_impl.h | 2 +-
> >   libdtrace/dt_pid.c  | 9 +++++----
> >   3 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> > index 9c05e831..2a6904a0 100644
> > --- a/libdtrace/dt_cc.c
> > +++ b/libdtrace/dt_cc.c
> > @@ -1560,7 +1560,7 @@ dt_compile_xlator(dt_node_t *dnp)
> >   }
> >   
> >   void
> > -dt_setcontext(dtrace_hdl_t *dtp, dtrace_probedesc_t *pdp)
> > +dt_setcontext(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> >   {
> >   	const dtrace_pattr_t *pap;
> >   	dt_probe_t *prp;
> > @@ -1585,7 +1585,7 @@ dt_setcontext(dtrace_hdl_t *dtp, dtrace_probedesc_t *pdp)
> >   	    isdigit(pdp->prv[strlen(pdp->prv) - 1]) &&
> >   	    ((pvp = dt_provider_lookup(dtp, pdp->prv)) == NULL ||
> >   	     pvp->pv_flags & DT_PROVIDER_PID) &&
> > -	    dt_pid_create_probes(pdp, dtp, yypcb) != 0) {
> > +	    dt_pid_create_probes((dtrace_probedesc_t *)pdp, dtp, yypcb) != 0) {
> >   		longjmp(yypcb->pcb_jmpbuf, EDT_COMPILER);
> >   	}
> >   
> > @@ -2117,7 +2117,7 @@ dt_construct(dtrace_hdl_t *dtp, dt_probe_t *prp, uint_t cflags, dt_ident_t *idp)
> >   	pcb.pcb_context = DT_CTX_DPROG;
> >   	pcb.pcb_token = DT_CTX_DPROG;
> >   	pcb.pcb_probe = prp;
> > -	pcb.pcb_pdesc = NULL;
> > +	pcb.pcb_pdesc = prp->desc;
> >   
> >   	if ((err = setjmp(yypcb->pcb_jmpbuf)) != 0)
> >   		goto out;
> > @@ -2134,6 +2134,7 @@ dt_construct(dtrace_hdl_t *dtp, dt_probe_t *prp, uint_t cflags, dt_ident_t *idp)
> >   	if (yypcb->pcb_pragmas != NULL)
> >   		dt_idhash_iter(yypcb->pcb_pragmas, dt_idpragma, NULL);
> >   
> > +	dt_setcontext(dtp, yypcb->pcb_pdesc);
> >   	tnp = dt_node_trampoline(prp);
> >   	dt_node_type_assign(tnp, dtp->dt_ints[0].did_ctfp,
> >   				 dtp->dt_ints[0].did_type);
> > @@ -2164,6 +2165,8 @@ out:
> >   	     ctf_write(dtp->dt_ddefs->dm_ctfp, dtp->dt_ddefs_fd) == CTF_ERR))
> >   		dt_dprintf("failed to update CTF cache: %s\n", strerror(errno));
> >   
> > +	dt_endcontext(dtp);
> > +
> >   	dt_pcb_pop(dtp, err);
> >   	dt_set_errno(dtp, err);
> >   
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > index d3e133e7..bd8d9943 100644
> > --- a/libdtrace/dt_impl.h
> > +++ b/libdtrace/dt_impl.h
> > @@ -698,7 +698,7 @@ extern uint64_t dt_stddev(uint64_t *, uint64_t);
> >   
> >   extern int dt_options_load(dtrace_hdl_t *);
> >   
> > -extern void dt_setcontext(dtrace_hdl_t *, dtrace_probedesc_t *);
> > +extern void dt_setcontext(dtrace_hdl_t *, const dtrace_probedesc_t *);
> >   extern void dt_endcontext(dtrace_hdl_t *);
> >   
> >   extern void dt_dlib_init(dtrace_hdl_t *dtp);
> > diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> > index dc710926..36e8e749 100644
> > --- a/libdtrace/dt_pid.c
> > +++ b/libdtrace/dt_pid.c
> > @@ -504,7 +504,7 @@ dt_pid_fix_mod(dt_pid_probe_t *pp, dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp,
> >   
> >   static int
> >   dt_pid_create_pid_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp,
> > -    dt_pcb_t *pcb, dt_proc_t *dpr)
> > +			 dt_pcb_t *pcb, dt_proc_t *dpr)
> >   {
> >   	dt_pid_probe_t pp;
> >   	int ret = 0;
> > @@ -685,8 +685,8 @@ dt_pid_create_usdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp,
> >   #endif
> >   
> >   static pid_t
> > -dt_pid_get_pid(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb,
> > -    dt_proc_t *dpr)
> > +dt_pid_get_pid(const dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb,
> > +	       dt_proc_t *dpr)
> >   {
> >   	pid_t pid;
> >   	char *end;
> > @@ -741,7 +741,8 @@ dt_pid_create_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb)
> >   		dpr = dt_proc_lookup(dtp, pid);
> >   		assert(dpr != NULL);
> >   
> > -		if ((err = dt_pid_create_pid_probes(pdp, dtp, pcb, dpr)) == 0) {
> > +		err = dt_pid_create_pid_probes(pdp, dtp, pcb, dpr);
> > +		if (err == 0) {
> >   			/*
> >   			 * Alert other retained enablings which may match
> >   			 * against the newly created probes.
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel