[DTrace-devel] [PATCH 05/10] dtrace provider: don't use functions from uprobes.h any more

[Kris Van Hees]

On Wed, Aug 02, 2023 at 02:26:55PM +0100, Nick Alcock via DTrace-devel wrote:
> This is about to go away, so the justification for using it (to make sure it
> was frequently used and didn't rust) no longer applies.
> 
> This more or less reverts 5b43ff17f839195a29cadf255a947514d15aa478.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>

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

... with required change below.

> ---
>  libdtrace/dt_prov_dtrace.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index 474274558be3..a5c07733479a 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -16,7 +16,6 @@
>  #include "dt_cg.h"
>  #include "dt_provider_tp.h"
>  #include "dt_probe.h"
> -#include "uprobes.h"
>  
>  static const char		prvname[] = "dtrace";
>  static const char		modname[] = "";
> @@ -172,8 +171,9 @@ static char *uprobe_spec(const char *prb)
>  	struct ps_prochandle	*P;
>  	int			perr = 0;
>  	char			*fun;
> -	char			*spec = NULL;
>  	GElf_Sym		sym;
> +	prsyminfo_t		si;
> +	char			*spec = NULL;
>  
>  	if (asprintf(&fun, "%s%s", prb, PROBE_FUNC_SUFFIX) < 0)
>  		return NULL;
> @@ -185,10 +185,22 @@ static char *uprobe_spec(const char *prb)
>  		return NULL;
>  	}
>  
> -	/* look up function and thus addr */
> -	if (Pxlookup_by_name(P, -1, PR_OBJ_EVERY, fun, &sym, NULL) == 0)
> -		spec = uprobe_spec_by_addr(P, sym.st_value, NULL);
> +	/* look up function, get the map, and record */
> +	if (Pxlookup_by_name(P, -1, PR_OBJ_EVERY, fun, &sym, &si) == 0) {
> +		const prmap_t	*mapp;
>  
> +		mapp = Paddr_to_map(P, sym.st_value);
> +		if (mapp == NULL)
> +			goto out;
> +
> +		if (mapp->pr_file->first_segment != mapp)
> +			mapp = mapp->pr_file->first_segment;
> +
> +		asprintf(&spec, "%s:0x%lx", mapp->pr_file->prf_mapname,
> +			 sym.st_value - mapp->pr_vaddr);

One problem is that asprintf() allocates memory for spec and that is done
using the libc allocation routines and not dt_alloc(), and so it is not
really correct that it gets freed in attach() using dt_free().  Right now,
in practical sense, that ends up being the same thing but it would be wrong
if dt_alloc() / dt_free() ever gets implemented to use alternative routines.

To avoid possible future issues, it should be freed using free().

> +	}
> +
> +out:
>  	free(fun);
>  	Prelease(P, PS_RELEASE_NORMAL);
>  	Pfree(P);
> -- 
> 2.41.0.270.g68fa1d84b5
> 
>