[DTrace-devel] [PATCH] Convert dvars to use the default block of zeros
Eugene Loh
eugene.loh at oracle.com
Thu Aug 4 17:03:13 UTC 2022
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
In the commit msg: s/relfect/reflect/.
I'm getting confused about the order of patches, but in dt_bpf.c I
thought there was a block of code that computed dt_zerosize, bumping it
up depending on dt_maxtuplesize, dt_aggsiz, etc. That would seem to be
the right place to bump it up per strsize + 1 and dt_maxdvarsize as
well. As it is, the current patch seems to compute dt_zerosize based
only on dt_maxdvarsize and strsize+1, ignoring any previous information
about dt_zerosize (which would have incorporated info on dt_maxtuplesize
and dt_aggsiz).
In dt_cg.c, is there any chance of introducing dt_cg_zerosptr() in the
correct place in the earlier patch rather than having to move it in the
current patch?
On 8/4/22 06:55, Kris Van Hees via DTrace-devel wrote:
> The code sequence for retrieving dynamic variables (TLS variables and
> associative arrays) is modified, so the disassembler annotation code is
> updated to relfect this. As a useful side effect, relocation symbols
> are now reported for mov and arithmetic intructions as well.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> bpf/get_dvar.c | 21 ++++++++----------
> libdtrace/dt_bpf.c | 20 +++++------------
> libdtrace/dt_cg.c | 32 ++++++++++++++++------------
> libdtrace/dt_dis.c | 24 ++++++++++++++-------
> test/unittest/disasm/tst.ann-tvar.r | 4 +++-
> test/unittest/disasm/tst.ann-tvar.sh | 4 +++-
> 6 files changed, 54 insertions(+), 51 deletions(-)
>
> diff --git a/bpf/get_dvar.c b/bpf/get_dvar.c
> index 7e9c0fc7..d11182df 100644
> --- a/bpf/get_dvar.c
> +++ b/bpf/get_dvar.c
> @@ -46,9 +46,9 @@ noinline uint64_t dt_tlskey(uint32_t id)
> return key;
> }
>
> -noinline void *dt_get_dvar(uint64_t key, uint64_t store, uint64_t nval)
> +noinline void *dt_get_dvar(uint64_t key, uint64_t store, uint64_t nval,
> + const char *dflt)
> {
> - uint64_t dflt_key = 0;
> void *val;
>
> /*
> @@ -79,11 +79,7 @@ noinline void *dt_get_dvar(uint64_t key, uint64_t store, uint64_t nval)
> * Not found and we are storing a non-zero value: create the variable
> * with the default value.
> */
> - val = bpf_map_lookup_elem(&dvars, &dflt_key);
> - if (val == 0)
> - return 0;
> -
> - if (bpf_map_update_elem(&dvars, &key, val, BPF_ANY) < 0)
> + if (bpf_map_update_elem(&dvars, &key, dflt, BPF_ANY) < 0)
> return 0;
>
> val = bpf_map_lookup_elem(&dvars, &key);
> @@ -93,13 +89,14 @@ noinline void *dt_get_dvar(uint64_t key, uint64_t store, uint64_t nval)
> return 0;
> }
>
> -noinline void *dt_get_tvar(uint32_t id, uint64_t store, uint64_t nval)
> +noinline void *dt_get_tvar(uint32_t id, uint64_t store, uint64_t nval,
> + const char *dflt)
> {
> - return dt_get_dvar(dt_tlskey(id), store, nval);
> + return dt_get_dvar(dt_tlskey(id), store, nval, dflt);
> }
>
> -noinline void *dt_get_assoc(uint32_t id, const char *tuple,
> - uint64_t store, uint64_t nval)
> +noinline void *dt_get_assoc(uint32_t id, const char *tuple, uint64_t store,
> + uint64_t nval, const char *dflt)
> {
> uint64_t *valp;
> uint64_t val;
> @@ -151,5 +148,5 @@ noinline void *dt_get_assoc(uint32_t id, const char *tuple,
> bpf_map_delete_elem(&tuples, tuple);
> }
>
> - return dt_get_dvar(val, store, nval);
> + return dt_get_dvar(val, store, nval, dflt);
> }
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 17562b9d..d5b8e21b 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -431,7 +431,8 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> */
> dtp->dt_strlen = dt_strtab_size(dtp->dt_ccstab);
> dtp->dt_zerooffset = P2ROUNDUP(dtp->dt_strlen, 8);
> - sz = dtp->dt_zerooffset + MAX(strsize + 1, dtp->dt_zerosize);
> + dtp->dt_zerosize = MAX(dtp->dt_maxdvarsize, strsize + 1);
> + sz = dtp->dt_zerooffset + dtp->dt_zerosize;
> strtab = dt_zalloc(dtp, sz);
> if (strtab == NULL)
> return dt_set_errno(dtp, EDT_NOMEM);
> @@ -481,22 +482,11 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> dvarc = dtp->dt_options[DTRACEOPT_DYNVARSIZE] /
> dtp->dt_maxdvarsize;
>
> - if (dvarc > 0) {
> - int fd;
> - char dflt[dtp->dt_maxdvarsize];
> -
> - /* Allocate one extra element for the default value. */
> - fd = create_gmap(dtp, "dvars", BPF_MAP_TYPE_HASH,
> - sizeof(uint64_t), dtp->dt_maxdvarsize,
> - dvarc + 1);
> - if (fd == -1)
> + if (dvarc > 0 &&
> + create_gmap(dtp, "dvars", BPF_MAP_TYPE_HASH,
> + sizeof(uint64_t), dtp->dt_maxdvarsize, dvarc) == -1)
> return -1; /* dt_errno is set for us */
>
> - /* Initialize the default value (key = 0). */
> - memset(dflt, 0, dtp->dt_maxdvarsize);
> - dt_bpf_map_update(fd, &key, &dflt);
> - }
> -
> if (dtp->dt_maxtuplesize > 0 &&
> create_gmap(dtp, "tuples", BPF_MAP_TYPE_HASH,
> dtp->dt_maxtuplesize, sizeof(uint64_t), dvarc) == -1)
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cb79c5b4..c5db5fa3 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2081,6 +2081,20 @@ dt_cg_membinfo(ctf_file_t *fp, ctf_id_t type, const char *s, ctf_membinfo_t *mp)
> return fp;
> }
>
> +/*
> + * Store a pointer to the 'memory block of zeros' in reg.
> + */
> +static void
> +dt_cg_zerosptr(int reg, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> + dtrace_hdl_t *dtp = yypcb->pcb_hdl;
> + dt_ident_t *zero_off = dt_dlib_get_var(dtp, "ZERO_OFF");
> +
> + emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_LOAD(BPF_DW, reg, reg, DCTX_STRTAB));
> + emite(dlp, BPF_ALU64_IMM(BPF_ADD, reg, -1), zero_off);
> +}
> +
> void
> dt_cg_xsetx(dt_irlist_t *dlp, dt_ident_t *idp, uint_t lbl, int reg, uint64_t x)
> {
> @@ -2217,6 +2231,7 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
> emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
> emit(dlp, BPF_MOV_IMM(BPF_REG_2, 0));
> emit(dlp, BPF_MOV_IMM(BPF_REG_3, 0));
> + dt_cg_zerosptr(BPF_REG_4, dlp, drp);
> dt_regset_xalloc(drp, BPF_REG_0);
> emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> dt_regset_free_args(drp);
> @@ -2561,20 +2576,6 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
> }
> }
>
> -/*
> - * Store a pointer to the 'memory block of zeros' in reg.
> - */
> -static void
> -dt_cg_zerosptr(int reg, dt_irlist_t *dlp, dt_regset_t *drp)
> -{
> - dtrace_hdl_t *dtp = yypcb->pcb_hdl;
> - dt_ident_t *zero_off = dt_dlib_get_var(dtp, "ZERO_OFF");
> -
> - emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_FP, DT_STK_DCTX));
> - emit(dlp, BPF_LOAD(BPF_DW, reg, reg, DCTX_STRTAB));
> - emite(dlp, BPF_ALU64_IMM(BPF_ADD, reg, -1), zero_off);
> -}
> -
> /*
> * Generate code to push the specified argument list on to the tuple stack.
> * We use this routine for handling the index tuple for associative arrays.
> @@ -2830,6 +2831,7 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> dt_regset_free(drp, dnp->dn_left->dn_args->dn_reg);
> emit(dlp, BPF_MOV_IMM(BPF_REG_3, 1));
> emit(dlp, BPF_MOV_REG(BPF_REG_4, dnp->dn_reg));
> + dt_cg_zerosptr(BPF_REG_5, dlp, drp);
> dt_regset_xalloc(drp, BPF_REG_0);
> emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> dt_regset_free_args(drp);
> @@ -2929,6 +2931,7 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
> emit(dlp, BPF_MOV_IMM(BPF_REG_2, 1));
> emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
> + dt_cg_zerosptr(BPF_REG_4, dlp, drp);
> dt_regset_xalloc(drp, BPF_REG_0);
> emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> dt_regset_free_args(drp);
> @@ -3632,6 +3635,7 @@ dt_cg_assoc_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dt_regset_free(drp, dnp->dn_args->dn_reg);
> emit(dlp, BPF_MOV_IMM(BPF_REG_3, 0));
> emit(dlp, BPF_MOV_IMM(BPF_REG_4, 0));
> + dt_cg_zerosptr(BPF_REG_5, dlp, drp);
> dt_regset_xalloc(drp, BPF_REG_0);
> emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> dt_regset_free_args(drp);
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index 9e6d3be8..ecbe7524 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -106,16 +106,24 @@ dt_dis_varname_off(const dtrace_difo_t *dp, uint_t off, uint_t scope, uint_t add
> * -1: ld dst dst DCTX_STRTAB 00000000
> * 0: add dst 0 0 var_offset
> * where instruction 0 is the current instruction.
> + *
> + * In all cases, if a relocation symbol name is provided, report that.
> */
> static void
> dt_dis_refname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> - int n, FILE *fp)
> + const char *rname, int n, FILE *fp)
> {
> __u8 ldcode = BPF_LDX | BPF_MEM | BPF_DW;
> __u8 addcode = BPF_ALU64 | BPF_ADD | BPF_K;
> int dst, scope = -1, var_offset = -1;
> const char *str;
>
> + /* if we have a relocation symbol name, use that */
> + if (rname != NULL) {
> + fprintf(fp, "%*s! %s", DT_DIS_PAD(n), "", rname);
> + goto out;
> + }
> +
> /* make sure in[-2] and in[-1] exist */
> if (addr < 2)
> goto out;
> @@ -211,7 +219,7 @@ dt_dis_op2imm(const dtrace_difo_t *dp, const char *name, uint_t addr,
> int n;
>
> n = fprintf(fp, "%-4s %s, %d", name, reg(in->dst_reg), in->imm);
> - dt_dis_refname(dp, in, addr, n, fp);
> + dt_dis_refname(dp, in, addr, rname, n, fp);
>
> return 0;
> }
> @@ -273,7 +281,7 @@ dt_dis_load(const dtrace_difo_t *dp, const char *name, uint_t addr,
> fprintf(fp, "%*s! restore %s\n", DT_DIS_PAD(n), "",
> reg(in->dst_reg));
> } else
> - dt_dis_refname(dp, in, addr, n, fp);
> + dt_dis_refname(dp, in, addr, rname, n, fp);
>
> return 0;
> }
> @@ -317,7 +325,7 @@ dt_dis_store(const dtrace_difo_t *dp, const char *name, uint_t addr,
> fprintf(fp, "%*s! spill %s\n", DT_DIS_PAD(n), "",
> reg(in->src_reg));
> } else
> - dt_dis_refname(dp, in, addr, n, fp);
> + dt_dis_refname(dp, in, addr, rname, n, fp);
>
> return 0;
> }
> @@ -357,12 +365,12 @@ dt_dis_bpf_args(const dtrace_difo_t *dp, const char *fn,
> return buf;
> } else if (strcmp(fn, "dt_get_tvar") == 0) {
> /*
> - * We know that the previous three instructions exist and
> + * We know that the previous six instructions exist and
> * move the variable id to a register in the first instruction
> * of that seqeuence (because we wrote the code generator to
> * emit the instructions in this exact order.)
> */
> - in -= 3;
> + in -= 6;
> snprintf(buf, len, "self->%s",
> dt_dis_varname_id(dp, in->imm + DIF_VAR_OTHER_UBASE,
> DIFV_SCOPE_THREAD, addr));
> @@ -371,12 +379,12 @@ dt_dis_bpf_args(const dtrace_difo_t *dp, const char *fn,
> uint_t varid;
>
> /*
> - * We know that the previous four instructions exist and
> + * We know that the previous seven instructions exist and
> * move the variable id to a register in the first instruction
> * of that seqeuence (because we wrote the code generator to
> * emit the instructions in this exact order.)
> */
> - in -= 4;
> + in -= 7;
> varid = in->imm + DIF_VAR_OTHER_UBASE;
>
> /*
> diff --git a/test/unittest/disasm/tst.ann-tvar.r b/test/unittest/disasm/tst.ann-tvar.r
> index 5194531b..39065fdc 100644
> --- a/test/unittest/disasm/tst.ann-tvar.r
> +++ b/test/unittest/disasm/tst.ann-tvar.r
> @@ -1 +1,3 @@
> -85 0 1 0000 ffffffff call dt_get_tvar ! self->var
> +85 0 1 0000 ffffffff call dt_get_tvar ! self->one
> +85 0 1 0000 ffffffff call dt_get_tvar ! self->two
> +85 0 1 0000 ffffffff call dt_get_tvar ! self->three
> diff --git a/test/unittest/disasm/tst.ann-tvar.sh b/test/unittest/disasm/tst.ann-tvar.sh
> index 66501840..729e703c 100755
> --- a/test/unittest/disasm/tst.ann-tvar.sh
> +++ b/test/unittest/disasm/tst.ann-tvar.sh
> @@ -11,7 +11,9 @@ dtrace=$1
> $dtrace $dt_flags -Sen '
> BEGIN
> {
> - self->var = 42;
> + self->one = 42;
> + self->two = 42;
> + self->three = 42;
> exit(0);
> }
> ' 2>&1 | awk '/ call dt_get_tvar/ { sub(/^[^:]+: /, ""); print; }'
More information about the DTrace-devel
mailing list