Re: [PATCH v3 6/6] Add support for pid function "-" with absolute offset

[Kris Van Hees <kris.van.hees@oracle.com>]

On Thu, Feb 20, 2025 at 07:08:31PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The pid providers allow users to specify a probe function "-",
> meaning that the probe name gives an absolute offset.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

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

> ---
>  include/dtrace/pid.h                |   3 +-
>  libdtrace/dt_pid.c                  |  79 ++++++++++++------
>  libdtrace/dt_prov_uprobe.c          |  38 ++++++---
>  test/unittest/pid/tst.dash.r        |   1 +
>  test/unittest/pid/tst.dash.sh       | 119 ++++++++++++++++++++++++++++
>  test/unittest/usdt/tst.pidprobes.sh |  21 ++++-
>  6 files changed, 224 insertions(+), 37 deletions(-)
>  create mode 100644 test/unittest/pid/tst.dash.r
>  create mode 100755 test/unittest/pid/tst.dash.sh
> 
> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
> index c53e60047..12934500a 100644
> --- a/include/dtrace/pid.h
> +++ b/include/dtrace/pid.h
> @@ -2,7 +2,7 @@
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   *
> - * Copyright (c) 2009, 2024, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved.
>   */
>  
>  /*
> @@ -22,6 +22,7 @@ typedef enum pid_probetype {
>  	DTPPT_ENTRY,
>  	DTPPT_RETURN,
>  	DTPPT_OFFSETS,
> +	DTPPT_ABSOFFSETS,
>  	DTPPT_USDT,
>  	DTPPT_IS_ENABLED
>  } pid_probetype_t;
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index 11b964561..76608f690 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2010, 2025, 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.
>   */
> @@ -155,7 +155,6 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  	uint_t nmatches = 0;
>  	ulong_t sz;
>  	int glob, rc = 0;
> -	int isdash = strcmp("-", func) == 0;
>  	pid_t pid;
>  
>  	/*
> @@ -183,7 +182,46 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  	psp->pps_nameoff = 0;
>  	psp->pps_off = symp->st_value - pp->dpp_vaddr;
>  
> -	if (!isdash && gmatch("return", pp->dpp_name)) {
> +	/*
> +	 * The special function "-" means the probe name is an absolute
> +	 * virtual address.
> +	 */
> +	if (strcmp("-", func) == 0) {
> +		char *end;
> +		GElf_Sym sym;
> +
> +		off = strtoull(pp->dpp_name, &end, 16);
> +		if (*end != '\0') {
> +			rc = dt_pid_error(dtp, pcb, dpr, D_PROC_NAME,
> +					  "'%s' is an invalid probe name",
> +					  pp->dpp_name);
> +			goto out;
> +		}
> +
> +		psp->pps_nameoff = off;
> +
> +		if (dt_Plookup_by_addr(dtp, pid, off, (const char **)&psp->pps_fun, &sym)) {
> +			rc = dt_pid_error(dtp, pcb, dpr, D_PROC_NAME,
> +			     "failed to lookup 0x%lx in module '%s'", off, pp->dpp_mod);
> +			if (psp->pps_fun != func && psp->pps_fun != NULL)
> +				free(psp->pps_fun);
> +			goto out;
> +		}
> +
> +		psp->pps_prb = (char*)pp->dpp_name;
> +		psp->pps_off = off - pp->dpp_vaddr;
> +
> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_ABSOFFSETS) < 0)
> +			rc = dt_pid_error(dtp, pcb, dpr, D_PROC_CREATEFAIL,
> +			    "failed to create probes at '%s+0x%llx': %s",
> +			    func, (unsigned long long)off, dtrace_errmsg(dtp, dtrace_errno(dtp)));
> +		else
> +			pp->dpp_nmatches++;
> +		free(psp->pps_fun);
> +		goto out;
> +	}
> +
> +	if (gmatch("return", pp->dpp_name)) {
>  		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_RETURN) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
> @@ -195,7 +233,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  		nmatches++;
>  	}
>  
> -	if (!isdash && gmatch("entry", pp->dpp_name)) {
> +	if (gmatch("entry", pp->dpp_name)) {
>  		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_ENTRY) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
> @@ -240,7 +278,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  		}
>  
>  		nmatches++;
> -	} else if (glob && !isdash) {
> +	} else if (glob) {
>  #if defined(__amd64)
>  		/*
>  		 * We need to step through the instructions to find their
> @@ -449,31 +487,25 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
>  	else
>  		pp->dpp_obj++;
>  
> +	/*
> +	 * If it is the special function "-", cut to dt_pid_per_sym() now.
> +	 */
> +	if (strcmp("-", pp->dpp_func) == 0)
> +		return dt_pid_per_sym(pp, &sym, pp->dpp_func);
> +
>  	/*
>  	 * If pp->dpp_func contains any globbing meta-characters, we need
>  	 * to iterate over the symbol table and compare each function name
>  	 * against the pattern.
>  	 */
>  	if (!strisglob(pp->dpp_func)) {
> -		/*
> -		 * If we fail to lookup the symbol, try interpreting the
> -		 * function as the special "-" function that indicates that the
> -		 * probe name should be interpreted as a absolute virtual
> -		 * address. If that fails and we were matching a specific
> -		 * function in a specific module, report the error, otherwise
> -		 * just fail silently in the hopes that some other object will
> -		 * contain the desired symbol.
> +		/* If we are matching a specific function in a specific module,
> +		 * report the error, otherwise just fail silently in the hopes
> +		 * that some other object will contain the desired symbol.
>  		 */
>  		if (dt_Pxlookup_by_name(dtp, pid, pp->dpp_lmid, obj,
>  					pp->dpp_func, &sym, NULL) != 0) {
> -			if (strcmp("-", pp->dpp_func) == 0) {
> -				sym.st_name = 0;
> -				sym.st_info =
> -				    GELF_ST_INFO(STB_LOCAL, STT_FUNC);
> -				sym.st_other = 0;
> -				sym.st_value = 0;
> -				sym.st_size = Pelf64(pp->dpp_pr) ? -1ULL : -1U;
> -			} else if (!strisglob(pp->dpp_mod)) {
> +			if (!strisglob(pp->dpp_mod)) {
>  				return dt_pid_error(
>  					dtp, pcb, dpr, D_PROC_FUNC,
>  					"failed to lookup '%s' in module '%s'",
> @@ -647,9 +679,10 @@ dt_pid_create_pid_probes_proc(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp,
>  	if (strcmp(pp.dpp_func, "-") == 0) {
>  		const prmap_t *aout, *pmp;
>  
> -		if (pdp->mod[0] == '\0') {
> -			pp.dpp_mod = pdp->mod;
> +		if (strcmp(pp.dpp_mod, "*") == 0) {
> +			/* Tolerate two glob cases:  "" and "*". */
>  			pdp->mod = "a.out";
> +			pp.dpp_mod = pdp->mod;
>  		} else if (strisglob(pp.dpp_mod) ||
>  		    (aout = dt_Pname_to_map(dtp, pid, "a.out")) == NULL ||
>  		    (pmp = dt_Pname_to_map(dtp, pid, pp.dpp_mod)) == NULL ||
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 11d595898..e96479963 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2021, 2025, 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.
>   *
> @@ -55,7 +55,8 @@ static const char	prvname[] = "uprobe";
>  typedef struct dt_uprobe {
>  	dev_t		dev;
>  	ino_t		inum;
> -	char		*fn;
> +	char		*fn;		   /* object full file name */
> +	char		*func;		   /* function */
>  	uint64_t	off;
>  	int		flags;
>  	tp_probe_t	*tp;
> @@ -128,6 +129,7 @@ static void probe_destroy_underlying(dtrace_hdl_t *dtp, void *datap)
>  	dt_tp_destroy(dtp, tpp);
>  	free_probe_list(dtp, dt_list_next(&upp->probes));
>  	dt_free(dtp, upp->fn);
> +	dt_free(dtp, upp->func);
>  	dt_free(dtp, upp->args);
>  	dt_free(dtp, upp->argvbuf);
>  	dt_free(dtp, upp);
> @@ -333,9 +335,13 @@ ignore_clause(dtrace_hdl_t *dtp, int n, const dt_probe_t *uprp)
>  	 */
>  
>  	/* We know what function we're in.  It must match the probe description (unless "-"). */
> -	if (strcmp(pdp->fun, "-") != 0 &&
> -	    !dt_gmatch(uprp->desc->fun, pdp->fun))
> -		return 1;
> +	if (strcmp(pdp->fun, "-") != 0) {
> +		dt_uprobe_t	*upp = uprp->prv_data;
> +
> +		assert(upp->func);  // never a return probe
> +		if (!dt_gmatch(upp->func, pdp->fun))
> +			return 1;
> +	}
>  
>  	return 0;
>  }
> @@ -623,11 +629,11 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>  	 *
>  	 * The probe description for non-return probes is:
>  	 *
> -	 *	uprobe:<dev>_<inode>:<func>:<offset>
> +	 *	uprobe:<dev>_<inode>::<offset>
>  	 *
>  	 * The probe description for return probes is:
>  	 *
> -	 *	uprobe:<dev>_<inode>:<func>:return
> +	 *	uprobe:<dev>_<inode>::return
>  	 */
>  	snprintf(mod, sizeof(mod), "%lx_%lx", psp->pps_dev, psp->pps_inum);
>  
> @@ -638,6 +644,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>  	case DTPPT_IS_ENABLED:
>  	case DTPPT_ENTRY:
>  	case DTPPT_OFFSETS:
> +	case DTPPT_ABSOFFSETS:
>  	case DTPPT_USDT:
>  		snprintf(prb, sizeof(prb), "%lx", psp->pps_off);
>  		break;
> @@ -649,7 +656,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>  	pd.id = DTRACE_IDNONE;
>  	pd.prv = prvname;
>  	pd.mod = mod;
> -	pd.fun = psp->pps_fun;
> +	pd.fun = "";
>  	pd.prb = prb;
>  
>  	dt_dprintf("Providing underlying probe %s:%s:%s:%s @ %lx\n", psp->pps_prv,
> @@ -672,6 +679,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>  		upp->inum = psp->pps_inum;
>  		upp->off = psp->pps_off;
>  		upp->fn = strdup(psp->pps_fn);
> +		upp->func = NULL;
>  		upp->tp = dt_tp_alloc(dtp);
>  		if (upp->tp == NULL)
>  			goto fail;
> @@ -692,6 +700,17 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>  		goto fail;
>  	}
>  
> +	/*
> +	 * The underlying probe should have the same function for all
> +	 * overlying probes unless it's a return probe.
> +	 */
> +	if (psp->pps_type != DTPPT_RETURN) {
> +		if (upp->func == NULL)
> +			upp->func = strdup(psp->pps_fun);
> +		else
> +			assert(strcmp(upp->func, psp->pps_fun) == 0);
> +	}
> +
>  	if (populate_args(dtp, psp, upp) < 0)
>  		goto fail;
>  
> @@ -735,7 +754,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>  	pd.id = DTRACE_IDNONE;
>  	pd.prv = prv;
>  	pd.mod = psp->pps_mod;
> -	pd.fun = psp->pps_fun;
> +	pd.fun = (psp->pps_type == DTPPT_ABSOFFSETS) ? "-" : psp->pps_fun;
>  	pd.prb = prb;
>  
>  	/* Get (or create) the provider for the PID of the probe. */
> @@ -823,6 +842,7 @@ static int provide_pid_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp)
>  		strcpy(prb, "return");
>  		break;
>  	case DTPPT_OFFSETS:
> +	case DTPPT_ABSOFFSETS:
>  		snprintf(prb, sizeof(prb), "%lx", psp->pps_nameoff);
>  		break;
>  	default:
> diff --git a/test/unittest/pid/tst.dash.r b/test/unittest/pid/tst.dash.r
> new file mode 100644
> index 000000000..2e9ba477f
> --- /dev/null
> +++ b/test/unittest/pid/tst.dash.r
> @@ -0,0 +1 @@
> +success
> diff --git a/test/unittest/pid/tst.dash.sh b/test/unittest/pid/tst.dash.sh
> new file mode 100755
> index 000000000..f364e3e3c
> --- /dev/null
> +++ b/test/unittest/pid/tst.dash.sh
> @@ -0,0 +1,119 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2025, 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.
> +#
> +# @@timeout: 80
> +
> +dtrace=$1
> +
> +DIRNAME=$tmpdir/pid-dash.$$.$RANDOM
> +mkdir $DIRNAME
> +cd $DIRNAME
> +
> +# Make trigger program.
> +
> +cat << EOF > main.c
> +int foo0(int i) {
> +    int j, k;
> +
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +
> +    return i;
> +}
> +int foo1(int i) {
> +    int j, k;
> +
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +
> +    return i;
> +}
> +int foo2(int i) {
> +    int j, k;
> +
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +    j = i ^ 1; k = j ^ 1; i = k ^ 1;
> +
> +    return i;
> +}
> +int main(int c, char **v) {
> +    int i = 0;
> +
> +    i = foo0(i) ^ i;
> +    i = foo1(i) ^ i;
> +    i = foo2(i) ^ i;
> +
> +    return i;
> +}
> +EOF
> +
> +gcc main.c
> +if [ $? -ne 0 ]; then
> +	echo ERROR compile
> +	exit 1
> +fi
> +
> +# Loop over functions in the program.
> +
> +for func in foo0 foo1 foo2 main; do
> +	# For each function, get the absolute and relative
> +	# (to the function) address of some instruction in
> +	# the function.
> +	read ABS REL <<< `objdump -d a.out | awk '
> +	  # Look for the function.
> +	  /^[0-9a-f]* <'$func'>:$/ {
> +
> +	  # Get the first instruction and note the base address of the function.
> +	  getline; sub(":", ""); base = strtonum("0x"$1);
> +
> +	  # Get the next instruction.
> +	  getline;
> +
> +	  # Get the next instruction.  Note its PC.
> +	  getline; sub(":", ""); pc = strtonum("0x"$1);
> +
> +	  # Print the address, both absolute and relative.
> +	  printf("%x %x\n", pc, pc - base);
> +	  exit(0);
> +	  }'`
> +
> +	# Write the expected output to the compare file.
> +	echo got $ABS $func:$REL >> dtrace.exp
> +	echo got $ABS "-":$ABS   >> dtrace.exp
> +
> +	# Write the actual dtrace output to the output file.
> +	# Specify the pid probe with both relative and absolute
> +	# forms.
> +	for probe in $func:$REL "-:$ABS"; do
> +		$dtrace -c ./a.out -o dtrace.out -qn '
> +		    pid$target:a.out:'$probe'
> +		    { printf("got %x %s:%s", uregs[R_PC], probefunc,
> +							  probename); }'
> +		if [ $? -ne 0 ]; then
> +			echo ERROR: dtrace
> +			cat dtrace.out
> +			exit 1
> +		fi
> +	done
> +done
> +
> +# Check results.
> +
> +if ! diff -q dtrace.exp dtrace.out; then
> +	echo ERROR: 
> +	echo "==== expected"
> +	cat dtrace.exp
> +	echo "==== actual"
> +	cat dtrace.out
> +	exit 1
> +fi
> +
> +echo success
> +exit 0
> diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/tst.pidprobes.sh
> index 54444d49b..0c75d7967 100755
> --- a/test/unittest/usdt/tst.pidprobes.sh
> +++ b/test/unittest/usdt/tst.pidprobes.sh
> @@ -1,7 +1,7 @@
>  #!/bin/bash
>  #
>  # Oracle Linux DTrace.
> -# Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2024, 2025, 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.
>  #
> @@ -121,7 +121,13 @@ fi
>  pcs=`awk '{print strtonum("0x"$1)}' disasm_foo.txt`
>  pc0=`echo $pcs | awk '{print $1}'`
>  
> -# Run dtrace.
> +# Construct D script:  add a pid$pid::-:$absoff probe for each PC in foo.
> +
> +for pc in $pcs; do
> +	printf 'p*d$target::-:%x,\n' $pc >> pidprobes.d
> +done
> +
> +# Construct D script:  add a glob for all pid and USDT pyramid probes in foo.
>  
>  cat >> pidprobes.d <<'EOF'
>  p*d$target::foo:
> @@ -130,6 +136,8 @@ p*d$target::foo:
>  }
>  EOF
>  
> +# Construct D script:  add a glob for all USDT pyramid probes, dumping args.
> +
>  if [[ -n $usdt ]]; then
>  	echo 'pyramid$target::foo: {' >> pidprobes.d
>  
> @@ -141,9 +149,12 @@ if [[ -n $usdt ]]; then
>  	echo '}' >> pidprobes.d
>  fi
>  
> +# Run dtrace.
> +
>  $dtrace $dt_flags -q -c ./main -o dtrace.out -s pidprobes.d > main.out2
>  if [ $? -ne 0 ]; then
>  	echo "failed to run dtrace" >&2
> +	cat pidprobes.d
>  	cat main.out2
>  	cat dtrace.out
>  	exit 1
> @@ -286,14 +297,16 @@ fi
>  # - a blank line
>  # - pid entry
>  # - pid return
> -# - pid offset
> +# - pid offset (relative -- that is, pid$pid:main:foo:$reloff)
> +# - pid offset (absolute -- that is, pid$pid:main:-:$absoff)
>  # - two USDT probes (ignore is-enabled probes)
>  
>  echo > dtrace.out.expected
> -printf "$pid pid$pid:main:foo:entry %x\n" $pc0 >> dtrace.out.expected
> +printf "$pid pid$pid:main:foo:entry %x\n" $pc0   >> dtrace.out.expected
>  echo   "$pid pid$pid:main:foo:return $pc_return" >> dtrace.out.expected
>  for pc in $pcs; do
>  	printf "$pid pid$pid:main:foo:%x %x\n" $(($pc - $pc0)) $pc >> dtrace.out.expected
> +	printf "$pid pid$pid:main:-:%x %x\n"      $pc          $pc >> dtrace.out.expected
>  done
>  echo $usdt_pcs | awk '{printf("'$pid' pyramid'$pid':main:foo:entry %x\n", $1);}' >> dtrace.out.expected
>  echo $usdt_pcs | awk '{printf("'$pid' pyramid'$pid':main:foo:entry %x\n", $2);}' >> dtrace.out.expected
> -- 
> 2.43.5
>