[DTrace-devel] [PATCH v2 dtrace 1/3] fbt: support "."-suffixed functions for kprobes

[Kris Van Hees]

On Fri, Oct 11, 2024 at 11:28:08AM +0100, Alan Maguire wrote:
> gcc adds suffixes when it carries out optimizations, but often
> these leave parameters to functions intact.  Many of these functions
> (like finish_task_switch()) are important for tracing (and they
> are present in available_filter_functions so are traceable) so it is
> valuable to support probing them.  For kprobes, all that is needed
> is to ensure that the event name does not contain a ".".

The challenge here is that not everything that is present in
available_filter_functions is actually traceable.  I.e. anything with a
compiler-optimization-generated name (*.*) cannot be traced with fprobes.
Also, at least with older kernels, my testing has shown that not all that
is listed in that file can even be traced with kprobes (probably due to a
kernel bug that has since been fixed).

This leaves us with an interesting challenge.  Since we prefer fprobes
because they are faster (and give us argument type info), using the content
of that file indiscriminately is not a good idea because it will cause us
to list probes that cannot be used.  On the other hand, if probes with "."
in their names *can* be probed using kprobe, then it would certainly be
beneficial to make use of that.

Now, since the .-names are compiler generated we do not necessarily need to
list those as probes.  We could just list the base function *if* we can
determine (for FBT) which suffixes refer to variants that actually constitute
a valid alternative (where function entry and return are valid representations
of the original function).

> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
> ---
>  libdtrace/dt_prov_fbt.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 21f63ddf..0ba0fb71 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -106,10 +106,6 @@ static int populate(dtrace_hdl_t *dtp)
>  				p++;
>  		}
>  
> -		/* Weed out synthetic symbol names (that are invalid). */
> -		if (strchr(buf, '.') != NULL)
> -			continue;
> -
>  #define strstarts(var, x) (strncmp(var, x, strlen (x)) == 0)
>  		/* Weed out __ftrace_invalid_address___* entries. */
>  		if (strstarts(buf, "__ftrace_invalid_address__") ||
> @@ -363,7 +359,7 @@ static int kprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>  {
>  	if (!dt_tp_probe_has_info(prp)) {
> -		char	*fn;
> +		char	*fn, *event, *s;
>  		FILE	*f;
>  		size_t	len;
>  		int	fd, rc = -1;
> @@ -376,22 +372,37 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>  		if (fd == -1)
>  			return -ENOENT;
>  
> +		if (asprintf(&event, "%s", prp->desc->fun) < 0)
> +			return -ENOMEM;
> +		/* replace possible "." in event name as "." is not supported
> +		 * in trace event names, and function could be named foo.isra.0
> +		 */
> +		for (s = strchr(event, '.'); s != NULL && *s != '\0'; s++) {
> +			if (*s == '.')
> +				*s = '_';
> +		}
> +
>  		rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n",
>  			     prp->desc->prb[0] == 'e' ? 'p' : 'r',
> -			     FBT_GROUP_DATA, prp->desc->fun, prp->desc->fun);
> +			     FBT_GROUP_DATA, event, prp->desc->fun);
>  		close(fd);
> -		if (rc == -1)
> +		if (rc == -1) {
> +			free(event);
>  			return -ENOENT;
> +		}
>  
>  		/* create format file name */
>  		len = snprintf(NULL, 0, "%s" FBT_GROUP_FMT "/%s/format",
> -			       EVENTSFS, FBT_GROUP_DATA, prp->desc->fun) + 1;
> +			       EVENTSFS, FBT_GROUP_DATA, event) + 1;
>  		fn = dt_alloc(dtp, len);
> -		if (fn == NULL)
> +		if (fn == NULL) {
> +			free(event);
>  			return -ENOENT;
> +		}
>  
>  		snprintf(fn, len, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS,
> -			 FBT_GROUP_DATA, prp->desc->fun);
> +			 FBT_GROUP_DATA, event);
> +		free(event);
>  
>  		/* open format file */
>  		f = fopen(fn, "r");
> -- 
> 2.43.5
>