[DTrace-devel] [PATCH v2] Add support for built-in variable execname

[Kris Van Hees]

On Thu, Jan 27, 2022 at 03:27:31PM -0500, Eugene Loh via DTrace-devel wrote:
> You can add my
>           Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> to Nick's if you like.  I'm curious about two things:
> 
> On 1/27/22 2:11 PM, Kris Van Hees via DTrace-devel wrote:
> > diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> > +	case DIF_VAR_EXECNAME: {
> > +		uint64_t	ptr;
> > +		uint32_t	key;
> > +		uint32_t	*comm_off;
> > +
> > +		/*
> > +		 * In the "state" map, look up the "struct task_struct" offset
> > +		 * of "comm".
> > +		 */
> > +		key = DT_STATE_TASK_COMM_OFF;
> > +		comm_off = bpf_map_lookup_elem(&state, &key);
> > +		if (comm_off == NULL)
> > +			return 0;
> > +
> > +		/* &(current->comm) */
> > +		ptr = bpf_get_current_task();
> > +		if (ptr == 0)
> > +			return error(dctx, DTRACEFLT_BADADDR, ptr);
> > +
> > +		return (uint64_t)ptr + *comm_off;
> > +	}
> 
> How are the error behaviors determined?  That is, why "return 0" in one case
> and "return error(BADADDR)" in the other?

The idea was that they are two distinct different conditions, but thinking
about it more, it would be better to use return error(dctx, DTRACEFLT_ILLOP, 0);
when comm_off is NULL.  It would make sense to update the similar cases in the
PPID support as well, but not in this patch.

> > diff --git a/test/unittest/variables/bvar/tst.execname.d b/test/unittest/variables/bvar/tst.execname.d
> > @@ -1,10 +1,9 @@
> >   /*
> >    * Oracle Linux DTrace.
> > - * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
> > + * Copyright (c) 2020, 2022, 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.
> >    */
> > -/* @@xfail: dtv2 */
> >   /*
> >    * ASSERTION: The 'execname' variable can be accessed and is not -1.
> 
> The comment describing the test should be updated to reflect the change in
> the test's behavior.

Yes.

> 
> > @@ -16,7 +15,7 @@
> >   BEGIN {
> >   	trace(execname);
> > -	exit(execname != -1 ? 0 : 1);
> > +	exit(0);
> >   }
> >   ERROR {
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel