[PATCH 1/6] Remove unused dpp_pc and dpp_size

[PATCH 1/6] Remove unused dpp_pc and dpp_size

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_pid.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index fd94a0706..2331d1aa7 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -53,9 +53,7 @@ typedef struct dt_pid_probe {
 	dev_t dpp_dev;
 	ino_t dpp_inum;
 	const char *dpp_fname;
-	uintptr_t dpp_pc;
 	uintptr_t dpp_vaddr;
-	size_t dpp_size;
 	Lmid_t dpp_lmid;
 	uint_t dpp_nmatches;
 	GElf_Sym dpp_last;
-- 
2.43.5

[PATCH 2/6] Use prb instead of psp.pps_prb

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

Replace psp.pps_prb with prb, both because it is simpler and
more importantly psp.pps_prb has not yet been assigned!

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_pid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 2331d1aa7..27cd7d13f 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -995,7 +995,7 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 					     "Cannot get name of mapping containing "
 					     "%sprobe %s for pid %d\n",
 					     tp->tracepoint.is_enabled ? "is-enabled ": "",
-					     psp.pps_prb, dpr->dpr_pid);
+					     prb, dpr->dpr_pid);
 				goto oom;
 			}
 
-- 
2.43.5

[PATCH 3/6] Simplify references to dtp

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_pid.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 27cd7d13f..e57478450 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -231,7 +231,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 
 		psp->pps_nameoff = off;
 		psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
-		if (dt_pid_create_one_probe(pp->dpp_pr, pp->dpp_dtp,
+		if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
 					psp, symp, DTPPT_OFFSETS) < 0) {
 			rc = dt_pid_error(
 				dtp, pcb, dpr, D_PROC_CREATEFAIL,
@@ -362,7 +362,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 
 			psp->pps_nameoff = off;
 			psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
-			if (dt_pid_create_one_probe(pp->dpp_pr, pp->dpp_dtp,
+			if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
 						psp, symp, DTPPT_OFFSETS) >= 0)
 				nmatches++;
 		}
@@ -434,7 +434,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
 	if (obj == NULL)
 		return 0;
 
-	dt_Plmid(pp->dpp_dtp, pid, pmp->pr_vaddr, &pp->dpp_lmid);
+	dt_Plmid(dtp, pid, pmp->pr_vaddr, &pp->dpp_lmid);
 
 	pp->dpp_dev = pmp->pr_dev;
 	pp->dpp_inum = pmp->pr_inum;
@@ -466,7 +466,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
 		 * just fail silently in the hopes that some other object will
 		 * contain the desired symbol.
 		 */
-		if (dt_Pxlookup_by_name(pp->dpp_dtp, pid, pp->dpp_lmid, obj,
+		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;
@@ -496,17 +496,17 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
 		 * dynamically rewritten, and, so, inherently dicey to
 		 * instrument.
 		 */
-		if (dt_Pwritable_mapping(pp->dpp_dtp, pid, sym.st_value))
+		if (dt_Pwritable_mapping(dtp, pid, sym.st_value))
 			return 0;
 // FIXME:  why are we changing to a spelling that differs from what the user asked for?
-		dt_Plookup_by_addr(pp->dpp_dtp, pid, sym.st_value,
+		dt_Plookup_by_addr(dtp, pid, sym.st_value,
 				   &pp->dpp_func, &sym);
 
 		return dt_pid_per_sym(pp, &sym, pp->dpp_func);
 	} else {
 		uint_t nmatches = pp->dpp_nmatches;
 
-		if (dt_Psymbol_iter_by_addr(pp->dpp_dtp, pid, obj, PR_SYMTAB,
+		if (dt_Psymbol_iter_by_addr(dtp, pid, obj, PR_SYMTAB,
 					    BIND_ANY | TYPE_FUNC,
 					    dt_pid_sym_filt, pp) == 1)
 			return 1;
@@ -516,8 +516,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
 			 * If we didn't match anything in the PR_SYMTAB, try
 			 * the PR_DYNSYM.
 			 */
-			if (dt_Psymbol_iter_by_addr(
-					pp->dpp_dtp, pid, obj,
+			if (dt_Psymbol_iter_by_addr(dtp, pid, obj,
 					PR_DYNSYM, BIND_ANY | TYPE_FUNC,
 					dt_pid_sym_filt, pp) == 1)
 				return 1;
-- 
2.43.5

[PATCH 4/6] Remove unused function arg

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_pid.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index e57478450..b8bbb0396 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -104,7 +104,7 @@ dt_pid_error(dtrace_hdl_t *dtp, dt_pcb_t *pcb, dt_proc_t *dpr,
 
 static int
 dt_pid_create_one_probe(struct ps_prochandle *P, dtrace_hdl_t *dtp,
-    pid_probespec_t *psp, const GElf_Sym *symp, pid_probetype_t type)
+    pid_probespec_t *psp, pid_probetype_t type)
 {
 	const dt_provider_t	*pvp = dtp->dt_prov_pid;
 
@@ -184,8 +184,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 	psp->pps_off = symp->st_value - pp->dpp_vaddr;
 
 	if (!isdash && gmatch("return", pp->dpp_name)) {
-		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, symp,
-		    DTPPT_RETURN) < 0) {
+		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_RETURN) < 0) {
 			rc = dt_pid_error(
 				dtp, pcb, dpr, D_PROC_CREATEFAIL,
 				"failed to create return probe for '%s': %s",
@@ -197,8 +196,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 	}
 
 	if (!isdash && gmatch("entry", pp->dpp_name)) {
-		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, symp,
-		    DTPPT_ENTRY) < 0) {
+		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_ENTRY) < 0) {
 			rc = dt_pid_error(
 				dtp, pcb, dpr, D_PROC_CREATEFAIL,
 				"failed to create entry probe for '%s': %s",
@@ -232,7 +230,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 		psp->pps_nameoff = off;
 		psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
 		if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
-					psp, symp, DTPPT_OFFSETS) < 0) {
+					psp, DTPPT_OFFSETS) < 0) {
 			rc = dt_pid_error(
 				dtp, pcb, dpr, D_PROC_CREATEFAIL,
 				"failed to create probes at '%s+0x%llx': %s",
@@ -363,7 +361,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 			psp->pps_nameoff = off;
 			psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
 			if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
-						psp, symp, DTPPT_OFFSETS) >= 0)
+						psp, DTPPT_OFFSETS) >= 0)
 				nmatches++;
 		}
 
-- 
2.43.5

[PATCH 5/6] test: Move disassembly and extracting PCs earlier

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

This will allow a future patch to use the PCs earlier in the test.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/usdt/tst.pidprobes.sh | 38 ++++++++++++++---------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/tst.pidprobes.sh
index aa7dd4bb0..54444d49b 100755
--- a/test/unittest/usdt/tst.pidprobes.sh
+++ b/test/unittest/usdt/tst.pidprobes.sh
@@ -102,6 +102,25 @@ if ! diff -q main.out main.out.expected > /dev/null; then
 	exit 1
 fi
 
+# Disassemble foo().  (simplify with --disassemble=foo)
+
+objdump -d main | awk '
+BEGIN { use = 0 }             # start by not printing lines
+use == 1 && NF == 0 { exit }  # if printing lines but hit a blank, then exit
+use == 1 { print }            # print lines
+/<foo>:/ { use = 1 }          # turn on printing when we hit "<foo>:" (without printing this line itself)
+' > disasm_foo.txt
+if [ $? -ne 0 ]; then
+	echo cannot objdump main
+	objdump -d main
+	exit 1
+fi
+
+# From the disassembly, get the PCs for foo()'s instructions.
+
+pcs=`awk '{print strtonum("0x"$1)}' disasm_foo.txt`
+pc0=`echo $pcs | awk '{print $1}'`
+
 # Run dtrace.
 
 cat >> pidprobes.d <<'EOF'
@@ -153,25 +172,6 @@ if [ `awk 'NF != 0 { print $1 }' dtrace.out | uniq | wc -l` -ne 1 ]; then
 fi
 pid=`awk 'NF != 0 { print $1 }' dtrace.out | uniq`
 
-# Disassemble foo().  (simplify with --disassemble=foo)
-
-objdump -d main | awk '
-BEGIN { use = 0 }             # start by not printing lines
-use == 1 && NF == 0 { exit }  # if printing lines but hit a blank, then exit
-use == 1 { print }            # print lines
-/<foo>:/ { use = 1 }          # turn on printing when we hit "<foo>:" (without printing this line itself)
-' > disasm_foo.txt
-if [ $? -ne 0 ]; then
-	echo cannot objdump main
-	objdump -d main
-	exit 1
-fi
-
-# From the disassembly, get the PCs for foo()'s instructions.
-
-pcs=`awk '{print strtonum("0x"$1)}' disasm_foo.txt`
-pc0=`echo $pcs | awk '{print $1}'`
-
 # From the disassembly, get the PCs for USDT probes.
 # Check libdtrace/dt_link.c's arch-dependent dt_modtext() to see
 # what sequence of instructions signal a USDT probe.
-- 
2.43.5

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

[eugene.loh]

From: Eugene Loh <eugene.loh@oracle.com>

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 include/dtrace/pid.h                |   1 +
 libdtrace/dt_pid.c                  |  75 +++++++++++++-----
 libdtrace/dt_prov_uprobe.c          |   2 +-
 test/unittest/pid/tst.dash.r        |   1 +
 test/unittest/pid/tst.dash.sh       | 118 ++++++++++++++++++++++++++++
 test/unittest/usdt/tst.pidprobes.sh |  18 ++++-
 6 files changed, 192 insertions(+), 23 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..ad0aad9d8 100644
--- a/include/dtrace/pid.h
+++ b/include/dtrace/pid.h
@@ -47,6 +47,7 @@ typedef struct pid_probespec {
 	/*
 	 * Fields below this point do not apply to underlying probes.
 	 */
+	int pps_absoff;				/* use "-" for overlying probe function */
 	pid_t pps_pid;				/* task PID */
 	uint64_t pps_nameoff;			/* offset to use for name */
 } pid_probespec_t;
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index b8bbb0396..dae499422 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -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;
 
 	/*
@@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
 	psp->pps_fun = (char *) func;
 	psp->pps_nameoff = 0;
 	psp->pps_off = symp->st_value - pp->dpp_vaddr;
+	psp->pps_absoff = 0;
 
-	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_absoff = 1;
+		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;  // make sure dt_prov_uprobe uses it
+		psp->pps_off = off - sym.st_value;   // make sure dt_prov_uprobe uses it // dump this
+		psp->pps_off = off - pp->dpp_vaddr;  // make sure dt_prov_uprobe uses it
+
+		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_OFFSETS) < 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 +236,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 +281,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 +490,26 @@ 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
+		/* 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 +683,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 acf1c914f..ae4b262ac 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -735,7 +735,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_absoff ? "-" : psp->pps_fun;
 	pd.prb = prb;
 
 	/* Get (or create) the provider for the PID of the probe. */
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..996b79f4f
--- /dev/null
+++ b/test/unittest/pid/tst.dash.sh
@@ -0,0 +1,118 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2024, 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.
+#
+
+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..e3e4e2043 100755
--- a/test/unittest/usdt/tst.pidprobes.sh
+++ b/test/unittest/usdt/tst.pidprobes.sh
@@ -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,6 +149,8 @@ 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
@@ -286,14 +296,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

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

[Kris Van Hees]

A first early comment.  You are changing the existing implementation of this
quite a bit so I am still tracing through it a bit to understand it all :)

Good stuff though.

On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  include/dtrace/pid.h                |   1 +
>  libdtrace/dt_pid.c                  |  75 +++++++++++++-----
>  libdtrace/dt_prov_uprobe.c          |   2 +-
>  test/unittest/pid/tst.dash.r        |   1 +
>  test/unittest/pid/tst.dash.sh       | 118 ++++++++++++++++++++++++++++
>  test/unittest/usdt/tst.pidprobes.sh |  18 ++++-
>  6 files changed, 192 insertions(+), 23 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..ad0aad9d8 100644
> --- a/include/dtrace/pid.h
> +++ b/include/dtrace/pid.h
> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
>  	/*
>  	 * Fields below this point do not apply to underlying probes.
>  	 */
> +	int pps_absoff;				/* use "-" for overlying probe function */

Why use this?  All you do with it is record that func is "-", and then later
in the pid provider, you actually use that to force the function name in the
probe description to be "-".  But that function name in the probe desc is (in
the pid provider) already coming from pps_fun, and the code was already
assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
to serve no purpose, and you could just rely on pps_fun being "-" when needed.

>  	pid_t pps_pid;				/* task PID */
>  	uint64_t pps_nameoff;			/* offset to use for name */
>  } pid_probespec_t;
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index b8bbb0396..dae499422 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -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;
>  
>  	/*
> @@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  	psp->pps_fun = (char *) func;
>  	psp->pps_nameoff = 0;
>  	psp->pps_off = symp->st_value - pp->dpp_vaddr;
> +	psp->pps_absoff = 0;
>  
> -	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_absoff = 1;
> +		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;  // make sure dt_prov_uprobe uses it
> +		psp->pps_off = off - sym.st_value;   // make sure dt_prov_uprobe uses it // dump this
> +		psp->pps_off = off - pp->dpp_vaddr;  // make sure dt_prov_uprobe uses it
> +
> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_OFFSETS) < 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 +236,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 +281,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 +490,26 @@ 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
> +		/* 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 +683,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 acf1c914f..ae4b262ac 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -735,7 +735,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_absoff ? "-" : psp->pps_fun;
>  	pd.prb = prb;
>  
>  	/* Get (or create) the provider for the PID of the probe. */
> 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..996b79f4f
> --- /dev/null
> +++ b/test/unittest/pid/tst.dash.sh
> @@ -0,0 +1,118 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2024, 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.
> +#
> +
> +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..e3e4e2043 100755
> --- a/test/unittest/usdt/tst.pidprobes.sh
> +++ b/test/unittest/usdt/tst.pidprobes.sh
> @@ -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,6 +149,8 @@ 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
> @@ -286,14 +296,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
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

