[DTrace-devel] [PATCH v3 1/2] Add support for the print() action
Nick Alcock
nick.alcock at oracle.com
Wed Nov 22 12:47:51 UTC 2023
On 22 Nov 2023, Alan Maguire via DTrace-devel verbalised:
> print() [1] prints the address, type and associated values of the data pointed
> to by the pointer passed in. For example:
Oh this is *so pretty*. Lovely code, lovely results :)
I have a pile of CTF niggles about the actual printing function, though,
unsurprisingly. Most are trivial, a few are... less so.
> $ /sbin/dtrace -n 'fbt::ip_rcv_core:entry { print((struct sk_buff *)arg0); }'
> dtrace: description 'fbt::ip_rcv_core:entry ' matched 1 probe
> CPU ID FUNCTION:NAME
> 0 75596 ip_output:entry 0xffff89ab58f88200 = *
> (struct sk_buff) {
> (struct) {
> .sk = (struct sock *)0xffff9b7ad5df2f80,
> .ip_defrag_offset = (int)-706793600,
> },
> (struct) {
> .tstamp = (ktime_t)504907694897760,
> .skb_mstamp_ns = (u64)504907694897760,
> },
> .cb = (char [48]) [
> ],
> (struct) {
> (struct) {
> ._skb_refdst = (long unsigned int)18446633550675199489,
> .destructor = (void (*)(*) ())0xffffffffb66c0b80,
> },
> .tcp_tsorted_anchor = (struct list_head) {
> .next = (struct list_head *)0xffff9b7ad9cc4601,
> .prev = (struct list_head *)0xffffffffb66c0b80,
> },
> ._sk_redir = (long unsigned int)18446633550675199489,
> },
> ._nfct = (long unsigned int)18446633547046898499,
> .len = (unsigned int)8788,
> .data_len = (unsigned int)8736,
> .hdr_len = (__u16)320,
> .cloned = (__u8)1,
> .ip_summed = (__u8)2,
It's even columnized right!
(Presumably (struct) could also be (union) etc. It looks like it would
take you extra work for that not to be true, since this is literally the
ctf_type_name().)
> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
Shame about the 64K limit, but honestly with strsize limits being what
they are I thought you'd be stuck with a much smaller limit.
> [1] https://docs.oracle.com/en/operating-systems/solaris/oracle-solaris/11.4/dtrace-guide/print-action.html#GUID-533E6BD9-8DE8-474E-9770-96F84244911C
> ---
> include/dtrace/actions_defines.h | 1 +
> include/dtrace/options_defines.h | 3 +-
> libdtrace/dt_cg.c | 48 +++++
> libdtrace/dt_consume.c | 26 +++
> libdtrace/dt_errtags.h | 3 +-
> libdtrace/dt_impl.h | 6 +-
> libdtrace/dt_open.c | 2 +
> libdtrace/dt_options.c | 25 +++
> libdtrace/dt_printf.c | 302 ++++++++++++++++++++++++++++++-
> libdtrace/dt_printf.h | 2 +
> 10 files changed, 413 insertions(+), 5 deletions(-)
>
> diff --git a/include/dtrace/actions_defines.h b/include/dtrace/actions_defines.h
> index f161df7c..4ff4fb83 100644
> --- a/include/dtrace/actions_defines.h
> +++ b/include/dtrace/actions_defines.h
> @@ -37,6 +37,7 @@
> #define DTRACEACT_LIBACT 5 /* library-controlled action */
> #define DTRACEACT_TRACEMEM 6 /* tracemem() action */
> #define DTRACEACT_PCAP 7 /* pcap() action */
> +#define DTRACEACT_PRINT 8 /* print() action */
>
> #define DTRACEACT_PROC 0x0100
> #define DTRACEACT_USTACK (DTRACEACT_PROC + 1)
> diff --git a/include/dtrace/options_defines.h b/include/dtrace/options_defines.h
> index 61a23f42..80246be8 100644
> --- a/include/dtrace/options_defines.h
> +++ b/include/dtrace/options_defines.h
> @@ -62,7 +62,8 @@
> #define DTRACEOPT_BPFLOG 32 /* always output BPF verifier log */
> #define DTRACEOPT_SCRATCHSIZE 33 /* max scratch size permitted */
> #define DTRACEOPT_LOCKMEM 34 /* max locked memory */
> -#define DTRACEOPT_MAX 35 /* number of options */
> +#define DTRACEOPT_PRINTSIZE 35 /* max # bytes printed by print() action */
> +#define DTRACEOPT_MAX 36 /* number of options */
>
> #define DTRACEOPT_UNSET (dtrace_optval_t)-2 /* unset option */
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 581ada4e..9c5bb2cf 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2560,6 +2560,53 @@ dt_cg_act_ustack(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> BPF_NOP());
> }
>
> +static void
> +dt_cg_act_print(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> +{
> + uint_t addr_off, data_off;
> + dt_node_t *addr = dnp->dn_args;
> + dt_irlist_t *dlp = &pcb->pcb_ir;
> + dt_regset_t *drp = pcb->pcb_regs;
> + dtrace_hdl_t *dtp = pcb->pcb_hdl;
> + ctf_file_t *fp = addr->dn_ctfp;
> + ctf_id_t type = addr->dn_type;
> + char n[DT_TYPE_NAMELEN];
> + size_t size;
> +
> + type = ctf_type_reference(fp, type);
> + if (type == CTF_ERR)
> + longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
> + size = ctf_type_size(fp, type);
> + if (size == 0)
> + dnerror(addr, D_PRINT_SIZE,
> + "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 */
> + addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> + sizeof(uint64_t), 8, NULL, type);
> + data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> + size, 8, NULL, size);
> +
> + dt_cg_node(addr, &pcb->pcb_ir, drp);
> + dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
> +
> + /* store address for later print()ing */
> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, addr_off, addr->dn_reg));
> +
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + emit(dlp, BPF_MOV_REG(BPF_REG_3, addr->dn_reg));
> + dt_regset_free(drp, addr->dn_reg);
> + emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, data_off));
> + emit(dlp, BPF_MOV_IMM(BPF_REG_2, size));
> + dt_regset_xalloc(drp, BPF_REG_0);
> + emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> + dt_regset_free_args(drp);
> + dt_regset_free(drp, BPF_REG_0);
> +}
So this (obviously, sensibly) does no pointer chasing, so this is for
printing a single contiguous block of bytes which happens to be a
complex aggregate structure? Sounds sensible.
> +static int
> +dt_print_visit(const char *name, ctf_id_t type, unsigned long offset,
> + int depth, void *arg)
> +{
> + struct dt_visit_arg *dva = arg;
Hm, I should probably implement a _next() variant of ctf_type_visit().
This is a bit clumsy, but it's the best you can do with the current API.
> + const void *data = (char *)dva->dv_data + offset/NBBY;
> + size_t typesize, typelen, asize;
type_size is an ssize_t. All those signed/unsigned warnings are not for
fun.
> + char typename[DT_NAME_LEN] = { 0 };
This limitation is unnecessary.
> + char datastr[DT_PRINT_LEN] = { 0 };
> + const char *membername = NULL;
> + int64_t intdata = 0;
> + const char *ename;
> + ctf_encoding_t e;
> + ctf_arinfo_t a;
> + ctf_id_t rtype;
> + int issigned;
> + int rkind;
> + int i;
> +
> + /* run out of data to display, or hit depth limit? */
> + if ((offset/NBBY) > dva->dv_size || depth >= DT_PRINT_DEPTH) {
> + dt_printf(dva->dv_dtp, dva->dv_fp, "%*s...\n",
> + dva->dv_startindent + depth, "");
> + return -1;
> + }
> + rtype = ctf_type_resolve(dva->dv_ctfp, type);
> + if (rtype == CTF_ERR)
> + rtype = type;
... no matter what the error is? That's... excitingly risky. I'm not
sure what sort of error you're even expecting: just about the only
errors you can get are ECTF_NONREPRESENTABLE (in which case this *is*
probably the right thing to do, only you should explicitly handle
printing nonrepresentable types in this case), or ECTF_CORRUPT, in
which case we should *definitely* emit some sort of indication that
something is very wrong.
Also, do we really want to resolve away typedefs entirely? Their textual
name surely provides useful information, but we're not printing it at
all.
> + rkind = ctf_type_kind(dva->dv_ctfp, rtype);
This can in theory fail (but in practice it would require corrupt CTF to
do so, since a disconnected slice is the only plausible cause, so you
can probably ignore it).
> + /* zeroed data? if so, do not display it. */
> + if (depth > 0) {
> + typesize = ctf_type_size(dva->dv_ctfp, rtype);
This can *definitely* fail, though all the failures are unlikely. Error
check!
> + if (typesize < sizeof(zeros) && memcmp(data, zeros, typesize) == 0)
> + return 0;
If ctf_type_size had failed, this would lead to fun! (I am surprised,
astonished and appalled that C has no more efficient way than this to
check for zeroness/nonzeroness of a memory region, but it doesn't.)
> + ctf_type_name(dva->dv_ctfp, type, typename, sizeof(typename));
It is almost always a mistake to use ctf_type_name for anything, because
of the horrible static buffers. Use ctf_type_aname(), which returns a
dynamically allocated string (which always fits) which you can free.
> + switch (rkind) {
> + case CTF_K_STRUCT:
> + case CTF_K_UNION:
> + snprintf(datastr, sizeof(datastr), "(%s) {", typename);
I twitch violently whenever I see s(n)printf(). This sort of thing is
what asprintf() is for.
> + break;
> + case CTF_K_ARRAY:
> + ctf_array_info(dva->dv_ctfp, rtype, &a);
> + asize = ctf_type_size(dva->dv_ctfp, a.ctr_contents);
> + snprintf(datastr, sizeof(datastr), "(%s) [%s", typename,
> + a.ctr_nelems == 0 ? "]," : "");
ctf_array_info and ctf_type_size can both fail. asprintf().
> + /* array member processing will be handled below... */
> + break;
> + case CTF_K_INTEGER:
> + /* type resolution sometimes fails for __u8 bitfields for some reason */
What's the failure? It really shouldn't be failing! The only unusualness
I can think of is that bitfields, including __u8 bitfields, will
probably be encoded as slices of the base type, but that should be
transparent to you: they should appear as suitably-encoded instances of
the underlying type. If the failures turn out to be a bug I'd like to
fix it.
> + case CTF_K_TYPEDEF:
> + ctf_type_encoding(dva->dv_ctfp, rtype, &e);
OK so this is based on rkind, which is the kind of a resolved
type. We've already chased through typedefs! I don't see how you can get
a typedef back out. Even a typedef of a slice of an int is going to
chase to the slice, then return ctf_type_kind() of the kind it is a
slice of (and ctf_type_encoding() will apply the slice to the base type
it is a ref of, then return that).
How can you ever get a CTF_K_TYPEDEF here?
> + issigned = (e.cte_format & CTF_INT_SIGNED) != 0;
> + switch (e.cte_bits) {
> + case 8:
> + if (issigned)
> + intdata = (int64_t)*(char *)data;
> + else
> + intdata = (int64_t)*(unsigned char *)data;
> + break;
> + case 16:
> + if (issigned)
> + intdata = (int64_t)*(int16_t *)data;
> + else
> + intdata = (int64_t)*(uint16_t *)data;
> + break;
> + case 32:
> + if (issigned)
> + intdata = (int64_t)*(int32_t *)data;
> + else
> + intdata = (int64_t)*(uint32_t *)data;
> + break;
> + case 64:
> + if (issigned) {
> + intdata = (int64_t)*(int64_t *)data;
> + } else {
> + /* cannot use intdata (int64_t) to hold uin64_t;
> + * stringify the cast and data here instead.
> + */
> + snprintf(datastr, sizeof(datastr), "(%s)%lu,",
> + typename, *(uint64_t *)data);
asprintf
> + goto doprint;
> + }
> + break;
> + default:
> + if (e.cte_bits <= 64) {
> + unsigned int lshift, rshift;
> + uint64_t bitdata = *(uint64_t *)((char *)dva->dv_data +
> + (offset + e.cte_offset)/NBBY);
> + /* shift bitfield data left then right to
> + * eliminate unneeded bits.
> + */
> + lshift = 63 - ((offset + e.cte_offset) % 64);
> + rshift = 64 - e.cte_bits;
> + bitdata = bitdata << lshift;
> + bitdata = bitdata >> rshift;
> + intdata = (int64_t)bitdata;
What a cunning way to get around the gross definition of bitfields in C.
It's almost all implementation-dependent, but this compiler is the same
implementation :)
> + /* zero-value bitfield; do not display */
> + if (intdata == 0)
> + return 0;
> + } else {
> + /* > 64 bit integer vals not supported. */
> + snprintf(datastr, sizeof(datastr),
> + "(%s)?(%d bit value),",
> + typename, e.cte_bits);
asprintf
> + goto doprint;
> + }
> + break;
> + }
> + if ((e.cte_format & CTF_INT_CHAR) != 0 &&
> + strcmp(typename, "char") == 0) {
> + snprintf(datastr, sizeof(datastr), "(%s)'%c',",
> + typename, (char)intdata);
> + } else {
> + snprintf(datastr, sizeof(datastr), "(%s)%ld,",
> + typename, intdata);
asprintf
> + }
> + break;
> + case CTF_K_ENUM:
> + ename = ctf_enum_name(dva->dv_ctfp, rtype, *(int *)data);
(this is, of course, routinely null, with the typedef name providing the
information we want, but we resolved that away.)
> + if (ename != NULL) {
> + /* exception to printing 0-valued data here;
> + * enum values are often meaningful with 0
> + * value so display the enum value string.
> + */
> + snprintf(datastr, sizeof(datastr), "(%s)%s,",
> + typename, ename);
> + } else {
> + snprintf(datastr, sizeof(datastr), "(%s)%d,",
> + typename, *(int *)data);
asprintf...
> + }
> + break;
> + case CTF_K_POINTER:
> + snprintf(datastr, sizeof(datastr), "(%s)0x%lx,",
> + typename, (uintptr_t)*(void **)data);
asprintf...
> + if (dt_printf(dva->dv_dtp, dva->dv_fp, "%*s%s%s%s%s\n",
> + dva->dv_startindent + depth, "",
> + membername != NULL ? "." : "",
> + membername != NULL ? membername : "",
> + membername != NULL ? " = " : "",
> + datastr) < 0)
> + return -1;
> +
> + /* type visit does not recurse into array elements... */
Mostly because it's so simple to visit them yourself, I think, and
because the visit function very much assumes it's visiting something
with elements distinguished via field names, which arrays don't have.
> + if (rkind == CTF_K_ARRAY) {
> + struct dt_visit_arg dva2;
> + int ischar = 0;
> +
> + dva2.dv_dtp = dva->dv_dtp;
> + dva2.dv_ctfp = dva->dv_ctfp;
> + dva2.dv_fp = dva->dv_fp;
> + dva2.dv_data = dva->dv_data;
> + dva2.dv_startindent = dva->dv_startindent;
> + ctf_type_encoding(dva->dv_ctfp, a.ctr_contents, &e);
(error checking)
--
NULL && (void)
More information about the DTrace-devel
mailing list