[DTrace-devel] [PATCH 1/2 v2] Add annotations for dctx->X and mst->X members
Eugene Loh
eugene.loh at oracle.com
Mon Mar 28 20:35:42 UTC 2022
Weren't there going to be tests?
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
*) might as well do this after the alloca() patch series has stabilized
*) then broaden the logic a little to pick up many more cases (I can do
this if you like)
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;
> }
>
More information about the DTrace-devel
mailing list