Re: [PATCH 1/6] Remove unused dpp_pc and dpp_size

[Kris Van Hees]

On Fri, Dec 20, 2024 at 05:27:11PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

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

> ---
>  libdtrace/dt_pid.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index fd94a0706..2331d1aa7 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -53,9 +53,7 @@ typedef struct dt_pid_probe {
>  	dev_t dpp_dev;
>  	ino_t dpp_inum;
>  	const char *dpp_fname;
> -	uintptr_t dpp_pc;
>  	uintptr_t dpp_vaddr;
> -	size_t dpp_size;
>  	Lmid_t dpp_lmid;
>  	uint_t dpp_nmatches;
>  	GElf_Sym dpp_last;
> -- 
> 2.43.5
> 

Re: [DTrace-devel] [PATCH 4/6] Remove unused function arg

[Kris Van Hees]

On Fri, Dec 20, 2024 at 05:27:14PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

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

... merging is pending because it depends on earier patches yet to be reviewed.

> ---
>  libdtrace/dt_pid.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index e57478450..b8bbb0396 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -104,7 +104,7 @@ dt_pid_error(dtrace_hdl_t *dtp, dt_pcb_t *pcb, dt_proc_t *dpr,
>  
>  static int
>  dt_pid_create_one_probe(struct ps_prochandle *P, dtrace_hdl_t *dtp,
> -    pid_probespec_t *psp, const GElf_Sym *symp, pid_probetype_t type)
> +    pid_probespec_t *psp, pid_probetype_t type)
>  {
>  	const dt_provider_t	*pvp = dtp->dt_prov_pid;
>  
> @@ -184,8 +184,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  	psp->pps_off = symp->st_value - pp->dpp_vaddr;
>  
>  	if (!isdash && gmatch("return", pp->dpp_name)) {
> -		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, symp,
> -		    DTPPT_RETURN) < 0) {
> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_RETURN) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
>  				"failed to create return probe for '%s': %s",
> @@ -197,8 +196,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  	}
>  
>  	if (!isdash && gmatch("entry", pp->dpp_name)) {
> -		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, symp,
> -		    DTPPT_ENTRY) < 0) {
> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_ENTRY) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
>  				"failed to create entry probe for '%s': %s",
> @@ -232,7 +230,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  		psp->pps_nameoff = off;
>  		psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
>  		if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
> -					psp, symp, DTPPT_OFFSETS) < 0) {
> +					psp, DTPPT_OFFSETS) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
>  				"failed to create probes at '%s+0x%llx': %s",
> @@ -363,7 +361,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  			psp->pps_nameoff = off;
>  			psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
>  			if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
> -						psp, symp, DTPPT_OFFSETS) >= 0)
> +						psp, DTPPT_OFFSETS) >= 0)
>  				nmatches++;
>  		}
>  
> -- 
> 2.43.5
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

