[DTrace-devel] [PATCH v3 5/6] Fix disassembly for global and local variables

Kris Van Hees kris.van.hees at oracle.com
Fri Apr 2 21:14:51 PDT 2021


Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

On Thu, Apr 01, 2021 at 11:32:10PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Recent patches have changed how global and local variables are
> accessed.  This means that the disassembler, which relies on
> recognizing BPF instructions to add annotations, must be updated.
> 
> Add a "struct dtrace_difv" field dtdv_offset so that variables
> can be looked up by offset rather than ID.
> 
> Rename dt_dis_varname() to dt_dis_varname_id().  Add a by-offset
> lookup function dt_dis_varname_off().  Eliminate dt_dis_lvarname().
> Replace calls to dt_dis_lvarname() with a new dt_dis_varname()
> that prints the variable name if any can be found.  Note that we
> do not need to check "store_imm" instructions for a variable name,
> but we must check op2imm instructions (for by-reference accesses).
> 
> Add a test, checking global and local scalars and structs.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  include/dtrace/dif.h                 |   1 +
>  libdtrace/dt_as.c                    |   1 +
>  libdtrace/dt_dis.c                   | 152 ++++++++++++++++++---------
>  test/unittest/dif/tst.DisVarNames.r  |  23 ++++
>  test/unittest/dif/tst.DisVarNames.sh |  53 ++++++++++
>  5 files changed, 180 insertions(+), 50 deletions(-)
>  create mode 100644 test/unittest/dif/tst.DisVarNames.r
>  create mode 100755 test/unittest/dif/tst.DisVarNames.sh
> 
> diff --git a/include/dtrace/dif.h b/include/dtrace/dif.h
> index ad4d2f3a..191aa73c 100644
> --- a/include/dtrace/dif.h
> +++ b/include/dtrace/dif.h
> @@ -50,6 +50,7 @@ typedef struct dtrace_diftype {
>  typedef struct dtrace_difv {
>  	uint32_t dtdv_name;
>  	uint32_t dtdv_id;
> +	uint32_t dtdv_offset;
>  	uint8_t dtdv_kind;
>  	uint8_t dtdv_scope;
>  	uint32_t dtdv_insn_from;
> diff --git a/libdtrace/dt_as.c b/libdtrace/dt_as.c
> index 786fc22c..f648f21a 100644
> --- a/libdtrace/dt_as.c
> +++ b/libdtrace/dt_as.c
> @@ -92,6 +92,7 @@ dt_copyvar(dt_idhash_t *dhp, dt_ident_t *idp, dtrace_hdl_t *dtp)
>  
>  	dvp->dtdv_name = (uint_t)stroff;
>  	dvp->dtdv_id = idp->di_id;
> +	dvp->dtdv_offset = idp->di_offset;
>  	dvp->dtdv_flags = 0;
>  
>  	switch (idp->di_kind) {
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index d4f4ad67..7b7c39f9 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -45,7 +45,7 @@ reg(int r)
>  }
>  
>  static const char *
> -dt_dis_varname(const dtrace_difo_t *dp, uint_t id, uint_t scope, uint_t addr)
> +dt_dis_varname_id(const dtrace_difo_t *dp, uint_t id, uint_t scope, uint_t addr)
>  {
>  	const dtrace_difv_t *dvp = dp->dtdo_vartab;
>  	uint_t i;
> @@ -63,26 +63,98 @@ dt_dis_varname(const dtrace_difo_t *dp, uint_t id, uint_t scope, uint_t addr)
>  	return NULL;
>  }
>  
> -static char *
> -dt_dis_lvarname(const dtrace_difo_t *dp, int reg, int var, uint_t addr,
> -		char *buf, int len)
> +static const char *
> +dt_dis_varname_off(const dtrace_difo_t *dp, uint_t off, uint_t scope, uint_t addr)
>  {
> -	if (reg == BPF_REG_FP) {
> -		var = -1; /* FIXME:  need to find variable offset */
> -		if (var != -1) {
> -			const char	*vname;
> -
> -			vname = dt_dis_varname(dp, var, DIFV_SCOPE_LOCAL, addr);
> -			if (vname) {
> -				snprintf(buf, len, "this->%s", vname);
> -				return buf;
> -			}
> +	uint_t i;
> +
> +	for (i = 0; i < dp->dtdo_varlen; i++) {
> +		const dtrace_difv_t *dvp = &dp->dtdo_vartab[i];
> +		if (dvp->dtdv_offset == off && dvp->dtdv_scope == scope &&
> +		    dvp->dtdv_insn_from <= addr && addr <= dvp->dtdv_insn_to) {
> +			if (dvp->dtdv_name < dp->dtdo_strlen)
> +				return dp->dtdo_strtab + dvp->dtdv_name;
> +			break;
>  		}
>  	}
>  
>  	return NULL;
>  }
>  
> +/*
> + * Check if we are loading either the gvar or lvar BPF map.  If so,
> + * we want to report the name of the variable it is looking up.
> + * 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
> + * where instruction 0 is the current instruction, which may be one
> + * of the three above cases.  The three cases represent:
> + *   - load by value
> + *   - store by value
> + *   - access by reference
> + */
> +static void
> +dt_dis_varname(const dtrace_difo_t *dp, const struct bpf_insn *in, uint_t addr,
> +	       int n, FILE *fp)
> +{
> +	__u8		ldcode = BPF_LDX | BPF_MEM | BPF_DW;
> +	__u8		stcode = BPF_STX | BPF_MEM | BPF_DW;
> +	__u8		addcode = BPF_ALU64 | BPF_ADD | BPF_K;
> +	int		dst, scope, var_offset = -1;
> +	const char	*vname;
> +
> +	/* make sure in[-2] and in[-1] exist */
> +	if (addr < 2)
> +		goto out;
> +
> +	/* get dst and scope */
> +	dst = in[-1].dst_reg;
> +
> +	if (in[-1].off == DCTX_GVARS)
> +		scope = DIFV_SCOPE_GLOBAL;
> +	else if (in[-1].off == DCTX_LVARS)
> +		scope = DIFV_SCOPE_LOCAL;
> +	else
> +		goto out;
> +
> +	/* check preceding instructions */
> +	if (in[-2].code != ldcode ||
> +	    in[-2].dst_reg != dst ||
> +	    in[-2].src_reg != BPF_REG_FP ||
> +	    in[-2].off != DT_STK_DCTX ||
> +	    in[-2].imm != 0 ||
> +	    in[-1].code != ldcode ||
> +	    in[-1].src_reg != dst ||
> +	    in[-1].imm != 0)
> +		goto out;
> +
> +	/* check the current instruction and read var_offset */
> +	if (in->dst_reg != dst)
> +		goto out;
> +	if (in->code == ldcode && in->src_reg == dst && in->imm == 0)
> +		var_offset = in->off;
> +	else if (in->code == stcode && in->src_reg != dst && in->imm == 0)
> +		var_offset = in->off;
> +	else if (in->code == addcode && in->src_reg == 0 && in->off == 0)
> +		var_offset = in->imm;
> +	else
> +		goto out;
> +
> +	/* print name */
> +	vname = dt_dis_varname_off(dp, var_offset, scope, addr);
> +	if (vname == NULL)
> +		goto out;
> +	fprintf(fp, "%*s! %s%s", DT_DIS_PAD(n), "",
> +		scope == DIFV_SCOPE_LOCAL ? "this->" : "", vname);
> +
> +out:
> +	fprintf(fp, "\n");
> +}
> +
>  /*ARGSUSED*/
>  static uint_t
>  dt_dis_str(const dtrace_difo_t *dp, const char *name, uint_t addr,
> @@ -115,7 +187,11 @@ static uint_t
>  dt_dis_op2imm(const dtrace_difo_t *dp, const char *name, uint_t addr,
>  	      const struct bpf_insn *in, const char *rname, FILE *fp)
>  {
> -	fprintf(fp, "%-4s %s, %d\n", name, reg(in->dst_reg), in->imm);
> +	int		n;
> +
> +	n = fprintf(fp, "%-4s %s, %d", name, reg(in->dst_reg), in->imm);
> +	dt_dis_varname(dp, in, addr, n, fp);
> +
>  	return 0;
>  }
>  
> @@ -150,19 +226,12 @@ static uint_t
>  dt_dis_load(const dtrace_difo_t *dp, const char *name, uint_t addr,
>  	      const struct bpf_insn *in, const char *rname, FILE *fp)
>  {
> -	char		buf[256];
> -	const char	*vname;
>  	int		n;
>  
>  	n = fprintf(fp, "%-4s %s, [%s%+d]", name, reg(in->dst_reg),
>  		    reg(in->src_reg), in->off);
>  
> -	vname = dt_dis_lvarname(dp, in->src_reg, in->off, addr, buf,
> -				sizeof(buf));
> -	if (vname)
> -		fprintf(fp, "%*s! %s\n", DT_DIS_PAD(n), "", vname);
> -	else
> -		fprintf(fp, "\n");
> +	dt_dis_varname(dp, in, addr, n, fp);
>  
>  	return 0;
>  }
> @@ -193,19 +262,12 @@ static uint_t
>  dt_dis_store(const dtrace_difo_t *dp, const char *name, uint_t addr,
>  	     const struct bpf_insn *in, const char *rname, FILE *fp)
>  {
> -	char		buf[256];
> -	const char	*vname;
>  	int		n;
>  
>  	n = fprintf(fp, "%-4s [%s%+d], %s", name, reg(in->dst_reg), in->off,
>  		    reg(in->src_reg));
>  
> -	vname = dt_dis_lvarname(dp, in->dst_reg, in->off, addr, buf,
> -				sizeof(buf));
> -	if (vname)
> -		fprintf(fp, "%*s! %s\n", DT_DIS_PAD(n), "", vname);
> -	else
> -		fprintf(fp, "\n");
> +	dt_dis_varname(dp, in, addr, n, fp);
>  
>  	return 0;
>  }
> @@ -215,24 +277,14 @@ static uint_t
>  dt_dis_store_imm(const dtrace_difo_t *dp, const char *name, uint_t addr,
>  		 const struct bpf_insn *in, const char *rname, FILE *fp)
>  {
> -	char		buf[256];
> -	const char	*vname;
>  	int		n;
>  
>  	n = fprintf(fp, "%-4s [%s%+d], %d", name, reg(in->dst_reg), in->off,
>  		    in->imm);
>  
> -	vname = dt_dis_lvarname(dp, in->dst_reg, in->off, addr, buf,
> -				sizeof(buf));
> -	if (vname) {
> -		if (rname)
> -			fprintf(fp, "%*s! %s = %s\n", DT_DIS_PAD(n), "",
> -				vname, rname);
> -		else
> -			fprintf(fp, "%*s! %s\n", DT_DIS_PAD(n), "", vname);
> -	} else if (rname) {
> +	if (rname)
>  		fprintf(fp, "%*s! = %s\n", DT_DIS_PAD(n), "", rname);
> -	} else
> +	else
>  		fprintf(fp, "\n");
>  
>  	return 0;
> @@ -252,7 +304,7 @@ dt_dis_bpf_args(const dtrace_difo_t *dp, const char *fn,
>  		 */
>  		in--;
>  		snprintf(buf, len, "%s",
> -			 dt_dis_varname(dp, in->imm, DIFV_SCOPE_GLOBAL, addr));
> +			 dt_dis_varname_id(dp, in->imm, DIFV_SCOPE_GLOBAL, addr));
>  		return buf;
>  	} else if (strcmp(fn, "dt_get_tvar") == 0 ||
>  		   strcmp(fn, "dt_set_tvar") == 0) {
> @@ -263,7 +315,7 @@ dt_dis_bpf_args(const dtrace_difo_t *dp, const char *fn,
>  		 */
>  		in--;
>  		snprintf(buf, len, "self->%s",
> -			 dt_dis_varname(dp, in->imm + DIF_VAR_OTHER_UBASE,
> +			 dt_dis_varname_id(dp, in->imm + DIF_VAR_OTHER_UBASE,
>  					DIFV_SCOPE_THREAD, addr));
>  		return buf;
>  	} else if (strcmp(fn, "dt_get_string") == 0) {
> @@ -656,8 +708,8 @@ dt_dis_difo(const dtrace_difo_t *dp, FILE *fp, const dt_ident_t *idp,
>  	}
>  
>  	if (dp->dtdo_varlen != 0) {
> -		fprintf(fp, "\n%-16s %-4s %-3s %-3s %-11s %-4s %s\n",
> -			"NAME", "ID", "KND", "SCP", "RANGE", "FLAG", "TYPE");
> +		fprintf(fp, "\n%-16s %-4s %-6s %-3s %-3s %-11s %-4s %s\n",
> +			"NAME", "ID", "OFFSET", "KND", "SCP", "RANGE", "FLAG", "TYPE");
>  	}
>  
>  	for (i = 0; i < dp->dtdo_varlen; i++) {
> @@ -705,9 +757,9 @@ dt_dis_difo(const dtrace_difo_t *dp, FILE *fp, const dt_ident_t *idp,
>  		if (v->dtdv_flags & DIFV_F_MOD)
>  			strcat(flags, "/w");
>  
> -		fprintf(fp, "%-16s %-4x %-3s %-3s %-11s %-4s %s\n",
> -			&dp->dtdo_strtab[v->dtdv_name], v->dtdv_id, kind,
> -			scope, range, flags + 1,
> +		fprintf(fp, "%-16s %-4x %-6x %-3s %-3s %-11s %-4s %s\n",
> +			&dp->dtdo_strtab[v->dtdv_name], v->dtdv_id,
> +			v->dtdv_offset, kind, scope, range, flags + 1,
>  			dt_dis_typestr(&v->dtdv_type, type, sizeof(type)));
>  	}
>  
> diff --git a/test/unittest/dif/tst.DisVarNames.r b/test/unittest/dif/tst.DisVarNames.r
> new file mode 100644
> index 00000000..d7fdef90
> --- /dev/null
> +++ b/test/unittest/dif/tst.DisVarNames.r
> @@ -0,0 +1,23 @@
> +CheckVariable_Z
> +CheckVariable_Y
> +CheckVariable_X
> +CheckVariable_A
> +CheckVariable_A
> +CheckVariable_A
> +CheckVariable_A
> +CheckVariable_B
> +CheckVariable_A
> +CheckVariable_C
> +CheckVariable_Z
> +this->CheckVariable_z
> +this->CheckVariable_y
> +this->CheckVariable_x
> +CheckVariable_C
> +this->CheckVariable_c
> +this->CheckVariable_b
> +this->CheckVariable_a
> +this->CheckVariable_b
> +CheckVariable_C
> +this->CheckVariable_x
> +CheckVariable_Y
> +this->CheckVariable_z
> diff --git a/test/unittest/dif/tst.DisVarNames.sh b/test/unittest/dif/tst.DisVarNames.sh
> new file mode 100755
> index 00000000..68a827b6
> --- /dev/null
> +++ b/test/unittest/dif/tst.DisVarNames.sh
> @@ -0,0 +1,53 @@
> +#!/bin/bash
> +#
> +# 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.
> +
> +# ASSERTION:  That dtrace -S recognizes global and local variable names.
> +
> +dtrace=$1
> +
> +# generate disassembly and print the emitted variable names
> +$dtrace $dt_flags -Sen '
> +struct foo {
> +    int foo_i, foo_j, foo_k;
> +} CheckVariable_A,
> +  CheckVariable_B,
> +  CheckVariable_C;
> +
> +this struct foo
> +  CheckVariable_a,
> +  CheckVariable_b,
> +  CheckVariable_c;
> +
> +BEGIN {
> +    CheckVariable_X =
> +    CheckVariable_Y =
> +    CheckVariable_Z = 1;
> +
> +    CheckVariable_A.foo_i =
> +    CheckVariable_A.foo_j =
> +    CheckVariable_A.foo_k = 1;
> +
> +    CheckVariable_B =
> +    CheckVariable_A;
> +
> +    CheckVariable_C =
> +    CheckVariable_A;
> +
> +    this->CheckVariable_x =
> +    this->CheckVariable_y =
> +    this->CheckVariable_z = CheckVariable_Z;
> +
> +    this->CheckVariable_a =
> +    this->CheckVariable_b =
> +    this->CheckVariable_c = CheckVariable_C;
> +
> +    trace(this->CheckVariable_b.foo_j);
> +    trace(      CheckVariable_C.foo_k);
> +    trace(this->CheckVariable_x);
> +    trace(      CheckVariable_Y);
> +    trace(this->CheckVariable_z);
> +}' |& awk '/ ! (|this->)CheckVariable_/ { print $NF }'
> -- 
> 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