[DTrace-devel] [PATCH] Fix annotation for dt_get_tvar()

[Eugene Loh]

On 12/17/21 1:16 AM, Kris Van Hees via DTrace-devel wrote:

> The disassembler generates the annotation for dt_get_tvar() based on the
> wrong instruction.  It looks at the immediate value stored in the
> (pc - 2) instruction whereas it should be looking at the one in pc - 3).
Missing a '(' before "pc - 3"?  But I think the body of the commit 
message might be superfluous.  It's clear from the patch what's going on.

Meanwhile, the block comment preceding the code change appears to need 
updating.  How about:
                  * We know that the previous two instruction exist and 
assigns
s/two/three/
s/instruction/instructions/
s/assigns/assign/

Finally, if the patch fixes a bug, there should be a test that 
demonstrates the fix.  E.g., (stripped-down version for illustration; 
needs to be turned into .d .r .r.p for test suite; plus versions for 
gvar/lvar):
# dtrace -Sn 'BEGIN { self->foo = 1234; trace(self->foo); exit(0) }' |& 
awk '/foo$/ {print $NF}'
self->foo
self->foo
# dtrace -Sn 'BEGIN { this->foo = 1234; trace(this->foo); exit(0) }' |& 
awk '/foo$/ {print $NF}'
this->foo
this->foo
# dtrace -Sn 'BEGIN { foo = 1234; trace(foo); exit(0) }' |& awk '/foo$/ 
{print $NF}'
foo
foo

> The generated sequence for calling dt_get_tvar is:
>
>      (pc - 3)	mov %r1, <val>
>      (pc - 2)	mov %r2, <val>
>      (pc - 1)	mov %r3, <val>
>      (pc)	call dt_get_tvar
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_dis.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index db8989a3..fc479274 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -312,7 +312,7 @@ dt_dis_bpf_args(const dtrace_difo_t *dp, const char *fn,
>   		 * the variable id to %r1 (because we wrote the code generator
>   		 * to emit these instructions in this exact order.)
>   		 */
> -		in -= 2;
> +		in -= 3;
>   		snprintf(buf, len, "self->%s",
>   			 dt_dis_varname_id(dp, in->imm + DIF_VAR_OTHER_UBASE,
>   					DIFV_SCOPE_THREAD, addr));