Re: [PATCH 3/6] Simplify references to dtp

[Kris Van Hees]

On Fri, Dec 20, 2024 at 05:27:13PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

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

> ---
>  libdtrace/dt_pid.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index 27cd7d13f..e57478450 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -231,7 +231,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  
>  		psp->pps_nameoff = off;
>  		psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
> -		if (dt_pid_create_one_probe(pp->dpp_pr, pp->dpp_dtp,
> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
>  					psp, symp, DTPPT_OFFSETS) < 0) {
>  			rc = dt_pid_error(
>  				dtp, pcb, dpr, D_PROC_CREATEFAIL,
> @@ -362,7 +362,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>  
>  			psp->pps_nameoff = off;
>  			psp->pps_off = symp->st_value - pp->dpp_vaddr + off;
> -			if (dt_pid_create_one_probe(pp->dpp_pr, pp->dpp_dtp,
> +			if (dt_pid_create_one_probe(pp->dpp_pr, dtp,
>  						psp, symp, DTPPT_OFFSETS) >= 0)
>  				nmatches++;
>  		}
> @@ -434,7 +434,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
>  	if (obj == NULL)
>  		return 0;
>  
> -	dt_Plmid(pp->dpp_dtp, pid, pmp->pr_vaddr, &pp->dpp_lmid);
> +	dt_Plmid(dtp, pid, pmp->pr_vaddr, &pp->dpp_lmid);
>  
>  	pp->dpp_dev = pmp->pr_dev;
>  	pp->dpp_inum = pmp->pr_inum;
> @@ -466,7 +466,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
>  		 * just fail silently in the hopes that some other object will
>  		 * contain the desired symbol.
>  		 */
> -		if (dt_Pxlookup_by_name(pp->dpp_dtp, pid, pp->dpp_lmid, obj,
> +		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;
> @@ -496,17 +496,17 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
>  		 * dynamically rewritten, and, so, inherently dicey to
>  		 * instrument.
>  		 */
> -		if (dt_Pwritable_mapping(pp->dpp_dtp, pid, sym.st_value))
> +		if (dt_Pwritable_mapping(dtp, pid, sym.st_value))
>  			return 0;
>  // FIXME:  why are we changing to a spelling that differs from what the user asked for?
> -		dt_Plookup_by_addr(pp->dpp_dtp, pid, sym.st_value,
> +		dt_Plookup_by_addr(dtp, pid, sym.st_value,
>  				   &pp->dpp_func, &sym);
>  
>  		return dt_pid_per_sym(pp, &sym, pp->dpp_func);
>  	} else {
>  		uint_t nmatches = pp->dpp_nmatches;
>  
> -		if (dt_Psymbol_iter_by_addr(pp->dpp_dtp, pid, obj, PR_SYMTAB,
> +		if (dt_Psymbol_iter_by_addr(dtp, pid, obj, PR_SYMTAB,
>  					    BIND_ANY | TYPE_FUNC,
>  					    dt_pid_sym_filt, pp) == 1)
>  			return 1;
> @@ -516,8 +516,7 @@ dt_pid_per_mod(void *arg, const prmap_t *pmp, const char *obj)
>  			 * If we didn't match anything in the PR_SYMTAB, try
>  			 * the PR_DYNSYM.
>  			 */
> -			if (dt_Psymbol_iter_by_addr(
> -					pp->dpp_dtp, pid, obj,
> +			if (dt_Psymbol_iter_by_addr(dtp, pid, obj,
>  					PR_DYNSYM, BIND_ANY | TYPE_FUNC,
>  					dt_pid_sym_filt, pp) == 1)
>  				return 1;
> -- 
> 2.43.5
> 

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

[Eugene Loh]

On 1/9/25 14:12, Kris Van Hees wrote:

> On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
>> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
>> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
>>   	/*
>>   	 * Fields below this point do not apply to underlying probes.
>>   	 */
>> +	int pps_absoff;				/* use "-" for overlying probe function */
> Why use this?  All you do with it is record that func is "-", and then later
> in the pid provider, you actually use that to force the function name in the
> probe description to be "-".  But that function name in the probe desc is (in
> the pid provider) already coming from pps_fun, and the code was already
> assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
> to serve no purpose, and you could just rely on pps_fun being "-" when needed.

Yeah, good point.  Let me see if I can remember/understand/explain 
what's going on here.

In essence, the problem is we have to pass two function names:  "-" (for 
the overlying probe) and the actual function name (for the underlying 
probe);  up until now, they were both the same.  I tossed around 
different ideas for this.  E.g., pass two function names in. Or, get the 
actual function name inside dt_prov_uprobe.  It seemed to make most 
sense (given the alternatives) to pass in the actual function name and 
add this flag to say when the overlying probe's function name should be 
"-".  Does that make sense?

Note that pps_fun is not "-".  Although we set it to func ("-"), the 
code that processes this case then fills pps_fun in with the actual 
function name.  Again, one option is not to do this until 
dt_prov_uprobe.c, but that introduces some other hassles.

>>   	pid_t pps_pid;				/* task PID */
>>   	uint64_t pps_nameoff;			/* offset to use for name */
>>   } pid_probespec_t;
>> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
>> @@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>>   	psp->pps_fun = (char *) func;
>>   	psp->pps_nameoff = 0;
>>   	psp->pps_off = symp->st_value - pp->dpp_vaddr;
>> +	psp->pps_absoff = 0;
>>   
>> -	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);
>> +
>> +		psp->pps_absoff = 1;
>> +		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;  // make sure dt_prov_uprobe uses it
>> +		psp->pps_off = off - sym.st_value;   // make sure dt_prov_uprobe uses it // dump this
>> +		psp->pps_off = off - pp->dpp_vaddr;  // make sure dt_prov_uprobe uses it
>> +
>> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_OFFSETS) < 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,
>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
>> index acf1c914f..ae4b262ac 100644
>> --- a/libdtrace/dt_prov_uprobe.c
>> +++ b/libdtrace/dt_prov_uprobe.c
>> @@ -735,7 +735,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_absoff ? "-" : psp->pps_fun;
>>   	pd.prb = prb;
>>   
>>   	/* Get (or create) the provider for the PID of the probe. */

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

[Eugene Loh]

On 1/9/25 15:52, Eugene Loh wrote:

> On 1/9/25 14:12, Kris Van Hees wrote:
>
>> On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via 
>> DTrace-devel wrote:
>>> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
>>> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
>>>       /*
>>>        * Fields below this point do not apply to underlying probes.
>>>        */
>>> +    int pps_absoff;                /* use "-" for overlying probe 
>>> function */
>> Why use this?  All you do with it is record that func is "-", and 
>> then later
>> in the pid provider, you actually use that to force the function name 
>> in the
>> probe description to be "-".  But that function name in the probe 
>> desc is (in
>> the pid provider) already coming from pps_fun, and the code was already
>> assigning func to pps_fun, so it would be "-" anyway.  So the 
>> pps_absoff seems
>> to serve no purpose, and you could just rely on pps_fun being "-" 
>> when needed.
>
> Yeah, good point.  Let me see if I can remember/understand/explain 
> what's going on here.
>
> In essence, the problem is we have to pass two function names: "-" 
> (for the overlying probe) and the actual function name (for the 
> underlying probe);  up until now, they were both the same.  I tossed 
> around different ideas for this.  E.g., pass two function names in. 
> Or, get the actual function name inside dt_prov_uprobe. It seemed to 
> make most sense (given the alternatives) to pass in the actual 
> function name and add this flag to say when the overlying probe's 
> function name should be "-".  Does that make sense?
>
> Note that pps_fun is not "-".  Although we set it to func ("-"), the 
> code that processes this case then fills pps_fun in with the actual 
> function name.  Again, one option is not to do this until 
> dt_prov_uprobe.c, but that introduces some other hassles.

Hmm.  Having said all that, maybe another (better?!) way of solving this 
problem is to add another probe type to pid_probetype_t.  Maybe 
DTPPT_ABSOFFSETS.  After all, we already have this problem of a 
"special" name for the overlying probe while requiring the "real" name 
for the underlying probe:  we have that problem for probe names.  There, 
we solve the problem by checking the pid_probetype_t.  So maybe that 
same approach should be used here, for probe function.

Let me know.  If you agree/prefer, I can revise the patch.

>
>>>       pid_t pps_pid;                /* task PID */
>>>       uint64_t pps_nameoff;            /* offset to use for name */
>>>   } pid_probespec_t;
>>> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
>>> @@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const 
>>> GElf_Sym *symp, const char *func)
>>>       psp->pps_fun = (char *) func;
>>>       psp->pps_nameoff = 0;
>>>       psp->pps_off = symp->st_value - pp->dpp_vaddr;
>>> +    psp->pps_absoff = 0;
>>>   -    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);
>>> +
>>> +        psp->pps_absoff = 1;
>>> +        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;  // make sure 
>>> dt_prov_uprobe uses it
>>> +        psp->pps_off = off - sym.st_value;   // make sure 
>>> dt_prov_uprobe uses it // dump this
>>> +        psp->pps_off = off - pp->dpp_vaddr;  // make sure 
>>> dt_prov_uprobe uses it
>>> +
>>> +        if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, 
>>> DTPPT_OFFSETS) < 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,
>>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
>>> index acf1c914f..ae4b262ac 100644
>>> --- a/libdtrace/dt_prov_uprobe.c
>>> +++ b/libdtrace/dt_prov_uprobe.c
>>> @@ -735,7 +735,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_absoff ? "-" : psp->pps_fun;
>>>       pd.prb = prb;
>>>         /* Get (or create) the provider for the PID of the probe. */

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

[Kris Van Hees]

On Thu, Jan 09, 2025 at 04:00:42PM -0500, Eugene Loh wrote:
> On 1/9/25 15:52, Eugene Loh wrote:
> 
> > On 1/9/25 14:12, Kris Van Hees wrote:
> > 
> > > On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via
> > > DTrace-devel wrote:
> > > > diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
> > > > @@ -47,6 +47,7 @@ typedef struct pid_probespec {
> > > >       /*
> > > >        * Fields below this point do not apply to underlying probes.
> > > >        */
> > > > +    int pps_absoff;                /* use "-" for overlying
> > > > probe function */
> > > Why use this?  All you do with it is record that func is "-", and
> > > then later
> > > in the pid provider, you actually use that to force the function
> > > name in the
> > > probe description to be "-".  But that function name in the probe
> > > desc is (in
> > > the pid provider) already coming from pps_fun, and the code was already
> > > assigning func to pps_fun, so it would be "-" anyway.  So the
> > > pps_absoff seems
> > > to serve no purpose, and you could just rely on pps_fun being "-"
> > > when needed.
> > 
> > Yeah, good point.  Let me see if I can remember/understand/explain
> > what's going on here.
> > 
> > In essence, the problem is we have to pass two function names: "-" (for
> > the overlying probe) and the actual function name (for the underlying
> > probe);  up until now, they were both the same.  I tossed around
> > different ideas for this.  E.g., pass two function names in. Or, get the
> > actual function name inside dt_prov_uprobe. It seemed to make most sense
> > (given the alternatives) to pass in the actual function name and add
> > this flag to say when the overlying probe's function name should be
> > "-".  Does that make sense?
> > 
> > Note that pps_fun is not "-".  Although we set it to func ("-"), the
> > code that processes this case then fills pps_fun in with the actual
> > function name.  Again, one option is not to do this until
> > dt_prov_uprobe.c, but that introduces some other hassles.

Ok, I see what you mean...  because we rely on underlying probes, we need to
track the function name that the absolute offset falls in in order to be able
to determine the underlying uprobe that the PID probe maps onto (especially
since there might be another probe on it already).

However, as far as I can see there is not really any need to have a function
name on the underlying probe.  After all, it is actually a uprobe that is
identified by dev/inode/offset where offset is an offset into the mapping at
dev/inode, i.e. not relative to any particular function at all.  So, not
having the function name associated with the underlying probe would make more
sense than what we are doing now.

And if we drop the function name from the underlying probe, we suddenly no
longer need to carry it across for absolute offset probe, and this entire
two-function name thing goes away.

> Hmm.  Having said all that, maybe another (better?!) way of solving this
> problem is to add another probe type to pid_probetype_t.  Maybe
> DTPPT_ABSOFFSETS.  After all, we already have this problem of a "special"
> name for the overlying probe while requiring the "real" name for the
> underlying probe:  we have that problem for probe names.  There, we solve
> the problem by checking the pid_probetype_t.  So maybe that same approach
> should be used here, for probe function.
> 
> Let me know.  If you agree/prefer, I can revise the patch.
> 
> > 
> > > >       pid_t pps_pid;                /* task PID */
> > > >       uint64_t pps_nameoff;            /* offset to use for name */
> > > >   } pid_probespec_t;
> > > > diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> > > > @@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const
> > > > GElf_Sym *symp, const char *func)
> > > >       psp->pps_fun = (char *) func;
> > > >       psp->pps_nameoff = 0;
> > > >       psp->pps_off = symp->st_value - pp->dpp_vaddr;
> > > > +    psp->pps_absoff = 0;
> > > >   -    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);
> > > > +
> > > > +        psp->pps_absoff = 1;
> > > > +        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;  // make sure
> > > > dt_prov_uprobe uses it
> > > > +        psp->pps_off = off - sym.st_value;   // make sure
> > > > dt_prov_uprobe uses it // dump this
> > > > +        psp->pps_off = off - pp->dpp_vaddr;  // make sure
> > > > dt_prov_uprobe uses it
> > > > +
> > > > +        if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp,
> > > > DTPPT_OFFSETS) < 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,
> > > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> > > > index acf1c914f..ae4b262ac 100644
> > > > --- a/libdtrace/dt_prov_uprobe.c
> > > > +++ b/libdtrace/dt_prov_uprobe.c
> > > > @@ -735,7 +735,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_absoff ? "-" : psp->pps_fun;
> > > >       pd.prb = prb;
> > > >         /* Get (or create) the provider for the PID of the probe. */

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

