[DTrace-devel] [PATCH 1/2] Add support for the print() action
Kris Van Hees
kris.van.hees at oracle.com
Tue Nov 21 16:23:29 UTC 2023
On Tue, Nov 21, 2023 at 03:10:10PM +0000, Alan Maguire wrote:
> On 20/11/2023 17:02, Kris Van Hees wrote:
> > On Fri, Nov 17, 2023 at 11:48:03AM +0000, Alan Maguire via DTrace-devel wrote:
> >> print() [1] prints the address, type and associated values of the data pointed
> >> to by the pointer passed in. For example:
> >>
> >> $ /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,
> >>
> >> ...etc.
> >>
> >> Zero-valued fields are skipped to improve compactness of the representation.
> >> Since some data structures are large, the "printsize" option can be used
> >> to restrict the amount of data displayed; in such cases a "..." connotes
> >> the fact that additional data was present but not traced.
> >>
> >> User-defined structures can also be printed:
> >>
> >> CPU ID FUNCTION:NAME
> >> 0 75596 ip_output:entry 0xffff89ab58f88200 = *
> >> (struct foo) {
> >> .a = (int)2,
> >> }
> >>
> >> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
> >>
> >> [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 | 58 ++++++
> >> libdtrace/dt_consume.c | 316 +++++++++++++++++++++++++++++++
> >> libdtrace/dt_errtags.h | 4 +-
> >> libdtrace/dt_impl.h | 6 +-
> >> libdtrace/dt_open.c | 2 +
> >> libdtrace/dt_options.c | 25 +++
> >> 8 files changed, 411 insertions(+), 4 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..02271684 100644
> >> --- a/libdtrace/dt_cg.c
> >> +++ b/libdtrace/dt_cg.c
> >> @@ -2560,6 +2560,63 @@ 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 basetype, type = addr->dn_type;
> >> + char n[DT_TYPE_NAMELEN];
> >> + size_t size;
> >> +
> >> + if (dt_node_is_pointer(addr) == 0) {
> >> + dnerror(addr, D_PRINT_ADDR,
> >> + "print( ) argument #1 is incompatible with "
> >> + "prototype:\n\tprototype: pointer\n"
> >> + "\t argument: %s\n",
> >> + dt_node_type_name(addr, n, sizeof(n)));
> >> + }
> >
> > You should check but you mauy not need this - I think that the prototype
> > argument checking done based on the prototype given in _dtrace_globals
> > (in dt_open.c) might be sufficient since it specifies that the argument
> > should be void *.
> >
>
> yep, that works, thanks!
>
> >> + basetype = ctf_type_reference(fp, type);
> >
> > You might as well make this:
> > type = ctf_type_reference(fp, type);
> > since the original value of type is not used any further.
> >
>
> sure.
>
> >> + if (basetype == CTF_ERR)
> >> + longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
> >> + size = ctf_type_size(fp, basetype);
> >> + 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, basetype, n, sizeof(n)));
> >> + if (size > dtp->dt_options[DTRACEOPT_PRINTSIZE])
> >> + size = dtp->dt_options[DTRACEOPT_PRINTSIZE];
> >
> > I am leaning towards doing this on the consumer side. Doing it here of course
> > allows for limiting the data stored in the output buffers, but the advantage
> > of dping it on the consumer isde is that the producer can make use of the
> > support code we already have for putting values into the output buffer *and*
> > that would allow for orintsize to become a dynamic option (i.e. its value can
> > change at runtime using setopt()).
> >
>
> Not sure I totally understand, but the aim here is to limit the amout of
> output we need to store as well as display. There are some huge data
> structures, and being able to minimize what we copy (as well as what we
> dump) is needed I think. Would the method you suggest limit both the
> size of the bpf_probe_read() and the display, or just the latter?
Just the latter.
>From what I know, the other DTrace implementations do not really limit the
amount of data being copied either. Then again, they do not seem to limit
the output either, so maybe that is a moot point :)
Overall though, I am not too concerned with dumping larger amounts of data
since it is usually short-lived.
> >> + /* reserve space for addr/type, data/size */
> >> + addr_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> >> + sizeof(uint64_t), 8, NULL, basetype);
> >
> > This one we need.
> >
> >> + data_off = dt_rec_add(dtp, dt_cg_fill_gap, DTRACEACT_PRINT,
> >> + size, 8, NULL, size);
> >
> > This one you could drop.
> >
>
> I _think_ this is presuming we can handle size limiting on the consumer
> side, right?
Correct.
> >> +
> >> + dt_cg_node(addr, &pcb->pcb_ir, drp);
> >> + dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
> >> +
> >> + /* store addrress for later print()ing */
> >> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, addr_off, addr->dn_reg));
> >
> > Move this to the end of the function (as I show below).
> >
> >> +
> >> + /* store addr, size, type and bpf_probe_read(data_off, size, addr); */
> >> + if (dt_regset_xalloc_args(drp) == -1)
> >> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> >> + emit(dlp, BPF_MOV_REG(BPF_REG_3, 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);
> >
> > I am pretty sure the following should work:
> >
> > You can force dt_cg_store_val() to dereference the node it is given (since you
> > know it is a pointer), and thus use:
> >
> > dt_cg_store_val(pcb, addr, DTRACEACT_PRINT, NULL, DT_NF_REF);
> > emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, addr_off, addr->dn_reg));
> >
> > This is valid because dt_cg_store_val() will have evaluated addr and thus the
> > address value will be in addr->dn_reg.
>
> I'm probably misunderstanding, but in removing
>
> dt_cg_node(addr, &pcb->pcb_ir, drp);
> dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
>
> ...and adding
>
> dt_cg_store_val(pcb, addr, DTRACEACT_PRINT, NULL, DT_NF_REF);
> emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, addr_off, addr->dn_reg));
>
>
> ...to the end as above I see:
>
> $ dtrace -n 'fbt::ip_rcv_core:entry { print((struct sk_buff *)arg0); }'
> dtrace: libdtrace/dt_regset.c:106: dt_regset_free: Assertion `reg >= 0
> && reg < drp->dr_size' failed.
> Aborted
Hm, yes, I overlooked that dt_cg_store_val() does not retain the register of
the value. That is unfortunate.
So, we have 3 options I guess:
- add a flag that can be passed in the 'arg' argument to dt_cg_store_val()
to indidcate that we want the value (register) to be retained. Then the
above would work.
- copy the entire structure data explicitly in dt_cg_act_print()
- go back to a limiting of data at producer end (what you had) along with
dt_cg_act_print() doing the copy explictly
> >
> >> +}
> >> +
> >> typedef void dt_cg_action_f(dt_pcb_t *, dt_node_t *, dtrace_actkind_t);
> >>
> >> typedef struct dt_cg_actdesc {
> >> @@ -2615,6 +2672,7 @@ static const dt_cg_actdesc_t _dt_cg_actions[DT_ACT_MAX] = {
> >> DTRACEACT_UADDR },
> >> [DT_ACT_IDX(DT_ACT_SETOPT)] = { &dt_cg_act_setopt, },
> >> [DT_ACT_IDX(DT_ACT_PCAP)] = { &dt_cg_act_pcap, },
> >> + [DT_ACT_IDX(DT_ACT_PRINT)] = { &dt_cg_act_print, },
> >> };
> >>
> >> dt_irnode_t *
> >> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> >> index 7e2b1005..751baab9 100644
> >> --- a/libdtrace/dt_consume.c
> >> +++ b/libdtrace/dt_consume.c
> >> @@ -14,6 +14,7 @@
> >> #include <ctype.h>
> >> #include <alloca.h>
> >> #include <dt_impl.h>
> >> +#include <dt_module.h>
> >> #include <dt_pcap.h>
> >> #include <dt_peb.h>
> >> #include <dt_state.h>
> >> @@ -1971,6 +1972,315 @@ dt_print_trace(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> >> return dt_print_rawbytes(dtp, fp, data, rec->dtrd_size);
> >> }
> >>
> >> +#define DT_PRINT_DEPTH 10
> >> +
> >> +struct dt_visit_arg {
> >> + dtrace_hdl_t *dv_dtp;
> >> + FILE *dv_fp;
> >> + ctf_file_t *dv_ctfp;
> >> + void *dv_data;
> >> + uint64_t dv_size;
> >> + int dv_kinds[DT_PRINT_DEPTH];
> >> + int dv_last_depth;
> >> + int dv_startindent;
> >> +};
> >> +
> >> +#define DT_NAME_LEN 32
> >> +#define DT_PRINT_LEN 80
> >> +
> >> +#define DT_PRINT_STARTINDENT 44
> >> +
> >> +/* used to compare nested objects like structs/unions/arrays to see if they are 0. */
> >> +static char zeros[65536] = {};
> >> +
> >> +static int
> >> +dt_print_close_parens(struct dt_visit_arg *dva, int depth)
> >> +{
> >> + int i;
> >> +
> >> + switch (dva->dv_kinds[depth]) {
> >> + case CTF_K_STRUCT:
> >> + case CTF_K_UNION:
> >> + /* if struct/union contents were 0, we still opened the
> >> + * struct "{" at the top level, even if we did not
> >> + * display any members, so need to close it. An empty
> >> + * struct looks like this:
> >> + * (struct foo) {
> >> + * }
> >> + *
> >> + * ...at the top level, but at any other depth of nesting
> >> + * it is not displayed at all.
> >> + */
> >> + if (depth == 0 && dva->dv_last_depth == 0)
> >> + dva->dv_last_depth = 1;
> >> + /* add a "}" for each level needed between current depth
> >> + * and last depth we displayed data at.
> >> + */
> >> + for (i = dva->dv_last_depth - depth; i > 0; i--) {
> >> + if (dt_printf(dva->dv_dtp, dva->dv_fp, "%*s}%s\n",
> >> + dva->dv_startindent + depth + i - 1, "",
> >> + (depth > 0 || i > 1) ? "," : "") < 0)
> >> + return -1;
> >> + dva->dv_last_depth = depth;
> >> + }
> >> + break;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +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;
> >> + const void *data = dva->dv_data + offset/NBBY;
> >> + size_t typesize, typelen, asize;
> >> + char typename[DT_NAME_LEN] = {};
> >> + char datastr[DT_PRINT_LEN] = {};
> >> + 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;
> >> + rkind = ctf_type_kind(dva->dv_ctfp, rtype);
> >> +
> >> + /* we may have just shown the last member in a struct/union
> >> + * at depth + 1 (or deeper than that). If so close the "{".
> >> + * The final parenthesis close (at depth 0) will be done
> >> + * by the caller.
> >> + */
> >> + if (dva->dv_last_depth > depth)
> >> + dt_print_close_parens(dva, depth);
> >> +
> >> + /* zeroed data? if so, do not display it. */
> >> + if (depth > 0) {
> >> + typesize = ctf_type_size(dva->dv_ctfp, rtype);
> >> + if (typesize < sizeof(zeros) && memcmp(data, zeros, typesize) == 0)
> >> + return 0;
> >> + }
> >> +
> >> + dva->dv_kinds[depth] = rkind;
> >> + dva->dv_last_depth = depth;
> >> +
> >> + ctf_type_name(dva->dv_ctfp, type, typename, sizeof(typename));
> >> + /* anon struct/union/enum will be "struct "; remove the space. */
> >> + typelen = strlen(typename);
> >> + if (typename[typelen - 1] == ' ')
> >> + typename[typelen - 1] = '\0';
> >> +
> >> + /* print .<member_name> where available */
> >> + if (depth > 0) {
> >> + if (name && strlen(name) > 0)
> >> + membername = name;
> >> + }
> >> +
> >> + switch (rkind) {
> >> + case CTF_K_STRUCT:
> >> + case CTF_K_UNION:
> >> + snprintf(datastr, sizeof(datastr), "(%s) {", typename);
> >> + 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 ? "]," : "");
> >> + /* array member processing will be handled below... */
> >> + break;
> >> + case CTF_K_INTEGER:
> >> + /* type resolution sometimes fails for __u8 bitfields for some reason */
> >> + case CTF_K_TYPEDEF:
> >> + ctf_type_encoding(dva->dv_ctfp, rtype, &e);
> >> + 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);
> >> + goto doprint;
> >> + }
> >> + break;
> >> + default:
> >> + if (e.cte_bits <= 64) {
> >> + unsigned int lshift, rshift;
> >> + uint64_t bitdata = *(uint64_t *)(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;
> >> + /* 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);
> >> + 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);
> >> + }
> >> + break;
> >> + case CTF_K_ENUM:
> >> + ename = ctf_enum_name(dva->dv_ctfp, rtype, *(int *)data);
> >> + 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);
> >> + }
> >> + break;
> >> + case CTF_K_POINTER:
> >> + snprintf(datastr, sizeof(datastr), "(%s)0x%lx,",
> >> + typename, (uintptr_t)*(void **)data);
> >> + break;
> >> + default:
> >> + return 0;
> >> + }
> >> +doprint:
> >> + /* format is
> >> + * [.<membername> = ](<type>)value,
> >> + * For example
> >> + * .foo = (bar)1,
> >> + */
> >> + 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... */
> >> + 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);
> >> + /* is array all 0s? Then skip... */
> >> + if (memcmp(data, zeros, asize * a.ctr_nelems) == 0)
> >> + return 0;
> >> + /* we check for early '\0' in char arrays... */
> >> + if ((e.cte_format & CTF_INT_CHAR) != 0 && e.cte_bits == 8)
> >> + ischar = 1;
> >> +
> >> + for (i = 0; i < a.ctr_nelems; i++) {
> >> + int aoffset = (asize * 8) * i;
> >> +
> >> + if (ischar && *(char *)(data + i) == '\0')
> >> + break;
> >> + if (dt_print_visit(NULL, a.ctr_contents,
> >> + offset + aoffset,
> >> + depth + 1, &dva2) < 0)
> >> + return -1;
> >> + }
> >> + if (a.ctr_nelems > 0) {
> >> + if (dt_printf(dva->dv_dtp, dva->dv_fp, "%*s],\n",
> >> + dva->dv_startindent + depth, "") < 0)
> >> + return -1;
> >> + }
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +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;
> >> + ctf_id_t type = (ctf_id_t)rec->dtrd_arg;
> >> + struct dt_visit_arg dva;
> >> + uint64_t printaddr;
> >> + ctf_file_t *ctfp;
> >> +
> >> + if (dt_read_scalar(buf, rec, &printaddr) < 0)
> >> + return dt_set_errno(dtp, EDT_PRINT);
> >> +
> >> + ctfp = dtp->dt_ddefs->dm_ctfp;
> >> +
> >> + dva.dv_dtp = dtp;
> >> + dva.dv_fp = fp;
> >> + dva.dv_ctfp = ctfp;
> >> + dva.dv_data = (caddr_t)buf + data_rec->dtrd_offset;
> >> + dva.dv_size = (size_t)data_rec->dtrd_arg;
> >> + dva.dv_last_depth = 0;
> >> + dva.dv_startindent = DT_PRINT_STARTINDENT;
> >> +
> >> + /* print address as * to type; type display will start on the
> >> + * next line.
> >> + */
> >> + if (dt_printf(dtp, fp, "%p = *\n", (void *)printaddr) < 0)
> >> + return -1;
> >> +
> >> + ctf_type_visit(ctfp, type, dt_print_visit, &dva);
> >> +
> >> + /* may need to close top-level parenthesis for struct/union. */
> >> + if (dt_print_close_parens(&dva, 0) < 0)
> >> + return -1;
> >> +
> >> + return 2;
> >> +}
> >
> > I would move all the above into dt_printf.c, especially because that might
> > make it possible to someday add a format conversion that does the same as
> > print(), i.e. (just making this up) a %P that prints the argument as an
> > type annotated data item.
> >
> > Either way, it seems enough like a more complex print function that dt_printf.c
> > seems appropriate.
> >
>
> Sure, will do. A type-specific format specifier is a great idea; I even
> tried to add one to the kernel's printk() at one stage (with no success).
>
> >> +
> >> /*
> >> * The lifecycle of speculation buffers is as follows:
> >> *
> >> @@ -2530,6 +2840,12 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
> >> case DTRACEACT_FREOPEN:
> >> func = dtrace_freopen;
> >> break;
> >> + case DTRACEACT_PRINT:
> >> + n = dt_print_print(dtp, fp, rec, data);
> >> + if (n < 0)
> >> + return -1;
> >> + i += n - 1;
> >> + continue;
> >> default:
> >> break;
> >> }
> >> diff --git a/libdtrace/dt_errtags.h b/libdtrace/dt_errtags.h
> >> index 86c9bd4e..93f93ede 100644
> >> --- a/libdtrace/dt_errtags.h
> >> +++ b/libdtrace/dt_errtags.h
> >> @@ -230,7 +230,9 @@ typedef enum {
> >> D_LLQUANT_MATCHSTEPS, /* llquantize() mismatch on steps */
> >> D_PCAP_ADDR, /* pcap() address bad type */
> >> D_PCAP_PROTO, /* pcap() prototype mismatch */
> >> - D_PCAP_SIZE /* pcap() bad size */
> >> + D_PCAP_SIZE, /* pcap() bad size */
> >> + D_PRINT_ADDR, /* print() address bad type */
> >> + D_PRINT_SIZE, /* print() address target bad size */
> >> } dt_errtag_t;
> >>
> >> extern const char *dt_errtag(dt_errtag_t);
> >> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> >> index 98a74910..5c429946 100644
> >> --- a/libdtrace/dt_impl.h
> >> +++ b/libdtrace/dt_impl.h
> >> @@ -585,8 +585,9 @@ struct dtrace_hdl {
> >> #define DT_ACT_UADDR DT_ACT(27) /* uaddr() action */
> >> #define DT_ACT_SETOPT DT_ACT(28) /* setopt() action */
> >> #define DT_ACT_PCAP DT_ACT(29) /* pcap() action */
> >> +#define DT_ACT_PRINT DT_ACT(30) /* print() action */
> >>
> >> -#define DT_ACT_MAX 30
> >> +#define DT_ACT_MAX 31
> >>
> >> /*
> >> * Aggregation functions.
> >> @@ -703,7 +704,8 @@ enum {
> >> EDT_OBJIO, /* cannot read object file or module name mapping */
> >> EDT_READMAXSTACK, /* cannot read kernel param perf_event_max_stack */
> >> EDT_TRACEMEM, /* missing or corrupt tracemem() record */
> >> - EDT_PCAP /* missing or corrupt pcap() record */
> >> + EDT_PCAP, /* missing or corrupt pcap() record */
> >> + EDT_PRINT, /* missing or corrupt print() record */
> >> };
> >>
> >> /*
> >> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> >> index b521d887..1ec165fc 100644
> >> --- a/libdtrace/dt_open.c
> >> +++ b/libdtrace/dt_open.c
> >> @@ -263,6 +263,8 @@ static const dt_ident_t _dtrace_globals[] = {
> >> &dt_idops_func, "void(@, ...)" },
> >> { "printf", DT_IDENT_ACTFUNC, 0, DT_ACT_PRINTF, DT_ATTR_STABCMN, DT_VERS_1_0,
> >> &dt_idops_func, "void(string, ...)" },
> >> +{ "print", DT_IDENT_ACTFUNC, 0, DT_ACT_PRINT, DT_ATTR_STABCMN, DT_VERS_2_0,
> >> + &dt_idops_func, "void(void *)" },
> >> { "probefunc", DT_IDENT_SCALAR, 0, DIF_VAR_PROBEFUNC,
> >> DT_ATTR_STABCMN, DT_VERS_1_0, &dt_idops_type, "string" },
> >> { "probemod", DT_IDENT_SCALAR, 0, DIF_VAR_PROBEMOD,
> >> diff --git a/libdtrace/dt_options.c b/libdtrace/dt_options.c
> >> index 33995a6f..3b8eae06 100644
> >> --- a/libdtrace/dt_options.c
> >> +++ b/libdtrace/dt_options.c
> >> @@ -915,6 +915,30 @@ dt_opt_strsize(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> >> return 0;
> >> }
> >>
> >> +#define DT_PRINT_DEF_SIZE 8192
> >> +
> >> +static int
> >> +dt_opt_printsize(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> >> +{
> >> + dtrace_optval_t val = DT_PRINT_DEF_SIZE;
> >> + int rval;
> >> +
> >> + if (arg != NULL) {
> >> + rval = dt_opt_size(dtp, arg, option);
> >> +
> >> + if (rval != 0)
> >> + return rval;
> >> +
> >> + val = dtp->dt_options[option];
> >> +
> >> + if (val <= 0 || val > 65536)
> >> + val = DT_PRINT_DEF_SIZE;
> >> + }
> >> + dtp->dt_options[option] = P2ROUNDUP(val, 8);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static const struct {
> >> const char *dtbp_name;
> >> int dtbp_policy;
> >> @@ -1160,6 +1184,7 @@ static const dt_option_t _dtrace_rtoptions[] = {
> >> { "strsize", dt_opt_strsize, DTRACEOPT_STRSIZE },
> >> { "ustackframes", dt_opt_runtime, DTRACEOPT_USTACKFRAMES },
> >> { "noresolve", dt_opt_runtime, DTRACEOPT_NORESOLVE },
> >> + { "printsize", dt_opt_printsize, DTRACEOPT_PRINTSIZE },
> >
> > I owuld add it to _dtrace_drtoptions instead so it becomes a dynamic runtime
> > option (as mentioned above) and then (of course) the consumer code needs to
> > do the truncation.
> >
> >> { NULL }
> >> };
> >>
> >> --
> >> 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