[DTrace-devel] [PATCH] Convert dvars to use the default block of zeros
Kris Van Hees
kris.van.hees at oracle.com
Thu Aug 4 19:25:38 UTC 2022
On Thu, Aug 04, 2022 at 10:03:13AM -0700, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>
> In the commit msg: s/relfect/reflect/.
Thanks.
> 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).
No, the patch that introduces the block of zeros never assigns a value to
zerosize (not in your patch, and not in my version). Since this patch is the
first use of that block, it introduces determining the value of zerosize.
Later uses (in your patches) will need to add further logic to determine the
size needed.
> 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?
Yes, I will update the previous patch (introducing the block of zeros) to
put the function in its proper place. I meant to do that but it ended up in\
this patch instead of in the previous one.
>
> 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; }'
>
> _______________________________________________
> 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