Re: [PATCH v2 15/19] Ignore clauses: some clauses are impossible regardless of uprp

[Kris Van Hees <kris.van.hees@oracle.com>]

On Tue, Sep 24, 2024 at 04:25:54PM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> In ignore_clauses, for an underlying probe uprp, we try to
> decide if we can safely ignore clause n.
> 
> Meanwhile, for some clauses, the probe description tells us the
> clause will not be called for any USDT probe, regardless of the
> underlying probe.  For example, "syscall::write:" can safely be
> ignored, for all uprp.
> 
> Add a dtsd_usdt variable to each statement to track status:
> 
>     USDT_FLAG_UNINITIALIZED  not yet initialized
> 
>     USDT_FLAG_POSSIBLE       clause could possibly be called
>                              for some USDT probe
> 
>     USDT_FLAG_IGNORE         clause can safely be ignored for
>                              all USDT probes

I think it would be better to use the dtsd_clauseflags member for this.
You can add dt_stmt_set_flag() and dt_stmt_test_flag() (or similar names)
to set and test for specific bits in the dtsd_clauseflags member using
DT_CLSFLAG_* constants.  You should be OK with just 2, one to indicate
POSSIBLE and one to indicate IGNORE (neither being set obviously meansnot
yet initialized).

I don't really know what symbol names would be best... perhaps for now use
DT_CLSFLAG_USDT_INCLUDE and DT_CLSFLAG_USDT_EXCLUDE?

Main thing I would like to accomplish here is simply to access flags on the
statements through functions rather than accessing them directly.

> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  libdtrace/dt_prov_uprobe.c | 56 ++++++++++++++++++++++++++++++++++++--
>  libdtrace/dtrace.h         |  1 +
>  2 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 51ad3b55f..938c5784a 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -27,6 +27,7 @@
>   */
>  #include <sys/types.h>
>  #include <assert.h>
> +#include <ctype.h>
>  #include <errno.h>
>  #include <string.h>
>  
> @@ -232,6 +233,10 @@ grow_strtab(dtrace_hdl_t *dtp)
>  	return 0;
>  }
>  
> +#define USDT_FLAG_UNINITIALIZED	0
> +#define USDT_FLAG_POSSIBLE	1
> +#define USDT_FLAG_IGNORE	2
> +
>  /*
>   * Judge whether clause "n" could ever be called as a USDT probe
>   * for this underlying probe.
> @@ -239,7 +244,53 @@ grow_strtab(dtrace_hdl_t *dtp)
>  static int
>  ignore_clause(dtrace_hdl_t *dtp, int n, const dt_probe_t *uprp)
>  {
> -	/* To be safe, ignore nothing. */
> +	dtrace_probedesc_t	*pdp = &dtp->dt_stmts[n]->dtsd_ecbdesc->dted_probe;
> +	int			*usdt_stat = &dtp->dt_stmts[n]->dtsd_usdt;
> +
> +	/*
> +	 * Some clauses could never be called for a USDT probe,
> +	 * regardless of the underlying probe uprp.  Cache this
> +	 * status in dt_stmts[n]->dtsd_usdt (pointed to by usdt_stat).
> +	 */
> +	if (*usdt_stat == USDT_FLAG_UNINITIALIZED) {
> +		char lastchar = pdp->prv[strlen(pdp->prv) - 1];
> +
> +		/*
> +		 * If the last char in the provider description is
> +		 * neither '*' nor a digit, it cannot be a USDT probe.
> +		 */
> +		if (lastchar != '*' && !isdigit(lastchar)) {
> +			*usdt_stat = USDT_FLAG_IGNORE;
> +			return 1;
> +		}
> +
> +		/*
> +		 * If the provider description is "pid[0-9]*", it
> +		 * is a pid probe, not USDT.
> +		 */
> +		if (strncmp(pdp->prv, "pid", 3) == 0) {
> +			int i, l = strlen(pdp->prv);
> +
> +			for (i = 3; i < l; i++)
> +				if (!isdigit((pdp->prv[i])))
> +					break;
> +
> +			if (i == l) {
> +				*usdt_stat = USDT_FLAG_IGNORE;
> +				return 1;
> +			}
> +		}
> +
> +		/* Otherwise, it is possibly a USDT probe. */
> +		*usdt_stat = USDT_FLAG_POSSIBLE;
> +	}
> +	if (*usdt_stat == USDT_FLAG_IGNORE)
> +		return 1;
> +
> +	/*
> +	 * If USDT_FLAG_POSSIBLE, try to use uprp.
> +	 */
> +
>  	return 0;
>  }
>  
> @@ -271,7 +322,8 @@ static void update_uprobe(dtrace_hdl_t *dtp, void *datap)
>  
>  		stp = dtp->dt_stmts[i];
>  		assert(stp != NULL);
> -		dt_pid_create_usdt_probes(&stp->dtsd_ecbdesc->dted_probe, dtp, &pcb);
> +		if (stp->dtsd_usdt != USDT_FLAG_IGNORE)
> +			dt_pid_create_usdt_probes(&stp->dtsd_ecbdesc->dted_probe, dtp, &pcb);
>  	}
>  
>  	while (prid < dtp->dt_probe_id) {
> diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
> index 0f716cd40..f99fb5b6e 100644
> --- a/libdtrace/dtrace.h
> +++ b/libdtrace/dtrace.h
> @@ -151,6 +151,7 @@ typedef struct dtrace_stmtdesc {
>  	dtrace_attribute_t dtsd_stmtattr;	/* statement attributes */
>  	int dtsd_clauseflags;			/* clause flags */
>  	int dtsd_id;				/* index in dtp->dt_stmts */
> +	int dtsd_usdt;				/* flags describing USDT use, see dt_prov_uprobe.c */
>  } dtrace_stmtdesc_t;
>  
>  /* dtsd clause flags */
> -- 
> 2.43.5
>