[DTrace-devel] [PATCH 1/2 v2] Add annotations for dctx->X and mst->X members
Kris Van Hees
kris.van.hees at oracle.com
Mon Mar 28 20:48:00 UTC 2022
On Mon, Mar 28, 2022 at 04:35:42PM -0400, Eugene Loh via DTrace-devel wrote:
> Weren't there going to be tests?
Argh, forgot to include them in the commit.
> Looking just at the mst->X stuff, there are quite a few places where we
> access mst->X. Looking at the dev branch, the only one of those cases that
> this patch would pick up would be the mst->fault check after get_bvar().
> That's not very much. Also not helpful since the get_bvar() is already
> annotated and the stuff afterwards is some boilerplate that isn't made much
> more helpful by adding mst->fault annotation.
>
> With the alloca() patch series, there may be more places to pick up mst->X
> fairly easily. If that's of interest, then let's let the dust settle on
> that patch series first. Improved disassembly can be done later.
>
> Granted, the narrow formula prescribed by this patch won't pick up all of
> the new instances that the alloca() patch series will introduce, but just
> loosening the formula up a little can improve things a lot. E.g. don't look
> just in in[-1] and in[-2]. Rather, walk instructions backwards to look for
> the instructions of interest. If something is a store, keep going.
> Likewise if it's a load to an unrelated register. It's a little more
> complicated, but not much and can sit in its own function.
>
> In short:
> *) need tests
Yes, I have the tests. Forgot to include them. My fault.
> *) might as well do this after the alloca() patch series has stabilized
I don't see why. And in fact, it helps with reviewing the alloca patch, and
there is nothing here that needs waiting for.
> *) then broaden the logic a little to pick up many more cases (I can do
> this if you like)
I'd like to do an improbement like this with a bit more planning, because if
we want to do this, we should do it well, e.g. also track reg-to-reg moves so
we can handle things like:
lddw %r1, [%fp+DT_STK_DCTX] ! dctx
lddw $r1, [%r1+DCTX_MST] ! dctx->mst
<< maybe some other code >>
mov %r2, %r1
lddw %r1, [%r1 + off] ! mst->X
<< maybe some other code >>
lddw %21, [%r2 + off] ! mst->Y
But requires a bit of design to handle disassembler state.
> On 3/26/22 3:04 AM, Kris Van Hees via DTrace-devel wrote:
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_dis.c | 128 +++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 100 insertions(+), 28 deletions(-)
> >
> > diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> > index 792cc99e..bc5eab54 100644
> > --- a/libdtrace/dt_dis.c
> > +++ b/libdtrace/dt_dis.c
> > @@ -82,18 +82,20 @@ dt_dis_varname_off(const dtrace_difo_t *dp, uint_t off, uint_t scope, uint_t add
> > }
> > /*
> > - * Check if we are loading from the gvar, lvar, or strtab BPF map.
> > + * Check if we are accessing the gvars, lvars, or strtab BPF map, or the DTrace
> > + * machine state.
> > *
> > - * If we are loading a gvar or lvar, we want to report the variable name.
> > - * If we are loading a string constant, we want to report its value.
> > + * If we are accessing the gvars or lvars map, we report the variable name.
> > + * If we are accessing strtab map, we report its value.
> > + * If we are accessing the DTrace machine state, we report the member name.
> > *
> > * For variables, the sequence of instructions we are looking for is:
> > * insn code dst src offset imm
> > * -2: ld dst %fp DT_STK_DCTX 00000000
> > * -1: ld dst dst DCTX_*VARS 00000000
> > - * 0: ld dst dst var_offset 00000000
> > - * 0: st dst src var_offset 00000000
> > - * 0: add dst 0 0 var_offset
> > + * 0: ld dst dst offset 00000000 -or-
> > + * 0: st dst src offset 00000000 -or-
> > + * 0: add dst 0 0 offset
> > * where instruction 0 is the current instruction, which may be one
> > * of the three above cases. The three cases represent:
> > * - load by value
> > @@ -104,7 +106,17 @@ dt_dis_varname_off(const dtrace_difo_t *dp, uint_t off, uint_t scope, uint_t add
> > * insn code dst src offset imm
> > * -2: ld dst %fp DT_STK_DCTX 00000000
> > * -1: ld dst dst DCTX_STRTAB 00000000
> > - * 0: add dst 0 0 var_offset
> > + * 0: add dst 0 0 offset
> > + * where instruction 0 is the current instruction.
> > + *
> > + * For DTrace machine state members, the sequence of instructions we are
> > + * looking for is:
> > + * insn code dst src offset imm
> > + * -2: ld dst %fp DT_STK_DCTX 00000000
> > + * -1: ld dst dst DCTX_MST 00000000
> > + * 0: ld dst dst offset 00000000 -or-
> > + * 0: st dst src offset 00000000 -or-
> > + * 0: add dst 0 0 offset
> > * where instruction 0 is the current instruction.
> > */
> > static void
> > @@ -113,7 +125,7 @@ dt_dis_refname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> > {
> > __u8 ldcode = BPF_LDX | BPF_MEM | BPF_DW;
> > __u8 addcode = BPF_ALU64 | BPF_ADD | BPF_K;
> > - int dst, scope = -1, var_offset = -1;
> > + int dst, scope = -1, offset = -1;
> > const char *str;
> > /* make sure in[-2] and in[-1] exist */
> > @@ -127,7 +139,7 @@ dt_dis_refname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> > scope = DIFV_SCOPE_GLOBAL;
> > else if (in[-1].off == DCTX_LVARS)
> > scope = DIFV_SCOPE_LOCAL;
> > - else if (in[-1].off != DCTX_STRTAB)
> > + else if (in[-1].off != DCTX_STRTAB && in[-1].off != DCTX_MST)
> > goto out;
> > /* check preceding instructions */
> > @@ -141,7 +153,7 @@ dt_dis_refname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> > in[-1].imm != 0)
> > goto out;
> > - /* check the current instruction and read var_offset */
> > + /* check the current instruction and read offset */
> > if (in->dst_reg != dst)
> > goto out;
> > if (in[-1].off == DCTX_STRTAB) {
> > @@ -156,21 +168,53 @@ dt_dis_refname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> > } else if (BPF_CLASS(in->code) == BPF_LDX &&
> > BPF_MODE(in->code) == BPF_MEM &&
> > in->src_reg == dst && in->imm == 0)
> > - var_offset = in->off;
> > + offset = in->off;
> > else if (BPF_CLASS(in->code) == BPF_STX &&
> > BPF_MODE(in->code) == BPF_MEM &&
> > in->src_reg != dst && in->imm == 0)
> > - var_offset = in->off;
> > + offset = in->off;
> > else if (in->code == addcode && in->src_reg == 0 && in->off == 0)
> > - var_offset = in->imm;
> > + offset = in->imm;
> > else
> > goto out;
> > /* print name */
> > - str = dt_dis_varname_off(dp, var_offset, scope, addr);
> > - if (str != NULL)
> > - fprintf(fp, "%*s! %s%s", DT_DIS_PAD(n), "",
> > - scope == DIFV_SCOPE_LOCAL ? "this->" : "", str);
> > + if (in[-1].off == DCTX_MST) {
> > + static const char *mst_members[sizeof(dt_mstate_t)] =
> > + {
> > + [0 ... sizeof(dt_mstate_t) - 1] = NULL,
> > + [DMST_EPID] = "epid",
> > + [DMST_PRID] = "prid",
> > + [DMST_CLID] = "clid",
> > + [DMST_TAG] = "tag",
> > + [DMST_ERRNO] = "syscall_errno",
> > + [DMST_FAULT] = "fault",
> > + [DMST_TSTAMP] = "tstamp",
> > + [DMST_REGS] = "regs",
> > + [DMST_ARG(0)] = "arg0",
> > + [DMST_ARG(1)] = "arg1",
> > + [DMST_ARG(2)] = "arg2",
> > + [DMST_ARG(3)] = "arg3",
> > + [DMST_ARG(4)] = "arg4",
> > + [DMST_ARG(5)] = "arg5",
> > + [DMST_ARG(6)] = "arg6",
> > + [DMST_ARG(7)] = "arg7",
> > + [DMST_ARG(8)] = "arg8",
> > + [DMST_ARG(9)] = "arg9",
> > + };
> > +
> > + if (offset < 0 || offset >= sizeof(dt_mstate_t))
> > + goto out;
> > +
> > + str = mst_members[offset];
> > + if (str != NULL)
> > + fprintf(fp, "%*s! mst->%s", DT_DIS_PAD(n), "", str);
> > + } else {
> > + str = dt_dis_varname_off(dp, offset, scope, addr);
> > + if (str != NULL)
> > + fprintf(fp, "%*s! %s%s", DT_DIS_PAD(n), "",
> > + scope == DIFV_SCOPE_LOCAL ? "this->" : "", str);
> > + }
> > out:
> > fprintf(fp, "\n");
> > @@ -252,16 +296,43 @@ dt_dis_load(const dtrace_difo_t *dp, const char *name, uint_t addr,
> > n = fprintf(fp, "%-4s %s, [%s%+d]", name, reg(in->dst_reg),
> > reg(in->src_reg), in->off);
> > - if (in->code == (BPF_LDX | BPF_MEM | BPF_DW) &&
> > - in->src_reg == BPF_REG_FP &&
> > - in->off <= DT_STK_SPILL(0) &&
> > - in->off >= DT_STK_SPILL(DT_STK_NREGS) &&
> > - in->dst_reg == -(in->off - DT_STK_SPILL_BASE) / DT_STK_SLOT_SZ) {
> > - fprintf(fp, "%*s! restore %s\n", DT_DIS_PAD(n), "",
> > - reg(in->dst_reg));
> > - } else
> > - dt_dis_refname(dp, in, addr, n, fp);
> > + if (in->code == (BPF_LDX | BPF_MEM | BPF_DW)) {
> > + if (in->src_reg == BPF_REG_FP && in->off <= DT_STK_SPILL(0) &&
> > + in->off >= DT_STK_SPILL(DT_STK_NREGS) &&
> > + in->dst_reg == -(in->off - DT_STK_SPILL_BASE) /
> > + DT_STK_SLOT_SZ) {
> > + fprintf(fp, "%*s! restore %s\n", DT_DIS_PAD(n), "",
> > + reg(in->dst_reg));
> > + return 0;
> > + } else if (in->src_reg == in->dst_reg && addr > 1 &&
> > + in[-1].code == (BPF_LDX | BPF_MEM | BPF_DW) &&
> > + in[-1].src_reg == BPF_REG_FP &&
> > + in[-1].dst_reg == in->src_reg &&
> > + in[-1].off == DT_STK_DCTX &&
> > + in->off >= 0 && in->off < DCTX_SIZE) {
> > + static const char *dctx_member[DCTX_SIZE] = {
> > + [0 ... DCTX_SIZE - 1] = NULL,
> > + [DCTX_CTX] = "ctx",
> > + [DCTX_ACT] = "act",
> > + [DCTX_MST] = "mst",
> > + [DCTX_BUF] = "buf",
> > + [DCTX_MEM] = "mem",
> > + [DCTX_STRTAB] = "strtab",
> > + [DCTX_AGG] = "agg",
> > + [DCTX_GVARS] = "gvars",
> > + [DCTX_LVARS] = "lvars",
> > + };
> > + const char *s = dctx_member[in->off];
> > +
> > + if (s != NULL) {
> > + fprintf(fp, "%*s! dctx->%s\n",
> > + DT_DIS_PAD(n), "", s);
> > + return 0;
> > + }
> > + }
> > + }
> > + dt_dis_refname(dp, in, addr, n, fp);
> > return 0;
> > }
> > @@ -303,9 +374,10 @@ dt_dis_store(const dtrace_difo_t *dp, const char *name, uint_t addr,
> > in->src_reg == -(in->off - DT_STK_SPILL_BASE) / DT_STK_SLOT_SZ) {
> > fprintf(fp, "%*s! spill %s\n", DT_DIS_PAD(n), "",
> > reg(in->src_reg));
> > - } else
> > - dt_dis_refname(dp, in, addr, n, fp);
> > + return 0;
> > + }
> > + dt_dis_refname(dp, in, addr, n, fp);
> > return 0;
> > }
>
> _______________________________________________
> 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