[DTrace-devel] [PATCH v2] print() action: identify ctf object used for print

Kris Van Hees kris.van.hees at oracle.com
Wed Apr 24 17:19:59 UTC 2024


On Fri, Apr 19, 2024 at 10:27:11AM +0100, Alan Maguire via DTrace-devel wrote:
> [resending since appears to have bounced last time]
> 
> when generating code for print() action we need to identify source
> of CTF; is it shared CTF, cdefs or ddefs or another module?
> Use DTRACE_OBJ* values for the first three cases, falling back
> to using module name pointer as a record argument if these fail.

I am thinking about this...  It seems unnecessary to add the conditionals
in the code since every module *has* a name.  So just passing in the module
name pointer as value would work for all cases anyway.  No need to use any
special cases.

But that brings up a different issue...  should we be passing in pointer
values and expect that they can be retrieved and used as pointers again on
the consumer end.  While that currently would work, it feels a bit fragile.
Especially if we might have to deal with modules getting unloaded (and that
might result in the module getting removed from dtrace also - I don't think it
does right now, but if we start looking towards long running dtrace sessions,
that might become something to do).

Perhaps we should store the module *name* itself?  Slightly less efficient
but also less fragile?

> Then when consuming records we can use either shared CTF,
> cdef, ddef or module CTF as appropriate.  Nick pointed out that
> the initial solution will not work for module-defined types.
> 
> This fixes an issue encountered when using a shared kernel type
> with alloca()ed memory; DTrace assumed the type was in ddefs
> and it wasn't so it wasn't displayed.  This change fixes that and
> all tests continue to pass.  Also tested that it works with
> kernel module-defined types now:
> 
> dtrace: description 'fbt::ieee80211_rx_napi:entry ' matched 1 probe
> CPU     ID                    FUNCTION:NAME
>   7 123161          ieee80211_rx_napi:entry 0xffff9f8980fa08e0 = *
>                                             (struct ieee80211_hw) {
>                                              .conf = (struct
> ieee80211_conf) {
>                                               .listen_interval = (u16)10,
>                                               .long_frame_max_tx_count =
> (u8)4,
>                                               .short_frame_max_tx_count
> = (u8)7,
>                                              },
> 
> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
> ---
>  libdtrace/dt_cg.c      | 18 +++++++++++++++++-
>  libdtrace/dt_consume.c |  7 +++++--
>  libdtrace/dt_printf.c  | 24 +++++++++++++++++++++---
>  libdtrace/dt_printf.h  |  2 +-
>  4 files changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 27246a40..542fb887 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -20,6 +20,7 @@
>  #include <dt_dctx.h>
>  #include <dt_cg.h>
>  #include <dt_grammar.h>
> +#include <dt_module.h>
>  #include <dt_parser.h>
>  #include <dt_printf.h>
>  #include <dt_provider.h>
> @@ -2831,6 +2832,7 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp,
> dtrace_actkind_t kind)
>  	ctf_file_t	*fp = addr->dn_ctfp;
>  	ctf_id_t	type = addr->dn_type;
>  	char		n[DT_TYPE_NAMELEN];
> +	const char	*ctf_obj;
>  	size_t		size;
>   	type = ctf_type_reference(fp, type);
> @@ -2842,9 +2844,23 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp,
> dtrace_actkind_t kind)
>  			"print( ) argument #1 reference has type '%s' with size 0; cannot
> print( ) it.\n",
>  			ctf_type_name(fp, type, n, sizeof(n)));
>  -	/* reserve space for addr/type, data/size */
> +	/* reserve space for addr/type, ctf file identification, data/size */
>  	addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
>  			      sizeof(uint64_t), 8, NULL, type);
> +	/* Use DTRACE_OBJ_* where possible, falling back to module name if
> +	 * none of these match.
> +	 */
> +	if (fp == dtp->dt_shared_ctf) {
> +		ctf_obj = DTRACE_OBJ_KMODS;
> +	} else if (fp == dtp->dt_cdefs->dm_ctfp) {
> +		ctf_obj = DTRACE_OBJ_CDEFS;
> +	} else if (fp == dtp->dt_ddefs->dm_ctfp) {
> +		ctf_obj = DTRACE_OBJ_DDEFS;
> +	} else {
> +		ctf_obj = dt_module_lookup_by_ctf(dtp, fp)->dm_name;
> +	}
> +	dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> +		   sizeof(uint64_t), 8, NULL, (uint64_t)ctf_obj);
>  	data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
>  			      size, 8, NULL, size);
>  diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index dec2314b..0d6f2642 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1987,9 +1987,11 @@ static int
>  dt_print_print(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
>  	       const caddr_t buf)
>  {
> -	dtrace_recdesc_t *data_rec = rec + 1;
> +	dtrace_recdesc_t *ctf_rec = rec + 1;
> +	dtrace_recdesc_t *data_rec = rec + 2;
>  	size_t max_size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
>  	size_t size = (size_t)data_rec->dtrd_arg;
> +	const char *ctf_obj = (const char *)ctf_rec->dtrd_arg;
>  	uint64_t printaddr;
>   	if (size > max_size)
> @@ -1998,7 +2000,8 @@ dt_print_print(dtrace_hdl_t *dtp, FILE *fp,
> dtrace_recdesc_t *rec,
>  	if (dt_read_scalar(buf, rec, &printaddr) < 0)
>  		return dt_set_errno(dtp, EDT_PRINT);
>  -	return dt_print_type(dtp, fp, printaddr, (ctf_id_t)rec->dtrd_arg,
> +	return dt_print_type(dtp, fp, printaddr, ctf_obj,
> +			     (ctf_id_t)rec->dtrd_arg,
>  			     (caddr_t)buf + data_rec->dtrd_offset, size);
>  }
>  diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index 50842216..2b5e4e0a 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -14,6 +14,7 @@
>  #include <limits.h>
>  #include <port.h>
>  +#include <dt_module.h>
>  #include <dt_printf.h>
>  #include <dt_string.h>
>  #include <dt_impl.h>
> @@ -2236,13 +2237,30 @@ err:
>   int
>  dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
> -	      ctf_id_t type, caddr_t data, size_t size)
> +	      const char *ctf_obj, ctf_id_t type, caddr_t data, size_t size)
>  {
>  	struct dt_visit_arg dva;
>   	dva.dv_dtp = dtp;
>  	dva.dv_fp = fp;
> -	dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> +
> +	if (ctf_obj == DTRACE_OBJ_KMODS) {
> +		dva.dv_ctfp = dtp->dt_shared_ctf;
> +	} else if (ctf_obj == DTRACE_OBJ_CDEFS) {
> +		dva.dv_ctfp = dtp->dt_cdefs->dm_ctfp;
> +	} else if (ctf_obj == DTRACE_OBJ_DDEFS) {
> +		dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> +	} else {
> +		struct dt_module *dm = dt_module_lookup_by_name(dtp, ctf_obj);
> +
> +		if (dm)
> +			dva.dv_ctfp = dt_module_getctf(dtp, dm);
> +		if (!dm || !dva.dv_ctfp) {
> +			dt_dprintf("error retrieving CTF for print() action\n");
> +			return dt_set_errno(dtp, EDT_CTF);
> +		}
> +	}
> +
>  	dva.dv_data = data;
>  	dva.dv_size = size;
>  	dva.dv_last_depth = 0;
> @@ -2260,5 +2278,5 @@ dt_print_type(dtrace_hdl_t *dtp, FILE *fp,
> uint64_t printaddr,
>  	if (dt_print_close_parens(&dva, 0) < 0)
>  		return -1;
>  -	return 2;
> +	return 3;
>  }
> diff --git a/libdtrace/dt_printf.h b/libdtrace/dt_printf.h
> index 9771c4a8..b72ff55b 100644
> --- a/libdtrace/dt_printf.h
> +++ b/libdtrace/dt_printf.h
> @@ -108,7 +108,7 @@ extern int dt_print_ustack(dtrace_hdl_t *, FILE *,
>      const char *, caddr_t, uint64_t);
>  extern int dt_print_mod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
>  extern int dt_print_umod(dtrace_hdl_t *, FILE *, const char *, caddr_t);
> -extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, ctf_id_t,
> +extern int dt_print_type(dtrace_hdl_t *, FILE *, uint64_t, const char
> *, ctf_id_t,
>  			 caddr_t, size_t);
>   #ifdef	__cplusplus
> -- 
> 2.39.3
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list