[Eugene Loh]

On 1/10/25 00:07, Kris Van Hees wrote:

> On Thu, Jan 09, 2025 at 04:00:42PM -0500, Eugene Loh wrote:
>> On 1/9/25 15:52, Eugene Loh wrote:
>>> On 1/9/25 14:12, Kris Van Hees wrote:
>>>> On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
>>>>> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
>>>>> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
>>>>>        /*
>>>>>         * Fields below this point do not apply to underlying probes.
>>>>>         */
>>>>> +    int pps_absoff;                /* use "-" for overlying probe function */
>>>> Why use this?  All you do with it is record that func is "-", and then later
>>>> in the pid provider, you actually use that to force the function name in the
>>>> probe description to be "-".  But that function name in the probe desc is (in
>>>> the pid provider) already coming from pps_fun, and the code was already
>>>> assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
>>>> to serve no purpose, and you could just rely on pps_fun being "-" when needed.
>>> Yeah, good point.  Let me see if I can remember/understand/explain
>>> what's going on here.
>>>
>>> In essence, the problem is we have to pass two function names: "-" (for
>>> the overlying probe) and the actual function name (for the underlying
>>> probe);  up until now, they were both the same.  I tossed around
>>> different ideas for this.  E.g., pass two function names in. Or, get the
>>> actual function name inside dt_prov_uprobe. It seemed to make most sense
>>> (given the alternatives) to pass in the actual function name and add
>>> this flag to say when the overlying probe's function name should be
>>> "-".  Does that make sense?
>>>
>>> Note that pps_fun is not "-".  Although we set it to func ("-"), the
>>> code that processes this case then fills pps_fun in with the actual
>>> function name.  Again, one option is not to do this until
>>> dt_prov_uprobe.c, but that introduces some other hassles.
> Ok, I see what you mean...  because we rely on underlying probes, we need to
> track the function name that the absolute offset falls in in order to be able
> to determine the underlying uprobe that the PID probe maps onto (especially
> since there might be another probe on it already).
>
> However, as far as I can see there is not really any need to have a function
> name on the underlying probe.  After all, it is actually a uprobe that is
> identified by dev/inode/offset where offset is an offset into the mapping at
> dev/inode, i.e. not relative to any particular function at all.  So, not
> having the function name associated with the underlying probe would make more
> sense than what we are doing now.
>
> And if we drop the function name from the underlying probe, we suddenly no
> longer need to carry it across for absolute offset probe, and this entire
> two-function name thing goes away.

