[DTrace-devel] [PATCH 06/14] Add clauses to per-uprobe list

[Kris Van Hees]

This seems like a very fragile mechanism to get a list of clauses that are
associated with a particular underlying probe.  It is assuming that
provide_probe() is called during the compilation of a clause, before it is
assigned an id, so anything that changes that would break this scheme.

I think this should probably use the mechanism we already have for adding
clauses to probes, but with a modification to allow us to add all clauses that
relate to a specific underlying probe to a single probe (perhaps the underlying
one, or some fake representative probe) so that we have this list of probes
available by the time the trampoline is generated, and we can just run through
that list and generate the code to perhaps bitmap checks etc.

I.e. I think inventing a new (fragile) mechanism is not warranted, and instead
we ought to be able to use what is already there.  It will need a bit of
design work to do so, but it leverages code we already have and a mechanism
that is known to work.

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_prov_uprobe.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index cace406d..34906aa0 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -62,6 +62,7 @@ typedef struct dt_uprobe {
>  	int		flags;
>  	tp_probe_t	*tp;
>  	dt_list_t	probes;		/* pid/USDT probes triggered by it */
> +	dt_list_t	clauses;	/* clauses possibly called by this uprobe */
>  } dt_uprobe_t;
>  
>  typedef struct list_probe {
> @@ -69,6 +70,11 @@ typedef struct list_probe {
>  	dt_probe_t	*probe;
>  } list_probe_t;
>  
> +typedef struct list_clause {
> +	dt_list_t	list;
> +	uint_t		clause;
> +} list_clause_t;
> +
>  static const dtrace_pattr_t	pattr = {
>  { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_ISA },
>  { DTRACE_STABILITY_PRIVATE, DTRACE_STABILITY_PRIVATE, DTRACE_CLASS_UNKNOWN },
> @@ -243,6 +249,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>  	dt_uprobe_t		*upp;
>  	dt_probe_t		*prp, *uprp;
>  	list_probe_t		*pop, *pup;
> +	list_clause_t		*cl;
>  
>  	snprintf(prv, sizeof(prv), "%s%d", psp->pps_prv, psp->pps_pid);
>  
> @@ -263,6 +270,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>  	/* Mark the provider as a PID-based provider. */
>  	pvp->pv_flags |= DT_PROVIDER_PID;
>  
> +	/* Look up or create the underlying probe. */
>  	uprp = create_underlying(dtp, psp);
>  	if (uprp == NULL)
>  		return -1;
> @@ -270,6 +278,17 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>  	upp = uprp->prv_data;
>  	upp->flags |= flags;
>  
> +	/* Add to this uprobe an index for the next clause to be assigned. */
> +	cl = dt_list_prev(&upp->clauses);
> +	if (cl == NULL || cl->clause != dtp->dt_clause_nextid) {
> +		cl = dt_zalloc(dtp, sizeof(list_clause_t));
> +		if (cl == NULL)
> +			return -1;
> +		cl->clause = dtp->dt_clause_nextid;
> +		dt_list_append(&upp->clauses, cl);
> +	}
> +
> +	/* Look up the overlying probe. */
>  	prp = dt_probe_lookup(dtp, &pd);
>  	if (prp != NULL) {
>  		/*
> -- 
> 2.42.0