[DTrace-devel] [PATCH v2 11/12] Add support for umod(), usym(), and uaddr()
Kris Van Hees
kris.van.hees at oracle.com
Fri Jun 11 17:36:27 PDT 2021
Found something else that also applies to ustack... See below...
On Fri, Jun 11, 2021 at 08:17:57PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_cg.c | 26 ++++++++++++++++++++++++--
> libdtrace/dt_consume.c | 17 +++++++++++++----
> test/unittest/ustack/tst.uaddr.d | 17 +++++++++++++++++
> test/unittest/ustack/tst.uaddr.r | 1 +
> test/unittest/ustack/tst.ufunc.d | 17 +++++++++++++++++
> test/unittest/ustack/tst.ufunc.r | 1 +
> test/unittest/ustack/tst.umod.d | 17 +++++++++++++++++
> test/unittest/ustack/tst.umod.r | 1 +
> test/unittest/ustack/tst.usym.d | 17 +++++++++++++++++
> test/unittest/ustack/tst.usym.r | 1 +
> 10 files changed, 109 insertions(+), 6 deletions(-)
> create mode 100644 test/unittest/ustack/tst.uaddr.d
> create mode 100644 test/unittest/ustack/tst.uaddr.r
> create mode 100644 test/unittest/ustack/tst.ufunc.d
> create mode 100644 test/unittest/ustack/tst.ufunc.r
> create mode 100644 test/unittest/ustack/tst.umod.d
> create mode 100644 test/unittest/ustack/tst.umod.r
> create mode 100644 test/unittest/ustack/tst.usym.d
> create mode 100644 test/unittest/ustack/tst.usym.r
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index aff718dd..f68f47dc 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -723,6 +723,7 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> dt_pfargv_t *pfp, int arg)
> {
> dtrace_diftype_t vtype;
> + dtrace_hdl_t *dtp = pcb->pcb_hdl;
> dt_irlist_t *dlp = &pcb->pcb_ir;
> dt_regset_t *drp = pcb->pcb_regs;
> uint_t off;
> @@ -739,13 +740,34 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> size = sizeof(dnp->dn_ident->di_id);
> } else {
> dt_cg_node(dnp, &pcb->pcb_ir, drp);
> - dt_node_diftype(pcb->pcb_hdl, dnp, &vtype);
> + dt_node_diftype(dtp, dnp, &vtype);
> size = vtype.dtdt_size;
> }
>
> + if (kind == DTRACEACT_USYM ||
> + kind == DTRACEACT_UMOD ||
> + kind == DTRACEACT_UADDR) {
> + off = dt_rec_add(dtp, dt_cg_fill_gap, kind, 16, 8, NULL, arg);
> +
> + /* preface the value with the user process tgid */
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + dt_regset_xalloc(drp, BPF_REG_0);
> + emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
> + dt_regset_free_args(drp);
> + emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, off, BPF_REG_0));
> + dt_regset_free(drp, BPF_REG_0);
The tgid is a 32-bit value that you are writing to a 64-bit wide space in the
output buffer using a BPF_W store instruction, so it only touches 4 bytes.
But that means that the other 8 bytes have undefined data in them. But then
you read that in (dt_consume functions) as a 64-bit value. Most of the time
that will work, but I can see situations where eventually you may run out of
luck and there will be non-zero data in those other 4 bytes.
The easiest solution would be to actually read the tgid as a 32-bit wide
integer, ignoring the 4 padding bytes.
Alternatively (but slightly more complex in code) you could actually have two
records for each U* action, one with a 4-byte data item, and one with a 8-byte
data item. That is supported (since there are actually quite a few actions
that have multiple data items associated with them), but again - slightly more
complex even if it is 'cleaner'.
Either way, I am pretty certain that you do need to do something about this
32-bit vs 64-bit data issue in view of the potential for non-zero data in the
padding bytes.
> +
> + /* then store the value */
> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_9, off + 8, dnp->dn_reg));
> + dt_regset_free(drp, dnp->dn_reg);
> +
> + return 0;
> + }
> +
> if (dt_node_is_scalar(dnp) || dt_node_is_float(dnp) ||
> dnp->dn_kind == DT_NODE_AGG) {
> - off = dt_rec_add(pcb->pcb_hdl, dt_cg_fill_gap, kind,
> + off = dt_rec_add(dtp, dt_cg_fill_gap, kind,
> size, size, pfp, arg);
>
> assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index afcaae42..8eeabcfe 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1234,9 +1234,9 @@ static int
> dt_print_usym(dtrace_hdl_t *dtp, FILE *fp, caddr_t addr, dtrace_actkind_t act)
> {
> /* LINTED - alignment */
> - uint64_t tgid = ((uint64_t *)addr)[1];
> + uint64_t tgid = ((uint64_t *)addr)[0];
> /* LINTED - alignment */
> - uint64_t pc = ((uint64_t *)addr)[2];
> + uint64_t pc = ((uint64_t *)addr)[1];
> const char *format = " %-50s";
> char *s;
> int n, len = 256;
> @@ -1268,9 +1268,9 @@ int
> dt_print_umod(dtrace_hdl_t *dtp, FILE *fp, const char *format, caddr_t addr)
> {
> /* LINTED - alignment */
> - uint64_t tgid = ((uint64_t *)addr)[1];
> + uint64_t tgid = ((uint64_t *)addr)[0];
> /* LINTED - alignment */
> - uint64_t pc = ((uint64_t *)addr)[2];
> + uint64_t pc = ((uint64_t *)addr)[1];
> int err = 0;
>
> char objname[PATH_MAX], c[PATH_MAX * 2];
> @@ -2082,6 +2082,15 @@ dt_consume_one(dtrace_hdl_t *dtp, FILE *fp, char *buf,
> recdata, rec->dtrd_arg) < 0)
> return -1;
> continue;
> + case DTRACEACT_USYM:
> + case DTRACEACT_UADDR:
> + if (dt_print_usym(dtp, fp, recdata, act) < 0)
> + return -1;
> + continue;
> + case DTRACEACT_UMOD:
> + if (dt_print_umod(dtp, fp, NULL, recdata) < 0)
> + return -1;
> + continue;
> case DTRACEACT_PRINTF:
> func = dtrace_fprintf;
> break;
> diff --git a/test/unittest/ustack/tst.uaddr.d b/test/unittest/ustack/tst.uaddr.d
> new file mode 100644
> index 00000000..0fce24d8
> --- /dev/null
> +++ b/test/unittest/ustack/tst.uaddr.d
> @@ -0,0 +1,17 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/* @@trigger: ustack-tst-basic */
> +
> +#pragma D option quiet
> +
> +profile-1
> +/pid == $target/
> +{
> + uaddr(ucaller);
> + exit(0);
> +}
> diff --git a/test/unittest/ustack/tst.uaddr.r b/test/unittest/ustack/tst.uaddr.r
> new file mode 100644
> index 00000000..be48a12a
> --- /dev/null
> +++ b/test/unittest/ustack/tst.uaddr.r
> @@ -0,0 +1 @@
> + ustack-tst-basic`myfunc_y+{ptr}
> diff --git a/test/unittest/ustack/tst.ufunc.d b/test/unittest/ustack/tst.ufunc.d
> new file mode 100644
> index 00000000..546653de
> --- /dev/null
> +++ b/test/unittest/ustack/tst.ufunc.d
> @@ -0,0 +1,17 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/* @@trigger: ustack-tst-basic */
> +
> +#pragma D option quiet
> +
> +profile-1
> +/pid == $target/
> +{
> + ufunc(ucaller);
> + exit(0);
> +}
> diff --git a/test/unittest/ustack/tst.ufunc.r b/test/unittest/ustack/tst.ufunc.r
> new file mode 100644
> index 00000000..2ebedb99
> --- /dev/null
> +++ b/test/unittest/ustack/tst.ufunc.r
> @@ -0,0 +1 @@
> + ustack-tst-basic`myfunc_y
> diff --git a/test/unittest/ustack/tst.umod.d b/test/unittest/ustack/tst.umod.d
> new file mode 100644
> index 00000000..b7d3e14f
> --- /dev/null
> +++ b/test/unittest/ustack/tst.umod.d
> @@ -0,0 +1,17 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/* @@trigger: ustack-tst-basic */
> +
> +#pragma D option quiet
> +
> +profile-1
> +/pid == $target/
> +{
> + umod(ucaller);
> + exit(0);
> +}
> diff --git a/test/unittest/ustack/tst.umod.r b/test/unittest/ustack/tst.umod.r
> new file mode 100644
> index 00000000..2a5b8297
> --- /dev/null
> +++ b/test/unittest/ustack/tst.umod.r
> @@ -0,0 +1 @@
> + ustack-tst-basic
> diff --git a/test/unittest/ustack/tst.usym.d b/test/unittest/ustack/tst.usym.d
> new file mode 100644
> index 00000000..3bd9dc23
> --- /dev/null
> +++ b/test/unittest/ustack/tst.usym.d
> @@ -0,0 +1,17 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/* @@trigger: ustack-tst-basic */
> +
> +#pragma D option quiet
> +
> +profile-1
> +/pid == $target/
> +{
> + usym(ucaller);
> + exit(0);
> +}
> diff --git a/test/unittest/ustack/tst.usym.r b/test/unittest/ustack/tst.usym.r
> new file mode 100644
> index 00000000..2ebedb99
> --- /dev/null
> +++ b/test/unittest/ustack/tst.usym.r
> @@ -0,0 +1 @@
> + ustack-tst-basic`myfunc_y
> --
> 2.18.4
>
>
> _______________________________________________
> 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