Right.  I considered that, but decided against it.  I think the reason 
was that, although we don't really need a function name for naming the 
underlying uprobe (as you point out), it's nice to have the function for 
the whole "USDT ignore clause" stuff.  Put another way, we *do* use the 
function name of the underlying probe... not so much for naming the 
uprobe but for an extra USDT ignore_clause() check.  If we wanted to, we 
could get rid of that check, but... well, that's the trade-off that 
we're facing here.

>> Hmm.  Having said all that, maybe another (better?!) way of solving this
>> problem is to add another probe type to pid_probetype_t.  Maybe
>> DTPPT_ABSOFFSETS.  After all, we already have this problem of a "special"
>> name for the overlying probe while requiring the "real" name for the
>> underlying probe:  we have that problem for probe names.  There, we solve
>> the problem by checking the pid_probetype_t.  So maybe that same approach
>> should be used here, for probe function.
>>
>> Let me know.  If you agree/prefer, I can revise the patch.

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

[Kris Van Hees]

On Fri, Jan 10, 2025 at 01:30:50AM -0500, Eugene Loh wrote:
> On 1/10/25 00:07, Kris Van Hees wrote:
> 
> > On Thu, Jan 09, 2025 at 04:00:42PM -0500, Eugene Loh wrote:
> > > On 1/9/25 15:52, Eugene Loh wrote:
> > > > On 1/9/25 14:12, Kris Van Hees wrote:
> > > > > On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
> > > > > > diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
> > > > > > @@ -47,6 +47,7 @@ typedef struct pid_probespec {
> > > > > >        /*
> > > > > >         * Fields below this point do not apply to underlying probes.
> > > > > >         */
> > > > > > +    int pps_absoff;                /* use "-" for overlying probe function */
> > > > > Why use this?  All you do with it is record that func is "-", and then later
> > > > > in the pid provider, you actually use that to force the function name in the
> > > > > probe description to be "-".  But that function name in the probe desc is (in
> > > > > the pid provider) already coming from pps_fun, and the code was already
> > > > > assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
> > > > > to serve no purpose, and you could just rely on pps_fun being "-" when needed.
> > > > Yeah, good point.  Let me see if I can remember/understand/explain
> > > > what's going on here.
> > > > 
> > > > In essence, the problem is we have to pass two function names: "-" (for
> > > > the overlying probe) and the actual function name (for the underlying
> > > > probe);  up until now, they were both the same.  I tossed around
> > > > different ideas for this.  E.g., pass two function names in. Or, get the
> > > > actual function name inside dt_prov_uprobe. It seemed to make most sense
> > > > (given the alternatives) to pass in the actual function name and add
> > > > this flag to say when the overlying probe's function name should be
> > > > "-".  Does that make sense?
> > > > 
> > > > Note that pps_fun is not "-".  Although we set it to func ("-"), the
> > > > code that processes this case then fills pps_fun in with the actual
> > > > function name.  Again, one option is not to do this until
> > > > dt_prov_uprobe.c, but that introduces some other hassles.
> > Ok, I see what you mean...  because we rely on underlying probes, we need to
> > track the function name that the absolute offset falls in in order to be able
> > to determine the underlying uprobe that the PID probe maps onto (especially
> > since there might be another probe on it already).
> > 
> > However, as far as I can see there is not really any need to have a function
> > name on the underlying probe.  After all, it is actually a uprobe that is
> > identified by dev/inode/offset where offset is an offset into the mapping at
> > dev/inode, i.e. not relative to any particular function at all.  So, not
> > having the function name associated with the underlying probe would make more
> > sense than what we are doing now.
> > 
> > And if we drop the function name from the underlying probe, we suddenly no
> > longer need to carry it across for absolute offset probe, and this entire
> > two-function name thing goes away.
> 
> Right.  I considered that, but decided against it.  I think the reason was
> that, although we don't really need a function name for naming the
> underlying uprobe (as you point out), it's nice to have the function for the
> whole "USDT ignore clause" stuff.  Put another way, we *do* use the function
> name of the underlying probe... not so much for naming the uprobe but for an
> extra USDT ignore_clause() check.  If we wanted to, we could get rid of that
> check, but... well, that's the trade-off that we're facing here.

II would propose that we do the following:

- For absolute offset probes, use new type DTPPT_ABSOFF (or ABSOFFSET) as you
  suggest.  The uprobe provider will use that to know that to put "-" as the
  function name component of the overlying probe.

- For the underlying probe, store the function name (from pps_fun) in a member
  of dt_uprobe_t (fun or func, perhaps).  That can be used in ignore_clause().

- For the underlying probe, leave the function name empty (to be consistent
  with the fact that the underlying probe is a uprobe, identified by dev/inode
  and offset within the inode.)

This way we no longer display the function name for underlying probes in the
probe listing, which is correct, and yet we retain the ignore_clause()
optimization.

	Kris