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

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

On Wed, Apr 24, 2024 at 01:19:59PM -0400, Kris Van Hees via DTrace-devel wrote:
> 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@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@oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel