[DTrace-devel] [PATCH v3 1/2] print() action: identify ctf object used for print
Alan Maguire
alan.maguire at oracle.com
Tue Jun 11 17:33:55 UTC 2024
On 26/04/2024 14:33, Alan Maguire wrote:
> when generating code for print() action we need to identify source
> of CTF; is it shared CTF, cdefs or ddefs or another module?
> Introduce a module id scheme where the module id can be stored
> at cg time for later retrieval at print time; this allows us
> to later look up the module and its associated CTF without relying
> on possibly freed pointers.
>
> Under the hood, the id scheme uses a monotonic counter coupled
> with the hash of the modules name; the latter allows us to quickly
> look up the right bucket via dt_htab_find_hval().
>
> 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>
ping on this one; I know things are busy with USDT right now but would
be good to land a fix for this issue if someone has the cycles to take a
look. If the BTF identification scheme doesn't work we can try something
else of course but above id scheme seems to be a reasonable compromise
between robustness+speed. Thanks!
Alan
> ---
> libdtrace/dt_cg.c | 7 ++++++-
> libdtrace/dt_consume.c | 7 +++++--
> libdtrace/dt_htab.c | 17 +++++++++++++++++
> libdtrace/dt_htab.h | 2 ++
> libdtrace/dt_impl.h | 1 +
> libdtrace/dt_module.c | 30 ++++++++++++++++++++++++++++++
> libdtrace/dt_module.h | 1 +
> libdtrace/dt_printf.c | 12 +++++++++---
> libdtrace/dt_printf.h | 2 +-
> 9 files changed, 72 insertions(+), 7 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 27246a40..30cccdaf 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>
> @@ -2832,7 +2833,9 @@ dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> ctf_id_t type = addr->dn_type;
> char n[DT_TYPE_NAMELEN];
> size_t size;
> + dt_module_t *dmp;
>
> + dmp = dt_module_lookup_by_ctf(dtp, fp);
> type = ctf_type_reference(fp, type);
> if (type == CTF_ERR)
> longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
> @@ -2842,9 +2845,11 @@ 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, module identification, data/size */
> addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> sizeof(uint64_t), 8, NULL, type);
> + dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> + sizeof(uint64_t), 8, NULL, (uint64_t)dmp->dm_id);
> 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..432b4e12 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;
> + unsigned long dm_id = (unsigned long)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, dm_id,
> + (ctf_id_t)rec->dtrd_arg,
> (caddr_t)buf + data_rec->dtrd_offset, size);
> }
>
> diff --git a/libdtrace/dt_htab.c b/libdtrace/dt_htab.c
> index 478c728a..3c167cb5 100644
> --- a/libdtrace/dt_htab.c
> +++ b/libdtrace/dt_htab.c
> @@ -229,6 +229,23 @@ void *dt_htab_find(const dt_htab_t *htab, const void *entry,
> return ent;
> }
>
> +/* Find an entry in the hashtable, using the provided callback function as a
> + * secondary comparison function to differentiate between entries in a bucket.
> + * A pre-computed hash value is passed in.
> + */
> +void *dt_htab_find_hval(const dt_htab_t *htab, uint32_t hval,
> + dt_htab_ecmp_fn *cmpf, void *arg)
> +{
> + int idx = hval & htab->mask;
> + dt_hbucket_t *bucket;
> +
> + for (bucket = htab->tab[idx]; bucket; bucket = bucket->next) {
> + if (cmpf(bucket->head, arg))
> + return bucket->head;
> + }
> + return NULL;
> +}
> +
> /*
> * Remove an entry from the hashtable. If we are deleting the last entry in a
> * bucket, get rid of the bucket.
> diff --git a/libdtrace/dt_htab.h b/libdtrace/dt_htab.h
> index d39ae65e..1ce5a80d 100644
> --- a/libdtrace/dt_htab.h
> +++ b/libdtrace/dt_htab.h
> @@ -100,6 +100,8 @@ extern void *dt_htab_lookup(const dt_htab_t *htab, const void *entry);
> typedef int dt_htab_ecmp_fn(const void *entry, void *arg);
> extern void *dt_htab_find(const dt_htab_t *htab, const void *entry,
> dt_htab_ecmp_fn *cmpf, void *arg);
> +extern void *dt_htab_find_hval(const dt_htab_t *htab, uint32_t hval,
> + dt_htab_ecmp_fn *cmpf, void *arg);
> extern size_t dt_htab_entries(const dt_htab_t *htab);
> extern int dt_htab_delete(dt_htab_t *htab, void *entry);
> extern void *dt_htab_next(const dt_htab_t *htab, dt_htab_next_t **it);
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 2dfb64d6..2bd1dafe 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -136,6 +136,7 @@ typedef struct dt_module {
> dt_list_t dm_list; /* list forward/back pointers */
> char dm_name[DTRACE_MODNAMELEN]; /* string name of module */
> char dm_file[PATH_MAX]; /* file path of module */
> + unsigned long dm_id; /* id of module */
> uint_t dm_flags; /* module flags (see below) */
> struct dt_hentry dm_he; /* htab links */
> dtrace_hdl_t *dm_dtp; /* backpointer to the dtrace instance */
> diff --git a/libdtrace/dt_module.c b/libdtrace/dt_module.c
> index 1e730426..7697f73b 100644
> --- a/libdtrace/dt_module.c
> +++ b/libdtrace/dt_module.c
> @@ -35,6 +35,8 @@
> #define KSYM_NAME_MAX 128 /* from kernel/scripts/kallsyms.c */
> #define GZCHUNKSIZE (1024*512) /* gzip uncompression chunk size */
>
> +unsigned int module_idx = 0;
> +
> static void
> dt_module_unload(dtrace_hdl_t *dtp, dt_module_t *dmp);
>
> @@ -50,6 +52,12 @@ dt_module_hval(const dt_module_t *mod)
> return str2hval(mod->dm_name, 0);
> }
>
> +/* id is combination of hash and index */
> +static void dt_module_init_id(dt_module_t *dmp)
> +{
> + dmp->dm_id = ((unsigned long)dt_module_hval(dmp)) << 32 | module_idx++;
> +}
> +
> static int
> dt_module_cmp(const dt_module_t *p, const dt_module_t *q)
> {
> @@ -150,6 +158,7 @@ dt_module_create(dtrace_hdl_t *dtp, const char *name)
>
> memset(dmp, 0, sizeof(dt_module_t));
> strlcpy(dmp->dm_name, name, sizeof(dmp->dm_name));
> + dt_module_init_id(dmp);
> if (dt_htab_insert(dtp->dt_mods, dmp) < 0) {
> free(dmp);
> return NULL;
> @@ -193,6 +202,27 @@ dt_module_lookup_by_ctf(dtrace_hdl_t *dtp, ctf_file_t *ctfp)
> return ctfp ? ctf_getspecific(ctfp) : NULL;
> }
>
> +static int
> +dt_module_idmatch(const dt_module_t *m1, dt_module_t *m2)
> +{
> + return m1->dm_id == m2->dm_id;
> +}
> +
> +/* High-order 32 bits of id are hash value, low order the module index.
> + * This allows us to limit cost of search based on bucket size.
> + */
> +dt_module_t *
> +dt_module_lookup_by_id(dtrace_hdl_t *dtp, unsigned long id)
> +{
> + uint32_t hval = id >> 32;
> + dt_module_t tmpl;
> +
> + tmpl.dm_id = id;
> +
> + return dt_htab_find_hval(dtp->dt_mods, hval, (dt_htab_ecmp_fn *)dt_module_idmatch,
> + &tmpl);
> +}
> +
> static int
> dt_module_init_elf(dtrace_hdl_t *dtp, dt_module_t *dmp)
> {
> diff --git a/libdtrace/dt_module.h b/libdtrace/dt_module.h
> index 56df17a6..68dadfc6 100644
> --- a/libdtrace/dt_module.h
> +++ b/libdtrace/dt_module.h
> @@ -18,6 +18,7 @@ extern dt_module_t *dt_module_create(dtrace_hdl_t *, const char *);
>
> extern dt_module_t *dt_module_lookup_by_name(dtrace_hdl_t *, const char *);
> extern dt_module_t *dt_module_lookup_by_ctf(dtrace_hdl_t *, ctf_file_t *);
> +extern dt_module_t *dt_module_lookup_by_id(dtrace_hdl_t *, unsigned long);
>
> extern ctf_file_t *dt_module_getctf(dtrace_hdl_t *, dt_module_t *);
> extern dt_ident_t *dt_module_extern(dtrace_hdl_t *, dt_module_t *,
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index 50842216..108adc23 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,18 @@ err:
>
> int
> dt_print_type(dtrace_hdl_t *dtp, FILE *fp, uint64_t printaddr,
> - ctf_id_t type, caddr_t data, size_t size)
> + unsigned long dm_id, ctf_id_t type, caddr_t data, size_t size)
> {
> struct dt_visit_arg dva;
> + dt_module_t *dmp = dt_module_lookup_by_id(dtp, dm_id);
>
> + if (!dmp) {
> + dt_dprintf("error retrieving CTF for print() action\n");
> + return dt_set_errno(dtp, EDT_CTF);
> + }
> + dva.dv_ctfp = dt_module_getctf(dtp, dmp);
> dva.dv_dtp = dtp;
> dva.dv_fp = fp;
> - dva.dv_ctfp = dtp->dt_ddefs->dm_ctfp;
> dva.dv_data = data;
> dva.dv_size = size;
> dva.dv_last_depth = 0;
> @@ -2260,5 +2266,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..8d55d82b 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, unsigned long, ctf_id_t,
> caddr_t, size_t);
>
> #ifdef __cplusplus
More information about the DTrace-devel
mailing list