[PATCH v7 0/6] usdt typed args, translators and arg mapping

[PATCH v7 0/6] usdt typed args, translators and arg mapping

[Nick Alcock]

This series adds back support for the probe foo (...) : (...) syntax
in USDT provider definitions.  The dtprobed side is routine, pulling
in the DOF and adding it to a new set of dof_parser.h records fed
through to the DOF stash and ultimately to dt_pid.c and eventually
dt_prov_uprobe.c: the end result is that the DOF for a given probe's
mappings ends up in the dt_uprobe_t in more or less the same form
as it was in the DOF.

After that things get tricky, because we want to map not only arg[N] but
also argN references (the single existing test for USDT arg mapping
verifies that this works), but the SDT machinery depends on argN being
left unchanged!  So we do the mapping in dt_prov_uprobe.c while reshuffling
things into dt_argdesc_t's, and tell the higher layers that no arg mapping
is ever being performed, then do the actual movement of args in the trampoline
by physically reshuffling the arguments.  This seems to work without breaking
SDT on all supported architectures, and is surprisingly simple (at least, it
surprised me: multiple places where I thought I'd need hundreds of lines of
complex code turned out to need only three or four lines).

New tests are added to verify that USDT translators work when types change,
when arg counts change, and when there are no args at all, and that dtrace
-vln produces the right output (all previous tests for USDT -vln, translated
or not, are currently disabled because they all use wildcards: this new one
doesn't).

We also add new tests to test that pid/USDT interoperability does not
break arg mapping and USDT args in general (deferred args are coming,
but are trickier to get working).  That's in a separate commit because
it renames and modifies an existing test significantly (it still does the
same thing).

We also fix a tiny error-path-related bug in usdt encountered in the
course of development.

Changes since v6:
  Move all references to DTPPT_USDT into the patch that defines it
  (one slipped into the first patch by mistake).

Changes since v5:
  Moved a couple of mispositioned hunks between patches.
  Rephrased a couple of terrible comments.
  A few formatting fixes.
  (No conditional added around call to populate_args(), since
  populate_args() doesn't care if the probe is USDT or not: if the
  probe has args, it'll populate them.)

Changes since v4:
  Moved the arg saving into the dt_prov_uprobe trampoline.
  Store the args in one contiguous strtab rather than two distinct
  ones.
  Drop some unnecessary "no xargs, fall back to nargs" code (there are
  always xargs if there are any user-visible args at all, the nargs
  are not user-visible).
  Set up the arguments even if USDT probes with args find that their
  underlying probe is already in place for a pid probe.
  Identify USDT probes reliably and use this to prevent creation of
  multiple USDT probes at the same locus (new DTPPT_USDT probe type:
  DTPPT_OFFSETS is now reserved for pid offset probes).
  Fixed up a few bad comments.
  Added test/unittest/usdt/tst.pid*sh, out of the guts of
  tst.pidprobes.sh, which is renamed and modified to pidprobes.sh and
  called by all three tests.

Changes since v3:
  Rebase against latest dev, adjusting the trampoline: we should now
  handle the overlapping pid and usdt case, though this still being
  verified.
  Eliminate remaining references to 'remapping'.
  Move mis-merged hunk of dt_cg.c changes into the right commit.
  Drop showUSDT arg stuff in favour of Kris's improved version.

Changes since v2:
  Populate dt_argdesc_t's in USDT probe discovery rather than in
  probe_info.

Changes since v1:
  Adapted to review comments (all comments but the stuff around
  trampolines and arg mapping, which I don't understand well
  enough to implement); in particular track xlated args and
  mappings together, and move from a flags word in the
  dof_parser struct's DIT_PROBE record to an arg count

Nick Alcock (6):
  usdt: get arg types and xlations into DTrace from the DOF
  dtprobed: stop skipping zero-tracepoint probes in dof_stash.c
  cg: add argument mapping in the trampoline
  usdt: typed args and arg mapping
  usdt: new tests for USDT arg sanity with overlapping pid probes
  usdt: fix create_underlying error path

 dtprobed/dof_stash.c                          |  21 +-
 dtprobed/dtprobed.c                           |  10 +-
 include/dtrace/pid.h                          |   8 +
 libcommon/dof_parser.c                        | 150 ++++++++++----
 libcommon/dof_parser.h                        |  64 +++++-
 libdtrace/dt_cg.c                             |  25 +++
 libdtrace/dt_cg.h                             |   1 +
 libdtrace/dt_pid.c                            |  62 +++++-
 libdtrace/dt_prov_uprobe.c                    | 186 ++++++++++++++++--
 test/triggers/usdt-tst-argmap-prov.d          |   5 +-
 test/triggers/usdt-tst-argmap.c               |   5 +-
 .../dtrace-util/tst.ListProbesArgsUSDT.r      | 130 ++++++++++++
 .../dtrace-util/tst.ListProbesArgsUSDT.r.p    |   2 +
 .../dtrace-util/tst.ListProbesArgsUSDT.sh     |  89 +++++++++
 test/unittest/usdt/err.argmap-null.d          |  41 ++++
 test/unittest/usdt/err.argmap-null.r          |   2 +
 test/unittest/usdt/err.argmap-null.r.p        |   2 +
 .../usdt/{tst.pidprobes.sh => pidprobes.sh}   |  56 ++++--
 test/unittest/usdt/tst.argmap-null.d          |  32 +++
 test/unittest/usdt/tst.argmap-typed-partial.d |  49 +++++
 test/unittest/usdt/tst.argmap-typed.d         |  48 +++++
 test/unittest/usdt/tst.argmap.d               |   5 +-
 test/unittest/usdt/tst.pidargmap.sh           |  11 ++
 test/unittest/usdt/tst.pidargs.sh             |  11 ++
 24 files changed, 929 insertions(+), 86 deletions(-)
 create mode 100644 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
 create mode 100644 test/unittest/usdt/err.argmap-null.d
 create mode 100644 test/unittest/usdt/err.argmap-null.r
 create mode 100755 test/unittest/usdt/err.argmap-null.r.p
 rename test/unittest/usdt/{tst.pidprobes.sh => pidprobes.sh} (82%)
 create mode 100644 test/unittest/usdt/tst.argmap-null.d
 create mode 100644 test/unittest/usdt/tst.argmap-typed-partial.d
 create mode 100644 test/unittest/usdt/tst.argmap-typed.d
 create mode 100755 test/unittest/usdt/tst.pidargmap.sh
 create mode 100755 test/unittest/usdt/tst.pidargs.sh

-- 
2.46.0.278.g36e3a12567

[PATCH v7 1/6] usdt: get arg types and xlations into DTrace from the DOF

[Nick Alcock]

This change propagates native and xlated arg types and mapping info all
the way from the DOF, through the parser and dtprobed and the DOF stash,
into DTrace, where they end up in the pid_probespec_t for USDT probes.
We don't do anything with them once they get there in this commit: no
user-visible impacts expected.

We bump the DOF_PARSED_VERSION since we add three new types of record to the
dof_parser_t, all optional, covering native and xlated args and arg mapping
tables; to make life easier for consumers we emit them in a defined order
(documented in dof_parser.h), and add arg counts to the DIT_PROBE record
that precedes them indicating which will be present and how many args are in
them.  This means we retain the property that you can always tell which
records within a provider are coming next purely from records you already
have: there's no need to make decisions once the records turn up.  The DOF
stash hardly changes: all that happens is that the parsed data written to
each per-probe file gains some extra types of record (it can have
DIT_ARGS_NATIVE, DIT_ARGS_XLAT and DIT_ARGS_MAP entries following the
DIT_PROBE record now).

As usual with DOF_PARSED_VERSION bumps, DTraces that cross this change will
refuse to read probes added by dtprobeds that are on the other side of it.
(Restarting dtprobed will reparse all the probes in the stash to match the
new layout, and newly-started DTraces will pick that up -- so this only
affects running DTraces picking up new probes, which DTrace cannot do yet:
so no visible effects are expected.)

The code is a bit repetitive, with nargs, xargs and arg mapping all handled
separately even though the code is very similar.  In future maybe we could
unify them (but my attempts led to code that was much harder to read than
the original).

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 dtprobed/dof_stash.c   |  15 +++--
 dtprobed/dtprobed.c    |  10 ++-
 include/dtrace/pid.h   |   7 ++
 libcommon/dof_parser.c | 150 +++++++++++++++++++++++++++++++----------
 libcommon/dof_parser.h |  64 ++++++++++++++++--
 libdtrace/dt_pid.c     |  60 +++++++++++++++++
 6 files changed, 257 insertions(+), 49 deletions(-)

diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 0e639076f655..8bf465678217 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -59,8 +59,10 @@
  *    startup if the dof_parsed_t structure changed.  The file consists of a
  *    uint64_t version number (DOF_PARSED_VERSION), then a stream of
  *    dof_parsed_t records, the first of type DIT_PROVIDER, the second
- *    DIT_PROBE, then as many struct dof_parsed_t's of type DIT_TRACEPOINT as
- *    the DIT_PROBE record specifies.
+ *    DIT_PROBE, then optionally some DIT_*ARGS records (if the count of native
+ *    args in the probe is >0, you get a DIT_NATIVE_ARGS: if the count of xlated
+ *    args is >0, you get DIT_XLAT_ARGS and DIT_MAP_ARGS), then as many struct
+ *    dof_parsed_t's of type DIT_TRACEPOINT as the DIT_PROBE record specifies.
  *
  * /run/dtrace/probes/: Per-probe info, written by dtprobed, read by DTrace.
  *
@@ -640,9 +642,14 @@ dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum)
 			break;
 		}
 
-		/* Tracepoint: add to already-open file.  */
+		/* Args info or tracepoint: add to already-open file.  */
+		case DIT_ARGS_NATIVE:
+		case DIT_ARGS_XLAT:
+		case DIT_ARGS_MAP:
 		case DIT_TRACEPOINT:
-			assert(state == DIT_PROBE || state == DIT_TRACEPOINT);
+			assert(state == DIT_PROBE || state == DIT_ARGS_NATIVE ||
+			       state == DIT_ARGS_XLAT || state == DIT_ARGS_MAP ||
+			       state == DIT_TRACEPOINT);
 			state = accump->parsed->type;
 
 			if (write_chunk(parsed_fd, accump->parsed, accump->parsed->size) < 0)
diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index f4c6be1e045d..2ca39b26f88a 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -791,16 +791,20 @@ process_dof(pid_t pid, int out, int in, dev_t dev, ino_t inum, dev_t exec_dev,
 		if (dof_stash_push_parsed(&accum, probe) < 0)
 			goto oom;
 
-		for (j = 0; j < probe->probe.ntp; j++) {
+		j = 0;
+		do {
 			dof_parsed_t *tp = dof_read(pid, in);
 
 			errmsg = "no tracepoints in a probe, or parse state corrupt";
-			if (!tp || tp->type != DIT_TRACEPOINT)
+			if (!tp || tp->type == DIT_PROVIDER || tp->type == DIT_PROBE)
 				goto err;
 
 			if (dof_stash_push_parsed(&accum, tp) < 0)
 				goto oom;
-		}
+
+			if (tp->type == DIT_TRACEPOINT)
+				j++;
+		} while (j < probe->probe.ntp);
 	}
 
 	if (!reparsing)
diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
index 0129674adf0c..25bfacfdbfe2 100644
--- a/include/dtrace/pid.h
+++ b/include/dtrace/pid.h
@@ -35,6 +35,13 @@ typedef struct pid_probespec {
 	ino_t pps_inum;				/* object inode */
 	char *pps_fn;				/* object full filename */
 	uint64_t pps_off;			/* probe offset (in object) */
+	int pps_nargc;				/* number of native args */
+	int pps_xargc;				/* number of xlated and mapped args */
+	char *pps_nargv;			/* array of native args */
+	size_t pps_nargvlen;			/* (high estimate of) length of array */
+	char *pps_xargv;			/* array of xlated args */
+	size_t pps_xargvlen;			/* (high estimate of) length of array */
+	int8_t *pps_argmap;			/* mapped arg indexes */
 
 	/*
 	 * Fields below this point do not apply to underlying probes.
diff --git a/libcommon/dof_parser.c b/libcommon/dof_parser.c
index d6631a1afdcb..1792a8bfcc84 100644
--- a/libcommon/dof_parser.c
+++ b/libcommon/dof_parser.c
@@ -807,10 +807,11 @@ validate_provider(int out, dof_hdr_t *dof, dof_sec_t *sec)
 		}
 
 		dt_dbg_dof("      Probe %d %s:%s:%s:%s with %d offsets, "
-			   "%d is-enabled offsets\n", j,
+			   "%d is-enabled offsets, %i args, %i nargs, argidx %i\n", j,
 			   strtab + prov->dofpv_name, "",
 			   strtab + prb->dofpr_func, strtab + prb->dofpr_name,
-			   prb->dofpr_noffs, prb->dofpr_nenoffs);
+			   prb->dofpr_noffs, prb->dofpr_nenoffs,
+			   prb->dofpr_xargc, prb->dofpr_nargc, prb->dofpr_argidx);
 	}
 
 	return 0;
@@ -879,12 +880,26 @@ validate_probe(int out, dtrace_helper_probedesc_t *dhpb)
 	return 0;
 }
 
+static size_t
+strings_len(const char *strtab, size_t count)
+{
+	size_t len = 0;
+
+	for (; count > 0; count--) {
+		size_t this_len = strlen(strtab) + 1;
+
+		len += this_len;
+		strtab += this_len;
+	}
+	return len;
+}
+
 static void
 emit_probe(int out, dtrace_helper_probedesc_t *dhpb)
 {
 	int		i;
-	dof_parsed_t	*probe_msg;
-	size_t		probe_msg_size;
+	dof_parsed_t	*msg;
+	size_t		msg_size;
 	char		*ptr;
 
 	dt_dbg_dof("      Creating probe %s:%s:%s:%s\n", dhpb->dthpb_prov,
@@ -893,35 +908,106 @@ emit_probe(int out, dtrace_helper_probedesc_t *dhpb)
 	if (validate_probe(out, dhpb) != 0)
 		return;
 
-	probe_msg_size = offsetof(dof_parsed_t, probe.name) +
-	    strlen(dhpb->dthpb_mod) + 1 + strlen(dhpb->dthpb_func) + 1 +
-	    strlen(dhpb->dthpb_name) + 1;
+	/*
+	 * Compute the size of all the optional elements first, to fill out the
+	 * flags.
+	 */
 
-	probe_msg = malloc(probe_msg_size);
-	if (!probe_msg) {
-		dof_error(out, ENOMEM, "Out of memory allocating probe");
-		return;
-	}
+	msg_size = offsetof(dof_parsed_t, probe.name) +
+		   strlen(dhpb->dthpb_mod) + 1 +
+		   strlen(dhpb->dthpb_func) + 1 +
+		   strlen(dhpb->dthpb_name) + 1;
 
-	memset(probe_msg, 0, probe_msg_size);
+	msg = malloc(msg_size);
+	if (!msg)
+		goto oom;
 
-	probe_msg->size = probe_msg_size;
-	probe_msg->type = DIT_PROBE;
-	probe_msg->probe.ntp = dhpb->dthpb_noffs + dhpb->dthpb_nenoffs;
-	ptr = stpcpy(probe_msg->probe.name, dhpb->dthpb_mod);
+	memset(msg, 0, msg_size);
+
+	msg->size = msg_size;
+	msg->type = DIT_PROBE;
+	msg->probe.ntp = dhpb->dthpb_noffs + dhpb->dthpb_nenoffs;
+	msg->probe.nargc = dhpb->dthpb_nargc;
+	msg->probe.xargc = dhpb->dthpb_xargc;
+
+	ptr = stpcpy(msg->probe.name, dhpb->dthpb_mod);
 	ptr++;
 	ptr = stpcpy(ptr, dhpb->dthpb_func);
 	ptr++;
 	strcpy(ptr, dhpb->dthpb_name);
-	dof_parser_write_one(out, probe_msg, probe_msg_size);
+	dof_parser_write_one(out, msg, msg_size);
 
-	free(probe_msg);
+	free(msg);
 
-	/* XXX TODO translated args
-	   pp->ftp_nargs = dhpb->dthpb_xargc;
-	   pp->ftp_xtypes = dhpb->dthpb_xtypes;
-	   pp->ftp_ntypes = dhpb->dthpb_ntypes;
-	*/
+	/*
+	 * Emit info on all native and translated args in turn.
+	 *
+	 * FIXME: this code is a bit repetitious.
+	 *
+	 * First native args (if any).
+	 */
+
+	if (dhpb->dthpb_nargc > 0) {
+		size_t	nargs_size;
+
+		nargs_size = strings_len(dhpb->dthpb_ntypes, dhpb->dthpb_nargc);
+		msg_size = offsetof(dof_parsed_t, nargs.args) + nargs_size;
+
+		msg = malloc(msg_size);
+		if (!msg)
+			goto oom;
+
+		memset(msg, 0, msg_size);
+
+		msg->size = msg_size;
+		msg->type = DIT_ARGS_NATIVE;
+		memcpy(msg->nargs.args, dhpb->dthpb_ntypes, nargs_size);
+		dof_parser_write_one(out, msg, msg_size);
+
+		free(msg);
+
+		/* Then translated args. */
+
+		if (dhpb->dthpb_xargc > 0) {
+			size_t	xargs_size, map_size;
+
+			xargs_size = strings_len(dhpb->dthpb_xtypes,
+						 dhpb->dthpb_xargc);
+			msg_size = offsetof(dof_parsed_t, xargs.args) +
+				   xargs_size;
+
+			msg = malloc(msg_size);
+			if (!msg)
+				goto oom;
+
+			memset(msg, 0, msg_size);
+
+			msg->size = msg_size;
+			msg->type = DIT_ARGS_XLAT;
+			memcpy(msg->xargs.args, dhpb->dthpb_xtypes, xargs_size);
+			dof_parser_write_one(out, msg, msg_size);
+
+			free(msg);
+
+			/* Then the mapping table. */
+
+			map_size = dhpb->dthpb_xargc * sizeof(int8_t);
+			msg_size = offsetof(dof_parsed_t, argmap.argmap) +
+				   map_size;
+
+			msg = malloc(msg_size);
+			if (!msg)
+				goto oom;
+
+			memset(msg, 0, msg_size);
+
+			msg->size = msg_size;
+			msg->type = DIT_ARGS_MAP;
+			memcpy(msg->argmap.argmap, dhpb->dthpb_args, map_size);
+			dof_parser_write_one(out, msg, msg_size);
+			free(msg);
+		}
+	}
 
 	/*
 	 * Emit info on each tracepoint in turn.
@@ -938,18 +1024,10 @@ emit_probe(int out, dtrace_helper_probedesc_t *dhpb)
 	for (i = 0; i < dhpb->dthpb_nenoffs; i++)
 		emit_tp(out, dhpb->dthpb_base, dhpb->dthpb_enoffs[i], 1);
 
-	/*
-	 * XXX later:
-	 * If the arguments are shuffled around we set the argument remapping
-	 * table. Later, when the probe fires, we only remap the arguments
-	 * if the table is non-NULL.
-	 *
-	for (i = 0; i < dhpb->dthpb_xargc; i++) {
-		if (dhpb->dthpb_args[i] != i) {
-			pp->ftp_argmap = dhpb->dthpb_args;
-			break;
-		}
-	} */
+	return;
+ oom:
+	dof_error(out, ENOMEM, "Out of memory allocating %zi bytes for probe",
+		  msg_size);
 }
 
 static void
diff --git a/libcommon/dof_parser.h b/libcommon/dof_parser.h
index d3a6836f15fd..8f42d00551e3 100644
--- a/libcommon/dof_parser.h
+++ b/libcommon/dof_parser.h
@@ -15,10 +15,19 @@
 #include <dtrace/helpers.h>
 
 /*
- * Result of DOF probe parsing.  We receive a provider info structure, followed
- * by N probe info structures each of which is followed by possibly many
- * tracepoint info structures, all tagged.  Things not yet used for probe
- * creation (like args, translated types, etc) are not returned.
+ * Result of DOF probe parsing.  The order of elements in the parsed stream
+ * is:
+ *
+ * DIT_PROVIDER (at least 1, which contains...)
+ *   DIT_PROBE (at least 1, each of which has...)
+ *     DIT_ARGS_NATIVE (1, optional)
+ *     DIT_ARGS_XLAT (1, optional)
+ *     DIT_ARGS_MAP (1, optional)
+ *     DIT_TRACEPOINT (any number >= 1)
+ *
+ * The dof_parsed.provider.flags word indicates the presence of the
+ * various optional args records in the following stream (you can rely on
+ * them if it simplifies things, but you don't have to).
  *
  * On error, a DIT_ERR structure is returned with an error message.
  */
@@ -27,7 +36,10 @@ typedef enum dof_parsed_info {
 	DIT_PROVIDER = 0,
 	DIT_PROBE = 1,
 	DIT_TRACEPOINT = 2,
-	DIT_ERR = 3
+	DIT_ERR = 3,
+	DIT_ARGS_NATIVE = 4,
+	DIT_ARGS_XLAT = 5,
+	DIT_ARGS_MAP = 6,
 } dof_parsed_info_t;
 
 /*
@@ -37,7 +49,7 @@ typedef enum dof_parsed_info {
  * start which is the version of the dof_parseds within it.  The data flowing
  * over the stream from the seccomped parser has no such prefix.
  */
-#define DOF_PARSED_VERSION 1
+#define DOF_PARSED_VERSION 2
 
 typedef struct dof_parsed {
 	/*
@@ -59,18 +71,58 @@ typedef struct dof_parsed {
 			 */
 			char name[1];
 		} provider;
+
 		struct dpi_probe_info {
 			/*
 			 * Number of tracepoints that follow.
 			 */
 			size_t ntp;
 
+			/*
+			 * Number of native arguments that follow (if > 0, a
+			 * DIT_ARGS_NATIVE will be received).
+			 */
+			size_t nargc;
+
+			/*
+			 * Number of xlated arguments that follow (if > 0, a
+			 * DIT_ARGS_XLAT and DIT_ARGS_MAP will be received).
+			 */
+			size_t xargc;
+
 			/*
 			 * Probe module, function, and name (\0-separated).
 			 */
 			char name[1];
 		} probe;
 
+		/* V2+ only.  */
+		struct dpi_probe_args_native_info {
+			/*
+			 * Array of native args.  nargc in length.
+			 */
+			char args[1];
+		} nargs;
+
+		/* V2+ only.  */
+		struct dpi_probe_args_xlat_info {
+			/*
+			 * Array of translated args.  xargc in length.
+			 */
+			char args[1];
+		} xargs;
+
+		/*
+		 * V2+ only.
+		 */
+		struct dpi_probe_args_map_info {
+			/*
+			 * Mapping from native arg index to xlated arg index.
+			 * xargc in length.
+			 */
+			int8_t argmap[1];
+		} argmap;
+
 		struct dpi_tracepoint_info {
 			/*
 			 * Offset of this tracepoint.
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 41a87955ec1b..189b15f61a89 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -866,6 +866,9 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 		uint64_t *dof_version;
 		char *prv, *mod, *fun, *prb;
 		dof_parsed_t *provider, *probe;
+		ssize_t nargvlen = 0, xargvlen = 0;
+		char *nargv = NULL, *xargv = NULL;
+		int8_t *argmap = NULL;
 
 		/*
 		 * Regular files only: in particular, skip . and ..,
@@ -917,6 +920,48 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 		p += probe->size;
 		seen_size += probe->size;
 
+		/*
+		 * Assume the order given in dof_parser.h, for simplicity.
+		 */
+		if (probe->probe.nargc > 0) {
+			dof_parsed_t *args = (dof_parsed_t *) p;
+
+			if (!validate_dof_record(path, args, DIT_ARGS_NATIVE,
+						 dof_buf_size, seen_size))
+				goto parse_err;
+
+			nargv = args->nargs.args;
+			nargvlen = args->size - offsetof(dof_parsed_t, nargs.args);
+			assert(nargvlen >= 0);
+
+			p += args->size;
+			seen_size += args->size;
+		}
+		if (probe->probe.xargc > 0) {
+			dof_parsed_t *args = (dof_parsed_t *) p;
+
+			if (!validate_dof_record(path, args, DIT_ARGS_XLAT,
+						 dof_buf_size, seen_size))
+				goto parse_err;
+
+			xargv = args->xargs.args;
+			xargvlen = args->size - offsetof(dof_parsed_t, xargs.args);
+			assert(xargvlen >= 0);
+
+			p += args->size;
+			seen_size += args->size;
+			args = (dof_parsed_t *) p;
+
+			if (!validate_dof_record(path, args, DIT_ARGS_MAP,
+						 dof_buf_size, seen_size))
+				goto parse_err;
+
+			argmap = args->argmap.argmap;
+
+			p += args->size;
+			seen_size += args->size;
+		}
+
 		/*
 		 * Now the parsed DOF for this probe's tracepoints.
 		 */
@@ -967,6 +1012,21 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 			psp.pps_off = tp->tracepoint.addr - pmp->pr_file->first_segment->pr_vaddr;
 			psp.pps_nameoff = 0;
 
+			if (nargv) {
+				psp.pps_nargc = probe->probe.nargc;
+				psp.pps_nargvlen = nargvlen;
+				psp.pps_nargv = nargv;
+			}
+
+			if (xargv) {
+				psp.pps_xargc = probe->probe.xargc;
+				psp.pps_xargvlen = xargvlen;
+				psp.pps_xargv = xargv;
+			}
+
+			if (argmap)
+				psp.pps_argmap = argmap;
+
 			dt_dprintf("providing %s:%s:%s:%s for pid %d\n", psp.pps_prv,
 				   psp.pps_mod, psp.pps_fun, psp.pps_prb, psp.pps_pid);
 			if (pvp->impl->provide_probe(dtp, &psp) < 0) {
-- 
2.46.0.278.g36e3a12567

[PATCH v7 2/6] dtprobed: stop skipping zero-tracepoint probes in dof_stash.c

[Nick Alcock]

We already validate that they don't exist in dof_parser.c, so no such probes
can ever get here.  Handling this impossible, untestable case has just got
harder, because we'd have to skip DIT_ARGS_{NATIVE,XLAT,MAP} records too.
Just stop considering it.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 dtprobed/dof_stash.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 8bf465678217..82fdd3174759 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -586,12 +586,6 @@ dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum)
 			if (err != 0)
 				goto err_provider;
 
-			/*
-			 * Skip probes with zero tracepoints entirely.
-			 */
-			if (accump->parsed->probe.ntp == 0)
-				break;
-
 			mod = accump->parsed->probe.name;
 			assert(accump->parsed->size > (mod - (char *) accump->parsed));
 			fun = mod + strlen(mod) + 1;
-- 
2.46.0.278.g36e3a12567

[PATCH v7 3/6] cg: add argument mapping in the trampoline

[Nick Alcock]

Before now, argument mapping was restricted to the args[] array, and was
implemented in dt_cg_array_op in the same place where we decide whether
to automatically copyin() userspace strings.

But existing DTrace testcases suggest that arg mapping is also applied to
argN for USDT, even though this is inconsistent with SDT and arguably less
flexible than having argN be the unmapped arguments in all cases.  Add a new
function dt_cg_tramp_map_args(), which can be called from trampolines to
apply mappings (destructively, but it keeps a copy of the old args in the
save area so you can unapply them if you need to).  No existing providers do
any mapping, so this is not yet called, but it's about to be.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 libdtrace/dt_cg.c | 25 +++++++++++++++++++++++++
 libdtrace/dt_cg.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 39c27ab0ec0b..aa230dcbae36 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -831,6 +831,29 @@ dt_cg_tramp_restore_args(dt_pcb_t *pcb)
 	}
 }
 
+/*
+ * Populate the probe arguments based on the provided dt_argdesc_t array.  The
+ * caller must save the arguments because argument mapping copies values from
+ * the saved arguments to the current arguments.  After this function returns,
+ * the caller should adjust the mapping to reflect that shuffling has been done.
+ *
+ * The caller must ensure that %r7 contains the value set by the
+ * dt_cg_tramp_prologue*() functions.
+ */
+void
+dt_cg_tramp_map_args(dt_pcb_t *pcb, dt_argdesc_t *args, size_t nargs)
+{
+	dt_irlist_t	*dlp = &pcb->pcb_ir;
+	int		i;
+
+	for (i = 0; i < nargs; i++) {
+		if (args[i].mapping != i) {
+			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_ORIG_ARG(args[i].mapping)));
+			emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
+		}
+	}
+}
+
 typedef struct {
 	dt_irlist_t	*dlp;
 	dt_activity_t	act;
@@ -5061,6 +5084,8 @@ dt_cg_array_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 	 * unless the argument reference is provided by a dynamic translator.
 	 * If we're using a dynamic translator for args[], then just set dn_reg
 	 * to an invalid reg and return: DIF_OP_XLARG will fetch the arg later.
+	 *
+	 * If this is a userland variable, note that we need to copy it in.
 	 */
 	if (idp->di_id == DIF_VAR_ARGS) {
 		if ((idp->di_kind == DT_IDENT_XLPTR ||
diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
index 0a7c7ba6a8c5..fb26c125adbc 100644
--- a/libdtrace/dt_cg.h
+++ b/libdtrace/dt_cg.h
@@ -34,6 +34,7 @@ extern void dt_cg_tramp_get_var(dt_pcb_t *pcb, const char *name, int isstore,
 extern void dt_cg_tramp_del_var(dt_pcb_t *pcb, const char *name);
 extern void dt_cg_tramp_save_args(dt_pcb_t *pcb);
 extern void dt_cg_tramp_restore_args(dt_pcb_t *pcb);
+extern void dt_cg_tramp_map_args(dt_pcb_t *pcb, dt_argdesc_t *args, size_t nargs);
 extern void dt_cg_tramp_call_clauses(dt_pcb_t *pcb, const dt_probe_t *prp,
 				     dt_activity_t act);
 extern void dt_cg_tramp_return(dt_pcb_t *pcb);
-- 
2.46.0.278.g36e3a12567

[PATCH v7 4/6] usdt: typed args and arg mapping

[Nick Alcock]

This change propagates the arg type and mapping info we just got into the
pid_probespec_t onward into the underlying dt_uprobe_t and shuffles the
argument values by calling dt_cg_tramp_map_args().  With this, USDT probes
should now have visible types which obey the :-notation translators
specified in their .d file (this is now tested).

The probe_info hook has been moved from the underlying probe to the
overlying one as part of this: underlying probes don't have any user-visible
args to get info for, and their probe_info functions are never called.  We
populate the dt_argdesc_t in a function ultimately called from USDT probe
discovery via provide_underlying, so it predates trampoline setup let alone
probe_info on the overlying probe and all the args info is present before it
is needed.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 include/dtrace/pid.h                          |   1 +
 libdtrace/dt_pid.c                            |   2 +-
 libdtrace/dt_prov_uprobe.c                    | 184 ++++++++++++++++--
 test/triggers/usdt-tst-argmap-prov.d          |   5 +-
 test/triggers/usdt-tst-argmap.c               |   5 +-
 .../dtrace-util/tst.ListProbesArgsUSDT.r      | 130 +++++++++++++
 .../dtrace-util/tst.ListProbesArgsUSDT.r.p    |   2 +
 .../dtrace-util/tst.ListProbesArgsUSDT.sh     |  89 +++++++++
 test/unittest/usdt/err.argmap-null.d          |  41 ++++
 test/unittest/usdt/err.argmap-null.r          |   2 +
 test/unittest/usdt/err.argmap-null.r.p        |   2 +
 test/unittest/usdt/tst.argmap-null.d          |  32 +++
 test/unittest/usdt/tst.argmap-typed-partial.d |  49 +++++
 test/unittest/usdt/tst.argmap-typed.d         |  48 +++++
 test/unittest/usdt/tst.argmap.d               |   5 +-
 15 files changed, 578 insertions(+), 19 deletions(-)
 create mode 100644 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
 create mode 100755 test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
 create mode 100644 test/unittest/usdt/err.argmap-null.d
 create mode 100644 test/unittest/usdt/err.argmap-null.r
 create mode 100755 test/unittest/usdt/err.argmap-null.r.p
 create mode 100644 test/unittest/usdt/tst.argmap-null.d
 create mode 100644 test/unittest/usdt/tst.argmap-typed-partial.d
 create mode 100644 test/unittest/usdt/tst.argmap-typed.d

diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
index 25bfacfdbfe2..c53e600475df 100644
--- a/include/dtrace/pid.h
+++ b/include/dtrace/pid.h
@@ -22,6 +22,7 @@ typedef enum pid_probetype {
 	DTPPT_ENTRY,
 	DTPPT_RETURN,
 	DTPPT_OFFSETS,
+	DTPPT_USDT,
 	DTPPT_IS_ENABLED
 } pid_probetype_t;
 
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 189b15f61a89..a0b3d892fd23 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -1001,7 +1001,7 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
 				goto oom;
 			}
 
-			psp.pps_type = tp->tracepoint.is_enabled ? DTPPT_IS_ENABLED : DTPPT_OFFSETS;
+			psp.pps_type = tp->tracepoint.is_enabled ? DTPPT_IS_ENABLED : DTPPT_USDT;
 			psp.pps_prv = prv;
 			psp.pps_mod = mod;
 			psp.pps_fun = fun;
diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index a8c1ab2761f0..1a1500c03ecc 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -46,9 +46,11 @@
 /* Provider name for the underlying probes. */
 static const char	prvname[] = "uprobe";
 
-#define PP_IS_RETURN	1
-#define PP_IS_FUNCALL	2
-#define PP_IS_ENABLED	4
+#define PP_IS_RETURN	0x1
+#define PP_IS_FUNCALL	0x2
+#define PP_IS_ENABLED	0x4
+#define PP_IS_USDT	0x8
+#define PP_IS_MAPPED	0x10
 
 typedef struct dt_uprobe {
 	dev_t		dev;
@@ -57,7 +59,10 @@ typedef struct dt_uprobe {
 	uint64_t	off;
 	int		flags;
 	tp_probe_t	*tp;
-	dt_list_t	probes;		/* pid/USDT probes triggered by it */
+	int		argc;		   /* number of args */
+	dt_argdesc_t	*args;		   /* args array (points into argvbuf) */
+	char		*argvbuf;	   /* arg strtab */
+	dt_list_t	probes;		   /* pid/USDT probes triggered by it */
 } dt_uprobe_t;
 
 typedef struct list_probe {
@@ -123,6 +128,8 @@ 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->args);
+	dt_free(dtp, upp->argvbuf);
 	dt_free(dtp, upp);
 }
 
@@ -516,6 +523,80 @@ static int discover(dtrace_hdl_t *dtp)
 	return 0;
 }
 
+/*
+ * Populate args for an underlying probe for use by the overlying USDT probe.
+ * The overlying probe does not exist yet at this point, so the arg data is
+ * stored in the underlying probe instead and will be accessed when probe_info
+ * is called in the overlying probe.
+ *
+ * Move it into dt_argdesc_t's for use later on. The char *'s in that structure
+ * are pointers into the argvbuf array, which is a straight concatenated copy of
+ * the nargv/xargv in the pid_probespec_t.
+ */
+static int populate_args(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
+			 dt_uprobe_t *upp)
+{
+	char	**nargv = NULL;
+	char	*nptr = NULL, *xptr = NULL;
+	size_t	i;
+
+	upp->argc = psp->pps_xargc;
+
+	/*
+	 * If we have a nonzero number of args, we always have at least one narg
+	 * and at least one xarg.  Double-check to be sure.  (These are not
+	 * populated, and thus left 0/NULL, for non-USDT probes.)
+	 */
+	if (upp->argc == 0 || psp->pps_xargv == NULL || psp->pps_nargv == NULL
+		|| psp->pps_xargvlen == 0 || psp->pps_nargvlen == 0)
+		return 0;
+
+	upp->argvbuf = dt_alloc(dtp, psp->pps_xargvlen + psp->pps_nargvlen);
+	if(upp->argvbuf == NULL)
+		return -1;
+	memcpy(upp->argvbuf, psp->pps_xargv, psp->pps_xargvlen);
+	xptr = upp->argvbuf;
+
+	memcpy(upp->argvbuf + psp->pps_xargvlen, psp->pps_nargv, psp->pps_nargvlen);
+	nptr = upp->argvbuf + psp->pps_xargvlen;
+
+	upp->args = dt_calloc(dtp, upp->argc, sizeof(dt_argdesc_t));
+	if (upp->args == NULL)
+		return -1;
+
+	/*
+	 * Construct an array to allow accessing native args by index.
+	 */
+	if ((nargv = dt_calloc(dtp, psp->pps_nargc, sizeof (char *))) == NULL)
+		goto fail;
+
+	for (i = 0; i < psp->pps_nargc; i++, nptr += strlen(nptr) + 1)
+		nargv[i] = nptr;
+
+	/*
+	 * Fill up the upp->args array based on xargs.  If this indicates that
+	 * mapping is needed, note as much.
+	 */
+	for (i = 0; i < upp->argc; i++, xptr += strlen(xptr) + 1) {
+		int map_arg = psp->pps_argmap[i];
+
+		upp->args[i].native = nargv[map_arg];
+		upp->args[i].xlate = xptr;
+		upp->args[i].mapping = map_arg;
+		upp->args[i].flags = 0;
+
+                if (i != map_arg)
+			upp->flags |= PP_IS_MAPPED;
+	}
+
+	free(nargv);
+	return 0;
+
+ fail:
+	free(nargv);
+	return -1;
+}
+
 /*
  * Look up or create an underlying (real) probe, corresponding directly to a
  * uprobe.  Since multiple pid and USDT probes may all map onto the same
@@ -530,7 +611,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 	char			prb[DTRACE_NAMELEN];
 	dtrace_probedesc_t	pd;
 	dt_probe_t		*uprp;
-	dt_uprobe_t		*upp;
+	dt_uprobe_t		*upp = NULL;
 
 	/*
 	 * The underlying probes (uprobes) represent the tracepoints that pid
@@ -555,6 +636,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 	case DTPPT_IS_ENABLED:
 	case DTPPT_ENTRY:
 	case DTPPT_OFFSETS:
+	case DTPPT_USDT:
 		snprintf(prb, sizeof(prb), "%lx", psp->pps_off);
 		break;
 	default:
@@ -568,6 +650,8 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 	pd.fun = psp->pps_fun;
 	pd.prb = prb;
 
+	dt_dprintf("Providing underlying probe %s:%s:%s:%s @ %lx\n", psp->pps_prv,
+		   psp->pps_mod, psp->pps_fn, psp->pps_prb, psp->pps_off);
 	uprp = dt_probe_lookup(dtp, &pd);
 	if (uprp == NULL) {
 		dt_provider_t	*pvp;
@@ -591,12 +675,24 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 			goto fail;
 
 		uprp = dt_probe_insert(dtp, pvp, pd.prv, pd.mod, pd.fun, pd.prb,
-				      upp);
+				       upp);
 		if (uprp == NULL)
 			goto fail;
 	} else
 		upp = uprp->prv_data;
 
+	/*
+	 * Only one USDT probe can correspond to each underlying probe.
+	 */
+	if (psp->pps_type == DTPPT_USDT && upp->flags == PP_IS_USDT) {
+		dt_dprintf("Found overlapping USDT probe at %lx/%lx/%lx/%s\n",
+			   upp->dev, upp->inum, upp->off, upp->fn);
+		goto fail;
+	}
+
+	if (populate_args(dtp, psp, upp) < 0)
+		goto fail;
+
 	switch (psp->pps_type) {
 	case DTPPT_RETURN:
 	    upp->flags |= PP_IS_RETURN;
@@ -604,15 +700,20 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 	case DTPPT_IS_ENABLED:
 	    upp->flags |= PP_IS_ENABLED;
 	    break;
+	case DTPPT_USDT:
+	    upp->flags |= PP_IS_USDT;
+	    break;
 	default: ;
 	    /*
 	     * No flags needed for other types.
 	     */
 	}
 
-	return uprp;
+        return uprp;
 
 fail:
+	dt_dprintf("Failed to instantiate %s:%s:%s:%s\n", psp->pps_prv,
+		   psp->pps_mod, psp->pps_fn, psp->pps_prb);
 	probe_destroy(dtp, upp);
 	return NULL;
 }
@@ -732,7 +833,7 @@ static int provide_pid_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp)
 
 static int provide_usdt_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp)
 {
-	if (psp->pps_type != DTPPT_OFFSETS &&
+	if (psp->pps_type != DTPPT_USDT &&
 	    psp->pps_type != DTPPT_IS_ENABLED) {
 		dt_dprintf("pid: unknown USDT probe type %i\n", psp->pps_type);
 		return -1;
@@ -786,7 +887,13 @@ static void enable_usdt(dtrace_hdl_t *dtp, dt_probe_t *prp)
  *	int dt_uprobe(dt_pt_regs *regs)
  *
  * The trampoline will first populate a dt_dctx_t struct.  It will then emulate
- * the firing of all dependent pid* probes and their clauses.
+ * the firing of all dependent pid* and USDT probes and their clauses, or (in
+ * the case of is-enabled probes), do the necessary copying (is-enabled probes
+ * have no associated clauses and their behaviour is hardwired).
+ *
+ * Trampolines associated with USDT probes will also arrange for argument
+ * shuffling before the USDT clauses are invoked if the argmap array calls for
+ * it.
  */
 static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 {
@@ -864,7 +971,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 	}
 
 	/*
-	 * USDT
+	 * USDT.
 	 */
 
 	/* In some cases, we know there are no USDT probes. */  // FIXME: add more checks
@@ -873,6 +980,16 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 
 	dt_cg_tramp_copy_args_from_regs(pcb, 0);
 
+	/*
+	 * Apply arg mappings, if needed.
+	 */
+	if (upp->flags & PP_IS_MAPPED) {
+
+		/* dt_cg_tramp_map_args() works from the saved args. */
+		dt_cg_tramp_save_args(pcb);
+		dt_cg_tramp_map_args(pcb, upp->args, upp->argc);
+	}
+
 	/*
 	 * Retrieve the PID of the process that caused the probe to fire.
 	 */
@@ -1083,10 +1200,51 @@ attach_bpf:
 static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
 		      int *argcp, dt_argdesc_t **argvp)
 {
-	*argcp = 0;			/* no known arguments */
-	*argvp = NULL;
+	size_t		i = 0;
+	list_probe_t	*pup = prp->prv_data;
+	dt_uprobe_t	*upp;
+	size_t		argc = 0;
+	dt_argdesc_t	*argv = NULL;
+
+	/* No underlying probes?  No args.  */
+	if (!pup)
+		goto done;
+
+	upp = pup->probe->prv_data;
+	if (!upp || upp->args == NULL)
+		goto done;
+
+	argc = upp->argc;
+	argv = dt_calloc(dtp, argc, sizeof(dt_argdesc_t));
+	if (argv == NULL)
+		return dt_set_errno(dtp, EDT_NOMEM);
+
+	for (i = 0; i < argc; i++) {
+		argv[i].native = strdup(upp->args[i].native);
+		if (upp->args[i].xlate)
+			argv[i].xlate = strdup(upp->args[i].xlate);
+		argv[i].mapping = i;
+
+		if (argv[i].native == NULL ||
+		    (upp->args[i].xlate != NULL && argv[i].xlate == NULL))
+			goto oom;
+	}
+
+done:
+	*argcp = argc;
+	*argvp = argv;
 
 	return 0;
+ oom:
+	size_t j;
+
+	for (j = 0; j <= i; j++) {
+		free((char *) argv[i].native);
+		free((char *) argv[i].xlate);
+	}
+
+        dt_free(dtp, argv);
+	return dt_set_errno(dtp, EDT_NOMEM);
 }
 
 /*
@@ -1152,7 +1310,6 @@ dt_provimpl_t	dt_uprobe = {
 	.load_prog	= &dt_bpf_prog_load,
 	.trampoline	= &trampoline,
 	.attach		= &attach,
-	.probe_info	= &probe_info,
 	.detach		= &detach,
 	.probe_destroy	= &probe_destroy_underlying,
 	.add_probe	= &add_probe_uprobe,
@@ -1178,6 +1335,7 @@ dt_provimpl_t	dt_usdt = {
 	.populate	= &populate_usdt,
 	.provide_probe	= &provide_usdt_probe,
 	.enable		= &enable_usdt,
+	.probe_info	= &probe_info,
 	.probe_destroy	= &probe_destroy,
 	.discover	= &discover,
 	.add_probe	= &add_probe_usdt,
diff --git a/test/triggers/usdt-tst-argmap-prov.d b/test/triggers/usdt-tst-argmap-prov.d
index 28134baa6fa3..d8c3e88c4616 100644
--- a/test/triggers/usdt-tst-argmap-prov.d
+++ b/test/triggers/usdt-tst-argmap-prov.d
@@ -1,10 +1,13 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 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.
  */
 
 provider test_prov {
 	probe place(int i, int j) : (int j, int i, int i, int j);
+	probe place2(int i, char *j) : (char *j, int i, int i, char *j);
+	probe place3(int i, char *j) : (char *j);
+	probe place4(int i, char *j) : ();
 };
diff --git a/test/triggers/usdt-tst-argmap.c b/test/triggers/usdt-tst-argmap.c
index 89a0a53fc1d5..9244092514ff 100644
--- a/test/triggers/usdt-tst-argmap.c
+++ b/test/triggers/usdt-tst-argmap.c
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 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.
  */
@@ -12,6 +12,9 @@ main(int argc, char **argv)
 {
 	for (;;) {
 		DTRACE_PROBE2(test_prov, place, 10, 4);
+		DTRACE_PROBE(test_prov, place2, 255, "foo");
+		DTRACE_PROBE(test_prov, place3, 126, "bar");
+		DTRACE_PROBE(test_prov, place4, 204, "baz");
 	}
 
 	return 0;
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
new file mode 100644
index 000000000000..d2e280e5f21c
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
@@ -0,0 +1,130 @@
+   ID   PROVIDER            MODULE                          FUNCTION NAME
+ XX test_provXXXX              test                              main go_vanishing
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		None
+
+ XX test_provXXXX              test                              main go_halved
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+
+ XX test_provXXXX              test                              main go_doubled
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+		args[2]: int
+		args[3]: char *
+
+ XX test_provXXXX              test                              main go
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+
+   ID   PROVIDER            MODULE                          FUNCTION NAME
+ XX test_provXXXX              test                              main go_vanishing
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		None
+
+ XX test_provXXXX              test                              main go_halved
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+
+ XX test_provXXXX              test                              main go_doubled
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+		args[2]: int
+		args[3]: char *
+
+ XX test_provXXXX              test                              main go
+
+	Probe Description Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Attributes
+		Identifier Names: Private
+		Data Semantics:   Private
+		Dependency Class: Unknown
+
+	Argument Types
+		args[0]: char *
+		args[1]: int
+
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
new file mode 100755
index 000000000000..c575983adf65
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r.p
@@ -0,0 +1,2 @@
+#!/bin/sed -rf
+s,test_prov[0-9]*,test_provXXXX,g; s,^ *[0-9]+, XX,g;
diff --git a/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
new file mode 100755
index 000000000000..b0a2345a6681
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
@@ -0,0 +1,89 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2013, 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.
+
+##
+#
+# ASSERTION:
+# Testing -lvn option with USDT probes with a valid probe name.
+#
+# SECTION: dtrace Utility/-ln Option
+#
+##
+
+if [ $# != 1 ]; then
+	echo expected one argument: '<'dtrace-path'>'
+	exit 2
+fi
+
+dtrace=$1
+CC=/usr/bin/gcc
+CFLAGS=
+
+DIRNAME="$tmpdir/list-probes-args-usdt.$$.$RANDOM"
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+cat > prov.d <<EOF
+provider test_prov {
+	probe go(int a, char *b) : (char *b, int a);
+	probe go_doubled(int a, char *b) : (char *b, int a, int a, char *b);
+	probe go_halved(int a, char *b) : (char *b);
+	probe go_vanishing(int a, char *b) : ();
+};
+EOF
+
+$dtrace -h -s prov.d
+if [ $? -ne 0 ]; then
+	echo "failed to generate header file" >& 2
+	exit 1
+fi
+
+cat > test.c <<EOF
+#include <sys/types.h>
+#include "prov.h"
+
+int
+main(int argc, char **argv)
+{
+	TEST_PROV_GO(666, "foo");
+	TEST_PROV_GO_DOUBLED(666, "foo");
+	TEST_PROV_GO_HALVED(666, "foo");
+	TEST_PROV_GO_VANISHING(666, "foo");
+	sleep(10);
+}
+EOF
+
+${CC} ${CFLAGS} -c test.c
+if [ $? -ne 0 ]; then
+	echo "failed to compile test.c" >& 2
+	exit 1
+fi
+$dtrace -G -s prov.d test.o
+if [ $? -ne 0 ]; then
+	echo "failed to create DOF" >& 2
+	exit 1
+fi
+${CC} ${CFLAGS} -o test test.o prov.o
+if [ $? -ne 0 ]; then
+	echo "failed to link final executable" >& 2
+	exit 1
+fi
+
+script()
+{
+	$dtrace -c ./test -lvn 'test_prov$target:::go*'
+	./test &
+	PID=$!
+	disown %+
+	$dtrace -p $PID -lvn 'test_prov$target:::go*'
+	kill -9 $PID
+}
+
+script
+status=$?
+
+exit $status
diff --git a/test/unittest/usdt/err.argmap-null.d b/test/unittest/usdt/err.argmap-null.d
new file mode 100644
index 000000000000..ba765bea7a04
--- /dev/null
+++ b/test/unittest/usdt/err.argmap-null.d
@@ -0,0 +1,41 @@
+/*
+ * 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.
+ */
+
+/* @@trigger: usdt-tst-argmap */
+/* @@trigger-timing: before */
+/* @@runtest-opts: $_pid */
+
+/*
+ * ASSERTION: Verify that a probe left with no args at all due to mapping
+ *            has no args.
+ */
+
+BEGIN
+{
+	/* Timeout after 5 seconds */
+	timeout = timestamp + 5000000000;
+}
+
+test_prov$1:::place4
+/args[0] != 204 || args[0] == 204/
+{
+	printf("this should never happen");
+	exit(1);
+}
+
+test_prov$1:::place4
+{
+	printf("nor should this");
+	exit(1);
+}
+
+profile:::tick-1
+/timestamp > timeout/
+{
+	trace("test timed out");
+	exit(1);
+}
diff --git a/test/unittest/usdt/err.argmap-null.r b/test/unittest/usdt/err.argmap-null.r
new file mode 100644
index 000000000000..215475e39b48
--- /dev/null
+++ b/test/unittest/usdt/err.argmap-null.r
@@ -0,0 +1,2 @@
+-- @@stderr --
+dtrace: failed to compile script test/unittest/usdt/err.argmap-null.d: line 24: index 0 is out of range for test_provXXXX:::place4 args[ ]
diff --git a/test/unittest/usdt/err.argmap-null.r.p b/test/unittest/usdt/err.argmap-null.r.p
new file mode 100755
index 000000000000..c575983adf65
--- /dev/null
+++ b/test/unittest/usdt/err.argmap-null.r.p
@@ -0,0 +1,2 @@
+#!/bin/sed -rf
+s,test_prov[0-9]*,test_provXXXX,g; s,^ *[0-9]+, XX,g;
diff --git a/test/unittest/usdt/tst.argmap-null.d b/test/unittest/usdt/tst.argmap-null.d
new file mode 100644
index 000000000000..dacf4c4f569a
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-null.d
@@ -0,0 +1,32 @@
+/*
+ * 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.
+ */
+
+/* @@trigger: usdt-tst-argmap */
+/* @@trigger-timing: before */
+/* @@runtest-opts: $_pid */
+
+/*
+ * ASSERTION: Verify that you can map to no args at all.
+ */
+
+BEGIN
+{
+	/* Timeout after 5 seconds */
+	timeout = timestamp + 5000000000;
+}
+
+test_prov$1:::place4
+{
+	exit(0);
+}
+
+profile:::tick-1
+/timestamp > timeout/
+{
+	trace("test timed out");
+	exit(1);
+}
diff --git a/test/unittest/usdt/tst.argmap-typed-partial.d b/test/unittest/usdt/tst.argmap-typed-partial.d
new file mode 100644
index 000000000000..8c4d7273c0ab
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-typed-partial.d
@@ -0,0 +1,49 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2007, 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.
+ */
+
+/* @@trigger: usdt-tst-argmap */
+/* @@trigger-timing: before */
+/* @@runtest-opts: $_pid */
+
+/*
+ * ASSERTION: Verify that args[N] variables are properly typed when mapped,
+ *            even if some args are unused.
+ */
+
+BEGIN
+{
+	/* Timeout after 5 seconds */
+	timeout = timestamp + 5000000000;
+}
+
+test_prov$1:::place3
+/stringof(args[0]) != "bar"/
+{
+	printf("arg is %s; should be \"bar\"",
+	    stringof(args[0]));
+	exit(1);
+}
+
+test_prov$1:::place3
+/stringof(copyinstr(arg0)) != "bar"/
+{
+	printf("arg is %s; should be \"bar\"",
+	    stringof(copyinstr(arg0)));
+	exit(1);
+}
+
+test_prov$1:::place3
+{
+	exit(0);
+}
+
+profile:::tick-1
+/timestamp > timeout/
+{
+	trace("test timed out");
+	exit(1);
+}
diff --git a/test/unittest/usdt/tst.argmap-typed.d b/test/unittest/usdt/tst.argmap-typed.d
new file mode 100644
index 000000000000..1243e059b8ae
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-typed.d
@@ -0,0 +1,48 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2007, 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.
+ */
+
+/* @@trigger: usdt-tst-argmap */
+/* @@trigger-timing: before */
+/* @@runtest-opts: $_pid */
+
+/*
+ * ASSERTION: Verify that args[N] variables are properly typed when mapped.
+ */
+
+BEGIN
+{
+	/* Timeout after 5 seconds */
+	timeout = timestamp + 5000000000;
+}
+
+test_prov$1:::place2
+/stringof(args[0]) != "foo" || args[1] != 255 || args[2] != 255 || stringof(args[3]) != "foo"/
+{
+	printf("args are %s, %d, %d, %s; should be \"foo\", 255, 255, \"foo\"",
+	    stringof(args[0]), args[1], args[2], stringof(args[3]));
+	exit(1);
+}
+
+test_prov$1:::place2
+/stringof(copyinstr(arg0)) != "foo" || arg1 != 255 || arg2 != 255 || stringof(copyinstr(arg3)) != "foo"/
+{
+	printf("args are %s, %d, %d, %s; should be \"foo\", 255, 255, \"foo\"",
+	    stringof(copyinstr(arg0)), arg1, arg2, stringof(copyinstr(arg3)));
+	exit(1);
+}
+
+test_prov$1:::place2
+{
+	exit(0);
+}
+
+profile:::tick-1
+/timestamp > timeout/
+{
+	trace("test timed out");
+	exit(1);
+}
diff --git a/test/unittest/usdt/tst.argmap.d b/test/unittest/usdt/tst.argmap.d
index 7896dc07e0e2..a7d68da3dfbc 100644
--- a/test/unittest/usdt/tst.argmap.d
+++ b/test/unittest/usdt/tst.argmap.d
@@ -1,17 +1,16 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 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.
  */
 
-/* @@xfail: dtv2 */
 /* @@trigger: usdt-tst-argmap */
 /* @@trigger-timing: before */
 /* @@runtest-opts: $_pid */
 
 /*
- * ASSERTION: Verify that argN and args[N] variables are properly remapped.
+ * ASSERTION: Verify that argN and args[N] variables are properly mapped.
  */
 
 BEGIN
-- 
2.46.0.278.g36e3a12567

[PATCH v7 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Nick Alcock]

This generalizes the existing tst.pidprobes.sh to check the args
reported by the probe both with and without arg mapping in place.

Everything seems to work.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 .../usdt/{tst.pidprobes.sh => pidprobes.sh}   | 56 +++++++++++++++----
 test/unittest/usdt/tst.pidargmap.sh           | 11 ++++
 test/unittest/usdt/tst.pidargs.sh             | 11 ++++
 3 files changed, 67 insertions(+), 11 deletions(-)
 rename test/unittest/usdt/{tst.pidprobes.sh => pidprobes.sh} (82%)
 create mode 100755 test/unittest/usdt/tst.pidargmap.sh
 create mode 100755 test/unittest/usdt/tst.pidargs.sh

diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/pidprobes.sh
similarity index 82%
rename from test/unittest/usdt/tst.pidprobes.sh
rename to test/unittest/usdt/pidprobes.sh
index 25fee85537fc..aedc6256f195 100755
--- a/test/unittest/usdt/tst.pidprobes.sh
+++ b/test/unittest/usdt/pidprobes.sh
@@ -5,9 +5,12 @@
 # Licensed under the Universal Permissive License v 1.0 as shown at
 # http://oss.oracle.com/licenses/upl.
 #
-# This test verifies that USDT and pid probes can share underlying probes.
+# This test verifies various properties of USDT and pid probes sharing
+# underlying probes.
 
 dtrace=$1
+usdt=$2
+mapping=$3
 
 # Set up test directory.
 
@@ -17,13 +20,21 @@ cd $DIRNAME
 
 # Create test source files.
 
-cat > prov.d <<EOF
+if [[ -z $mapping ]]; then
+    cat > prov.d <<EOF
 provider pyramid {
 	probe entry(int, char, int, int);
 };
 EOF
+else
+    cat > prov.d <<EOF
+provider pyramid {
+	probe entry(int a, char b, int c, int d) : (int c, int d, int a, char b);
+};
+EOF
+fi
 
-cat > main.c <<EOF
+cat > main.c <<'EOF'
 #include <stdio.h>
 #include "prov.h"
 
@@ -48,7 +59,7 @@ EOF
 
 # Build the test program.
 
-$dtrace -h -s prov.d
+$dtrace $dt_flags -h -s prov.d
 if [ $? -ne 0 ]; then
 	echo "failed to generate header file" >&2
 	exit 1
@@ -61,12 +72,12 @@ fi
 if [[ `uname -m` = "aarch64" ]]; then
 	objdump -d main.o > disasm_foo.txt.before
 fi
-$dtrace -G -64 -s prov.d main.o
+$dtrace $dt_flags -G -64 -s prov.d main.o
 if [ $? -ne 0 ]; then
 	echo "failed to create DOF" >&2
 	exit 1
 fi
-cc $test_cppflags -o main main.o prov.o
+cc $test_ldflags -o main main.o prov.o
 if [ $? -ne 0 ]; then
 	echo "failed to link final executable" >&2
 	exit 1
@@ -75,7 +86,7 @@ fi
 # Check that the program output is 0 when the USDT probe is not enabled.
 # That is, the PYRAMID_ENTRY_ENABLED() is-enabled checks should not pass.
 
-./main > main.out
+./main standalone > main.out
 echo "my result: 0" > main.out.expected
 if ! diff -q main.out main.out.expected > /dev/null; then
 	echo '"my result"' looks wrong when not using DTrace
@@ -88,11 +99,25 @@ fi
 
 # Run dtrace.
 
-$dtrace $dt_flags -q -c ./main -o dtrace.out -n '
+cat >> pidprobes.d <<'EOF'
 p*d$target::foo:
 {
 	printf("%d %s:%s:%s:%s %x\n", pid, probeprov, probemod, probefunc, probename, uregs[R_PC]);
-}' > main.out2
+}
+EOF
+
+if [[ -n $usdt ]]; then
+	echo 'pyramid$target::foo: {' >> pidprobes.d
+
+	if [[ -n $mapping ]]; then
+		echo 'printf("%d %s:%s:%s:%s %i %i %i %c\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
+	else
+		echo 'printf("%d %s:%s:%s:%s %i %c %i %i\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
+	fi
+	echo '}' >> pidprobes.d
+fi
+
+$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 main.out2
@@ -106,7 +131,7 @@ fi
 echo "my result: 10" > main.out2.expected
 
 if ! diff -q main.out2 main.out2.expected > /dev/null; then
-	echo '"my result"' looks wrong when using DTrace
+	echo '"my result"' looks wrong
 	echo === got ===
 	cat main.out2
 	echo === expected ===
@@ -262,8 +287,17 @@ for pc in $pcs; do
 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
+if [[ -n $usdt ]]; then
+	if [[ -z $mapping ]]; then
+		echo "$pid pyramid$pid:main:foo:entry 2 a 16 128" >> dtrace.out.expected
+		echo "$pid pyramid$pid:main:foo:entry 4 b 32 256" >> dtrace.out.expected
+	else
+		echo "$pid pyramid$pid:main:foo:entry 16 128 2 a" >> dtrace.out.expected
+		echo "$pid pyramid$pid:main:foo:entry 32 256 4 b" >> dtrace.out.expected
+	fi
+fi
 
-# Sort and check.
+# Sort and check (dropping any wake-up firings from deferred probing).
 
 sort dtrace.out          > dtrace.out.sorted
 sort dtrace.out.expected > dtrace.out.expected.sorted
diff --git a/test/unittest/usdt/tst.pidargmap.sh b/test/unittest/usdt/tst.pidargmap.sh
new file mode 100755
index 000000000000..0c83f8703539
--- /dev/null
+++ b/test/unittest/usdt/tst.pidargmap.sh
@@ -0,0 +1,11 @@
+#!/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.
+#
+# This test verifies that USDT and pid probes that share underlying probes
+# do not apply arg mappings (incorrectly) to the pid probes.
+
+exec $(dirname $_test)/pidprobes.sh $1 t t
diff --git a/test/unittest/usdt/tst.pidargs.sh b/test/unittest/usdt/tst.pidargs.sh
new file mode 100755
index 000000000000..53cba7c39624
--- /dev/null
+++ b/test/unittest/usdt/tst.pidargs.sh
@@ -0,0 +1,11 @@
+#!/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.
+#
+# This test verifies that USDT and pid probes that share underlying probes
+# get the arguments correct for the USDT probes.
+
+exec $(dirname $_test)/pidprobes.sh $1 t ""
-- 
2.46.0.278.g36e3a12567

[PATCH v7 6/6] usdt: fix create_underlying error path

[Nick Alcock]

On error, we were destroying the underlying probe using the wrong call,
as if it were an overlying probe.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_prov_uprobe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 1a1500c03ecc..205014617586 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -714,7 +714,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 fail:
 	dt_dprintf("Failed to instantiate %s:%s:%s:%s\n", psp->pps_prv,
 		   psp->pps_mod, psp->pps_fn, psp->pps_prb);
-	probe_destroy(dtp, upp);
+	probe_destroy_underlying(dtp, upp);
 	return NULL;
 }
 
-- 
2.46.0.278.g36e3a12567

Re: [PATCH v7 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Eugene Loh]

On 11/6/24 06:29, Nick Alcock wrote:

> This generalizes the existing tst.pidprobes.sh to check the args
> reported by the probe both with and without arg mapping in place.
>
> Everything seems to work.

That comment seems to be unnecessary.

Also, is there a "dtrace -lv" test for USDT args?  (I'm not even sure if 
that question makes sense.)

> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
> ---
>   .../usdt/{tst.pidprobes.sh => pidprobes.sh}   | 56 +++++++++++++++----

Why the renaming?  And, will "./runtest.sh" run pidprobes.sh without any 
command-line args?

>   test/unittest/usdt/tst.pidargmap.sh           | 11 ++++
>   test/unittest/usdt/tst.pidargs.sh             | 11 ++++
>   3 files changed, 67 insertions(+), 11 deletions(-)
>   rename test/unittest/usdt/{tst.pidprobes.sh => pidprobes.sh} (82%)
>   create mode 100755 test/unittest/usdt/tst.pidargmap.sh
>   create mode 100755 test/unittest/usdt/tst.pidargs.sh
>
> diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/pidprobes.sh
> similarity index 82%
> rename from test/unittest/usdt/tst.pidprobes.sh
> rename to test/unittest/usdt/pidprobes.sh
> index 25fee85537fc..aedc6256f195 100755
> --- a/test/unittest/usdt/tst.pidprobes.sh
> +++ b/test/unittest/usdt/pidprobes.sh
> @@ -5,9 +5,12 @@
>   # Licensed under the Universal Permissive License v 1.0 as shown at
>   # http://oss.oracle.com/licenses/upl.
>   #
> -# This test verifies that USDT and pid probes can share underlying probes.
> +# This test verifies various properties of USDT and pid probes sharing
> +# underlying probes.
>   
>   dtrace=$1
> +usdt=$2

Why is there a usdt=$2?  It seems that usdt is always "t" and you'd 
expect that in test/unittest/usdt.

> +mapping=$3
>   
>   # Set up test directory.
>   
> @@ -17,13 +20,21 @@ cd $DIRNAME
>   
>   # Create test source files.
>   
> -cat > prov.d <<EOF
> +if [[ -z $mapping ]]; then
> +    cat > prov.d <<EOF
>   provider pyramid {
>   	probe entry(int, char, int, int);
>   };
>   EOF
> +else
> +    cat > prov.d <<EOF
> +provider pyramid {
> +	probe entry(int a, char b, int c, int d) : (int c, int d, int a, char b);
> +};
> +EOF
> +fi
>   
> -cat > main.c <<EOF
> +cat > main.c <<'EOF'
>   #include <stdio.h>
>   #include "prov.h"
>   
> @@ -48,7 +59,7 @@ EOF
>   
>   # Build the test program.
>   
> -$dtrace -h -s prov.d
> +$dtrace $dt_flags -h -s prov.d
>   if [ $? -ne 0 ]; then
>   	echo "failed to generate header file" >&2
>   	exit 1
> @@ -61,12 +72,12 @@ fi
>   if [[ `uname -m` = "aarch64" ]]; then
>   	objdump -d main.o > disasm_foo.txt.before
>   fi
> -$dtrace -G -64 -s prov.d main.o
> +$dtrace $dt_flags -G -64 -s prov.d main.o
>   if [ $? -ne 0 ]; then
>   	echo "failed to create DOF" >&2
>   	exit 1
>   fi
> -cc $test_cppflags -o main main.o prov.o
> +cc $test_ldflags -o main main.o prov.o
>   if [ $? -ne 0 ]; then
>   	echo "failed to link final executable" >&2
>   	exit 1
> @@ -75,7 +86,7 @@ fi
>   # Check that the program output is 0 when the USDT probe is not enabled.
>   # That is, the PYRAMID_ENTRY_ENABLED() is-enabled checks should not pass.
>   
> -./main > main.out
> +./main standalone > main.out

I'd vote against this change.  Introducing a command-line arg that is 
not used?  Or, add it as a comment?  But there is already a comment.

>   echo "my result: 0" > main.out.expected
>   if ! diff -q main.out main.out.expected > /dev/null; then
>   	echo '"my result"' looks wrong when not using DTrace
> @@ -88,11 +99,25 @@ fi
>   
>   # Run dtrace.
>   
> -$dtrace $dt_flags -q -c ./main -o dtrace.out -n '
> +cat >> pidprobes.d <<'EOF'
>   p*d$target::foo:
>   {
>   	printf("%d %s:%s:%s:%s %x\n", pid, probeprov, probemod, probefunc, probename, uregs[R_PC]);
> -}' > main.out2
> +}
> +EOF
> +
> +if [[ -n $usdt ]]; then
> +	echo 'pyramid$target::foo: {' >> pidprobes.d
> +
> +	if [[ -n $mapping ]]; then
> +		echo 'printf("%d %s:%s:%s:%s %i %i %i %c\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
> +	else
> +		echo 'printf("%d %s:%s:%s:%s %i %c %i %i\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
> +	fi
> +	echo '}' >> pidprobes.d
> +fi
> +
> +$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 main.out2
> @@ -106,7 +131,7 @@ fi
>   echo "my result: 10" > main.out2.expected
>   
>   if ! diff -q main.out2 main.out2.expected > /dev/null; then
> -	echo '"my result"' looks wrong when using DTrace
> +	echo '"my result"' looks wrong

Why?  In particular, "when using DTrace" stands in contrast to the 
earlier message 'echo '"my result"' looks wrong when not using DTrace'.

>   	echo === got ===
>   	cat main.out2
>   	echo === expected ===
> @@ -262,8 +287,17 @@ for pc in $pcs; do
>   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
> +if [[ -n $usdt ]]; then
> +	if [[ -z $mapping ]]; then
> +		echo "$pid pyramid$pid:main:foo:entry 2 a 16 128" >> dtrace.out.expected
> +		echo "$pid pyramid$pid:main:foo:entry 4 b 32 256" >> dtrace.out.expected
> +	else
> +		echo "$pid pyramid$pid:main:foo:entry 16 128 2 a" >> dtrace.out.expected
> +		echo "$pid pyramid$pid:main:foo:entry 32 256 4 b" >> dtrace.out.expected
> +	fi
> +fi
>   
> -# Sort and check.
> +# Sort and check (dropping any wake-up firings from deferred probing).

I guess I do not get this.  Where does any "dropping" take place?

>   
>   sort dtrace.out          > dtrace.out.sorted
>   sort dtrace.out.expected > dtrace.out.expected.sorted
> diff --git a/test/unittest/usdt/tst.pidargmap.sh b/test/unittest/usdt/tst.pidargmap.sh
> new file mode 100755
> index 000000000000..0c83f8703539
> --- /dev/null
> +++ b/test/unittest/usdt/tst.pidargmap.sh
> @@ -0,0 +1,11 @@
> +#!/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.
> +#
> +# This test verifies that USDT and pid probes that share underlying probes
> +# do not apply arg mappings (incorrectly) to the pid probes.

How do we verify that mappings do not apply (incorrectly) to pid 
probes?  Is there supposed to be a check on pid probe args?

> +
> +exec $(dirname $_test)/pidprobes.sh $1 t t
> diff --git a/test/unittest/usdt/tst.pidargs.sh b/test/unittest/usdt/tst.pidargs.sh
> new file mode 100755
> index 000000000000..53cba7c39624
> --- /dev/null
> +++ b/test/unittest/usdt/tst.pidargs.sh
> @@ -0,0 +1,11 @@
> +#!/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.
> +#
> +# This test verifies that USDT and pid probes that share underlying probes
> +# get the arguments correct for the USDT probes.
> +
> +exec $(dirname $_test)/pidprobes.sh $1 t ""

Re: [PATCH v7 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Nick Alcock]

On 7 Nov 2024, Eugene Loh uttered the following:

> On 11/6/24 06:29, Nick Alcock wrote:
>
>> This generalizes the existing tst.pidprobes.sh to check the args
>> reported by the probe both with and without arg mapping in place.
>>
>> Everything seems to work.
>
> That comment seems to be unnecessary.

I think it's worth noting that all this arg-mapping infrastructure
(mostly entirely untested for fifteen years) still works :)

> Also, is there a "dtrace -lv" test for USDT args?  (I'm not even sure if that question makes sense.)

Yep. See the previous patch. (Multiple, actually, Kris had me add a few
more.)

>> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
>> ---
>>   .../usdt/{tst.pidprobes.sh => pidprobes.sh}   | 56 +++++++++++++++----
>
> Why the renaming?  And, will "./runtest.sh" run pidprobes.sh without any command-line args?

Nope! It's a library now, called by tst.pid*sh, including, uh, the new
tst.pidprobes.sh I meant to keep around and call it. Oops!

>> --- a/test/unittest/usdt/tst.pidprobes.sh
>> +++ b/test/unittest/usdt/pidprobes.sh
>> @@ -5,9 +5,12 @@
>>   # Licensed under the Universal Permissive License v 1.0 as shown at
>>   # http://oss.oracle.com/licenses/upl.
>>   #
>> -# This test verifies that USDT and pid probes can share underlying probes.
>> +# This test verifies various properties of USDT and pid probes sharing
>> +# underlying probes.
>>     dtrace=$1
>> +usdt=$2
>
> Why is there a usdt=$2?  It seems that usdt is always "t" and you'd expect that in test/unittest/usdt.

Oh crap. Sorry sorry this is a bug -- I lost the tst.pidprobes.sh I
meant to add back. Will fix and repush this patch.

-- which reads, in entirety

exec $(dirname $_test)/pidprobes.sh $1 "" ""

>> @@ -75,7 +86,7 @@ fi
>>   # Check that the program output is 0 when the USDT probe is not enabled.
>>   # That is, the PYRAMID_ENTRY_ENABLED() is-enabled checks should not pass.
>>   -./main > main.out
>> +./main standalone > main.out
>
> I'd vote against this change.  Introducing a command-line arg that is not used?  Or, add it as a comment?  But there is already a
> comment.

I... think that was fallout from my attempt to add deferred support that
I forgot to clean out again. Dropped.

>>   echo "my result: 10" > main.out2.expected
>>     if ! diff -q main.out2 main.out2.expected > /dev/null; then
>> -	echo '"my result"' looks wrong when using DTrace
>> +	echo '"my result"' looks wrong
>
> Why?  In particular, "when using DTrace" stands in contrast to the earlier message 'echo '"my result"' looks wrong when not using
> DTrace'.

Sorry! I didn't notice that the previous stanza existed and thought
"well of course we're using DTrace, that's redundant". Dropped.

>>   -# Sort and check.
>> +# Sort and check (dropping any wake-up firings from deferred probing).
>
> I guess I do not get this.  Where does any "dropping" take place?

In the deferred patch that caused massive ructions here and then I tried
to drop by hand very late at night and clearly didn't do as good a job
at as I thought I did. Dropped this reference to, uh, dropping.

>>     sort dtrace.out          > dtrace.out.sorted
>>   sort dtrace.out.expected > dtrace.out.expected.sorted
>> diff --git a/test/unittest/usdt/tst.pidargmap.sh b/test/unittest/usdt/tst.pidargmap.sh
>> new file mode 100755
>> index 000000000000..0c83f8703539
>> --- /dev/null
>> +++ b/test/unittest/usdt/tst.pidargmap.sh
>> @@ -0,0 +1,11 @@
>> +#!/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.
>> +#
>> +# This test verifies that USDT and pid probes that share underlying probes
>> +# do not apply arg mappings (incorrectly) to the pid probes.
>
> How do we verify that mappings do not apply (incorrectly) to pid probes?  Is there supposed to be a check on pid probe args?

Yeah, that's back now, apologies :) verified that it still works, too.
Will send as a followup to this reply in a mo.

-- 
NULL && (void)

[PATCH v8 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Nick Alcock]

This generalizes the existing tst.pidprobes.sh to check the args
reported by the probe both with and without arg mapping in place.

Everything seems to work.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
 .../usdt/{tst.pidprobes.sh => pidprobes.sh}   |  50 +++-
 test/unittest/usdt/tst.pidargmap.sh           |  11 +
 test/unittest/usdt/tst.pidargs.sh             |  11 +
 test/unittest/usdt/tst.pidprobes.sh           | 275 +-----------------
 4 files changed, 65 insertions(+), 282 deletions(-)
 copy test/unittest/usdt/{tst.pidprobes.sh => pidprobes.sh} (84%)
 create mode 100755 test/unittest/usdt/tst.pidargmap.sh
 create mode 100755 test/unittest/usdt/tst.pidargs.sh

diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/pidprobes.sh
similarity index 84%
copy from test/unittest/usdt/tst.pidprobes.sh
copy to test/unittest/usdt/pidprobes.sh
index 25fee85537fc..0840805b70fd 100755
--- a/test/unittest/usdt/tst.pidprobes.sh
+++ b/test/unittest/usdt/pidprobes.sh
@@ -5,9 +5,12 @@
 # Licensed under the Universal Permissive License v 1.0 as shown at
 # http://oss.oracle.com/licenses/upl.
 #
-# This test verifies that USDT and pid probes can share underlying probes.
+# This test verifies various properties of USDT and pid probes sharing
+# underlying probes.
 
 dtrace=$1
+usdt=$2
+mapping=$3
 
 # Set up test directory.
 
@@ -17,13 +20,21 @@ cd $DIRNAME
 
 # Create test source files.
 
-cat > prov.d <<EOF
+if [[ -z $mapping ]]; then
+    cat > prov.d <<EOF
 provider pyramid {
 	probe entry(int, char, int, int);
 };
 EOF
+else
+    cat > prov.d <<EOF
+provider pyramid {
+	probe entry(int a, char b, int c, int d) : (int c, int d, int a, char b);
+};
+EOF
+fi
 
-cat > main.c <<EOF
+cat > main.c <<'EOF'
 #include <stdio.h>
 #include "prov.h"
 
@@ -48,7 +59,7 @@ EOF
 
 # Build the test program.
 
-$dtrace -h -s prov.d
+$dtrace $dt_flags -h -s prov.d
 if [ $? -ne 0 ]; then
 	echo "failed to generate header file" >&2
 	exit 1
@@ -61,12 +72,12 @@ fi
 if [[ `uname -m` = "aarch64" ]]; then
 	objdump -d main.o > disasm_foo.txt.before
 fi
-$dtrace -G -64 -s prov.d main.o
+$dtrace $dt_flags -G -64 -s prov.d main.o
 if [ $? -ne 0 ]; then
 	echo "failed to create DOF" >&2
 	exit 1
 fi
-cc $test_cppflags -o main main.o prov.o
+cc $test_ldflags -o main main.o prov.o
 if [ $? -ne 0 ]; then
 	echo "failed to link final executable" >&2
 	exit 1
@@ -88,11 +99,25 @@ fi
 
 # Run dtrace.
 
-$dtrace $dt_flags -q -c ./main -o dtrace.out -n '
+cat >> pidprobes.d <<'EOF'
 p*d$target::foo:
 {
 	printf("%d %s:%s:%s:%s %x\n", pid, probeprov, probemod, probefunc, probename, uregs[R_PC]);
-}' > main.out2
+}
+EOF
+
+if [[ -n $usdt ]]; then
+	echo 'pyramid$target::foo: {' >> pidprobes.d
+
+	if [[ -n $mapping ]]; then
+		echo 'printf("%d %s:%s:%s:%s %i %i %i %c\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
+	else
+		echo 'printf("%d %s:%s:%s:%s %i %c %i %i\n", pid, probeprov, probemod, probefunc, probename, args[0], args[1], args[2], args[3]);' >> pidprobes.d
+	fi
+	echo '}' >> pidprobes.d
+fi
+
+$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 main.out2
@@ -262,6 +287,15 @@ for pc in $pcs; do
 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
+if [[ -n $usdt ]]; then
+	if [[ -z $mapping ]]; then
+		echo "$pid pyramid$pid:main:foo:entry 2 a 16 128" >> dtrace.out.expected
+		echo "$pid pyramid$pid:main:foo:entry 4 b 32 256" >> dtrace.out.expected
+	else
+		echo "$pid pyramid$pid:main:foo:entry 16 128 2 a" >> dtrace.out.expected
+		echo "$pid pyramid$pid:main:foo:entry 32 256 4 b" >> dtrace.out.expected
+	fi
+fi
 
 # Sort and check.
 
diff --git a/test/unittest/usdt/tst.pidargmap.sh b/test/unittest/usdt/tst.pidargmap.sh
new file mode 100755
index 000000000000..0c83f8703539
--- /dev/null
+++ b/test/unittest/usdt/tst.pidargmap.sh
@@ -0,0 +1,11 @@
+#!/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.
+#
+# This test verifies that USDT and pid probes that share underlying probes
+# do not apply arg mappings (incorrectly) to the pid probes.
+
+exec $(dirname $_test)/pidprobes.sh $1 t t
diff --git a/test/unittest/usdt/tst.pidargs.sh b/test/unittest/usdt/tst.pidargs.sh
new file mode 100755
index 000000000000..53cba7c39624
--- /dev/null
+++ b/test/unittest/usdt/tst.pidargs.sh
@@ -0,0 +1,11 @@
+#!/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.
+#
+# This test verifies that USDT and pid probes that share underlying probes
+# get the arguments correct for the USDT probes.
+
+exec $(dirname $_test)/pidprobes.sh $1 t ""
diff --git a/test/unittest/usdt/tst.pidprobes.sh b/test/unittest/usdt/tst.pidprobes.sh
index 25fee85537fc..e63d9f5dc096 100755
--- a/test/unittest/usdt/tst.pidprobes.sh
+++ b/test/unittest/usdt/tst.pidprobes.sh
@@ -7,277 +7,4 @@
 #
 # This test verifies that USDT and pid probes can share underlying probes.
 
-dtrace=$1
-
-# Set up test directory.
-
-DIRNAME=$tmpdir/pidprobes.$$.$RANDOM
-mkdir -p $DIRNAME
-cd $DIRNAME
-
-# Create test source files.
-
-cat > prov.d <<EOF
-provider pyramid {
-	probe entry(int, char, int, int);
-};
-EOF
-
-cat > main.c <<EOF
-#include <stdio.h>
-#include "prov.h"
-
-void foo() {
-	int n = 0;
-
-	PYRAMID_ENTRY(2, 'a', 16, 128);
-	if (PYRAMID_ENTRY_ENABLED())
-		n += 2;
-	PYRAMID_ENTRY(4, 'b', 32, 256);
-	if (PYRAMID_ENTRY_ENABLED())
-		n += 8;
-	printf("my result: %d\n", n);
-}
-
-int
-main(int argc, char **argv)
-{
-	foo();
-}
-EOF
-
-# Build the test program.
-
-$dtrace -h -s prov.d
-if [ $? -ne 0 ]; then
-	echo "failed to generate header file" >&2
-	exit 1
-fi
-cc $test_cppflags -c main.c
-if [ $? -ne 0 ]; then
-	echo "failed to compile test" >&2
-	exit 1
-fi
-if [[ `uname -m` = "aarch64" ]]; then
-	objdump -d main.o > disasm_foo.txt.before
-fi
-$dtrace -G -64 -s prov.d main.o
-if [ $? -ne 0 ]; then
-	echo "failed to create DOF" >&2
-	exit 1
-fi
-cc $test_cppflags -o main main.o prov.o
-if [ $? -ne 0 ]; then
-	echo "failed to link final executable" >&2
-	exit 1
-fi
-
-# Check that the program output is 0 when the USDT probe is not enabled.
-# That is, the PYRAMID_ENTRY_ENABLED() is-enabled checks should not pass.
-
-./main > main.out
-echo "my result: 0" > main.out.expected
-if ! diff -q main.out main.out.expected > /dev/null; then
-	echo '"my result"' looks wrong when not using DTrace
-	echo === got ===
-	cat main.out
-	echo === expected ===
-	cat main.out.expected
-	exit 1
-fi
-
-# Run dtrace.
-
-$dtrace $dt_flags -q -c ./main -o dtrace.out -n '
-p*d$target::foo:
-{
-	printf("%d %s:%s:%s:%s %x\n", pid, probeprov, probemod, probefunc, probename, uregs[R_PC]);
-}' > main.out2
-if [ $? -ne 0 ]; then
-	echo "failed to run dtrace" >&2
-	cat main.out2
-	cat dtrace.out
-	exit 1
-fi
-
-# Check that the program output is 10 when the USDT probe is enabled.
-# That is, the PYRAMID_ENTRY_ENABLED() is-enabled checks should pass.
-
-echo "my result: 10" > main.out2.expected
-
-if ! diff -q main.out2 main.out2.expected > /dev/null; then
-	echo '"my result"' looks wrong when using DTrace
-	echo === got ===
-	cat main.out2
-	echo === expected ===
-	cat main.out2.expected
-	exit 1
-fi
-
-# Get the reported pid.
-
-if [ `awk 'NF != 0 { print $1 }' dtrace.out | uniq | wc -l` -ne 1 ]; then
-	echo no unique pid
-	cat dtrace.out
-	exit 1
-fi
-pid=`awk 'NF != 0 { print $1 }' dtrace.out | uniq`
-
-# 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
-
-# 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.
-
-if [[ `uname -m` = "x86_64" ]]; then
-
-	# It is the first of five nop instructions in a row.
-	# So track pc[-6], pc[-5], pc[-4], pc[-3], pc[-2], pc[-1], pc[0]
-	# as well as whether they are nop.
-
-	usdt_pcs_all=`awk '
-	BEGIN {
-		pc6 = -1; is_nop6 = 0;
-		pc5 = -1; is_nop5 = 0;
-		pc4 = -1; is_nop4 = 0;
-		pc3 = -1; is_nop3 = 0;
-		pc2 = -1; is_nop2 = 0;
-		pc1 = -1; is_nop1 = 0;
-	}
-	{
-		# pc0 is current instruction
-		pc0 = strtonum("0x"$1);
-
-		# decide whether it is a nop
-		is_nop0 = 0;
-		if (NF == 3 &&
-		    $2 == "90" &&
-		    $3 == "nop")
-			is_nop0 = 1;
-
-		# report if pc[-5] is a USDT instruction
-		if (is_nop6 == 0 &&
-		    is_nop5 == 1 &&
-		    is_nop4 == 1 &&
-		    is_nop3 == 1 &&
-		    is_nop2 == 1 &&
-		    is_nop1 == 1 &&
-		    is_nop0 == 0)
-			print pc5;
-
-		# prepare advance to next instruction
-		pc6 = pc5;  is_nop6 = is_nop5;
-		pc5 = pc4;  is_nop5 = is_nop4;
-		pc4 = pc3;  is_nop4 = is_nop3;
-		pc3 = pc2;  is_nop3 = is_nop2;
-		pc2 = pc1;  is_nop2 = is_nop1;
-		pc1 = pc0;  is_nop1 = is_nop0;
-	}' disasm_foo.txt`
-
-	# We expect 4 USDT probes (2 USDT and 2 is-enabled).
-	if [ `echo $usdt_pcs_all | awk '{print NF}'` -ne 4 ]; then
-		echo ERROR: expected 4 USDT probes but got $usdt_pcs_all
-		cat disasm_foo.txt
-		exit 1
-	fi
-
-	# Separate them into regular and is-enabled PCs.
-	# We assume they alternate.
-	usdt_pcs=`echo $usdt_pcs_all | awk '{ print $1, $3 }'`
-	usdt_pcs_isenabled=`echo $usdt_pcs_all | awk '{ print $2, $4 }'`
-
-elif [[ `uname -m` = "aarch64" ]]; then
-
-	# The initial compilation of foo() makes it obvious where the
-	# USDT probes are.  We just have to add the function offset in.
-	usdt_pcs=`awk '/<__dtrace_pyramid___entry>/ { print strtonum("0x"$1) + '$pc0' }' disasm_foo.txt.before`
-	usdt_pcs_isenabled=`awk '/<__dtraceenabled_pyramid___entry>/ { print strtonum("0x"$1) + '$pc0' }' disasm_foo.txt.before`
-
-	# We expect 4 USDT probes (2 USDT and 2 is-enabled).
-	if [ `echo $usdt_pcs | awk '{print NF}'` -ne 2 -o \
-	     `echo $usdt_pcs_isenabled | awk '{print NF}'` -ne 2 ]; then
-		echo ERROR: expected 4 USDT probes but got $usdt_pcs and $usdt_pcs_isenabled
-		cat disasm_foo.txt.before
-		exit 1
-	fi
-
-else
-	echo ERROR unrecognized machine hardware name
-	exit 1
-fi
-
-# We expect all of the USDT probe PCs to be among the PCs in objdump output.
-
-for pc in $usdt_pcs $usdt_pcs_isenabled; do
-	if echo $pcs | grep -q -vw $pc ; then
-		echo ERROR: cannot find USDT PC $pc in $pcs
-		exit 1
-	fi
-done
-
-# Get the PC for the pid return probe.  (Just keep it in hex.)
-
-pc_return=`awk '/'$pid' pid'$pid':main:foo:return/ { print $NF }' dtrace.out`
-
-objdump -d main | awk '
-/^[0-9a-f]* <.*>:$/ { myfunc = $NF }         # enter a new function
-/^ *'$pc_return'/ { print myfunc; exit(0) }  # report the function $pc_return is in
-' > return_func.out
-
-echo "<main>:" > return_func.out.expected    # since we use uretprobe for pid return probes, the PC will be in the caller
-
-if ! diff -q return_func.out return_func.out.expected > /dev/null; then
-	echo ERROR: return PC looks to be in the wrong function
-	echo === got ===
-	cat return_func.out
-	echo === expected ===
-	cat return_func.out.expected
-	exit 1
-fi
-
-# Build up a list of expected dtrace output:
-# - a blank line
-# - pid entry
-# - pid return
-# - pid offset
-# - two USDT probes (ignore is-enabled probes)
-
-echo > 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
-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
-
-# Sort and check.
-
-sort dtrace.out          > dtrace.out.sorted
-sort dtrace.out.expected > dtrace.out.expected.sorted
-
-if ! diff -q dtrace.out.sorted dtrace.out.expected.sorted ; then
-	echo ERROR: dtrace output looks wrong
-	echo === got ===
-	cat dtrace.out.sorted
-	echo === expected ===
-	cat dtrace.out.expected.sorted
-	echo === diff ===
-	diff dtrace.out.sorted dtrace.out.expected.sorted
-	exit 1
-fi
-
-echo success
-exit 0
+exec $(dirname $_test)/pidprobes.sh $1 "" ""
-- 
2.46.0.278.g36e3a12567

Re: [PATCH v7 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Eugene Loh]

On 11/8/24 08:13, Nick Alcock wrote:

> On 7 Nov 2024, Eugene Loh uttered the following:
>
>> On 11/6/24 06:29, Nick Alcock wrote:
>>> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
>>> ---
>>>    .../usdt/{tst.pidprobes.sh => pidprobes.sh}   | 56 +++++++++++++++----
>> Why the renaming?  And, will "./runtest.sh" run pidprobes.sh without any command-line args?
> Nope! It's a library now, called by tst.pid*sh, including, uh, the new
> tst.pidprobes.sh I meant to keep around and call it. Oops!

"Nope"?  It seems to me that runtest.sh does indeed call pidprobes.sh 
without any command-line args.  It looks in test/unittest/usdt and sees 
a .sh file and runs it.  The fact that the .sh does not start with 
"tst." is irrelevant.  Indeed, we have lots of tests that do not start 
with "tst." (nor "err." nor "drp." etc.).  So if pidprobes.sh is not 
supposed to be run by runtest.sh directly, it has to be hidden better 
than simply by stripping tst. from its name.

I suppose there is no harm in runtest.sh running pidprobes.sh directly 
(with no command-line arguments) except generations from now someone 
will be wondering what it's supposed to do.

Incidentally, there is an orphaned tst.pidprobes.r file.
>>> --- a/test/unittest/usdt/tst.pidprobes.sh
>>> +++ b/test/unittest/usdt/pidprobes.sh
>>> @@ -5,9 +5,12 @@
>>>    # Licensed under the Universal Permissive License v 1.0 as shown at
>>>    # http://oss.oracle.com/licenses/upl.
>>>    #
>>> -# This test verifies that USDT and pid probes can share underlying probes.
>>> +# This test verifies various properties of USDT and pid probes sharing
>>> +# underlying probes.
>>>      dtrace=$1
>>> +usdt=$2
>> Why is there a usdt=$2?  It seems that usdt is always "t" and you'd expect that in test/unittest/usdt.
> Oh crap. Sorry sorry this is a bug -- I lost the tst.pidprobes.sh I
> meant to add back. Will fix and repush this patch.

The patch has perhaps already landed.  So, a delta patch might be needed.

> -- which reads, in entirety
>
> exec $(dirname $_test)/pidprobes.sh $1 "" ""

Or, just let runtest.sh call it directly, as it does anyhow.

Re: [PATCH v7 5/6] usdt: new tests for USDT arg sanity with overlapping pid probes

[Nick Alcock]

On 8 Nov 2024, Eugene Loh outgrape:

> On 11/8/24 08:13, Nick Alcock wrote:
>
>> On 7 Nov 2024, Eugene Loh uttered the following:
>>
>>> On 11/6/24 06:29, Nick Alcock wrote:
>>>> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
>>>> ---
>>>>    .../usdt/{tst.pidprobes.sh => pidprobes.sh}   | 56 +++++++++++++++----
>>> Why the renaming?  And, will "./runtest.sh" run pidprobes.sh without any command-line args?
>> Nope! It's a library now, called by tst.pid*sh, including, uh, the new
>> tst.pidprobes.sh I meant to keep around and call it. Oops!
>
> "Nope"?  It seems to me that runtest.sh does indeed call pidprobes.sh
> without any command-line args.  It looks in test/unittest/usdt and
> sees a .sh file and runs it.  The fact that the .sh does not start
> with "tst." is irrelevant.  Indeed, we have lots of tests that do not
> start with "tst." (nor "err." nor "drp." etc.).  So if pidprobes.sh is
> not supposed to be run by runtest.sh directly, it has to be hidden
> better than simply by stripping tst. from its name.

What? That's not what's supposed to happen!

... oh, wait, I'm wrong, I'm thinking of tst.* versus err.*, but of
course non-tst stuff runs anyway.

I guess library things have to have non-.sh extensions?

> Incidentally, there is an orphaned tst.pidprobes.r file.

Yeah, that's the bug :) there's suppoed to be a tst.pidprobes.sh -- and
now (as of my most recent push, and mail here) there is again.

It was not my intention to disappear your test! :)

>>>> --- a/test/unittest/usdt/tst.pidprobes.sh
>>>> +++ b/test/unittest/usdt/pidprobes.sh
>>>> @@ -5,9 +5,12 @@
>>>>    # Licensed under the Universal Permissive License v 1.0 as shown at
>>>>    # http://oss.oracle.com/licenses/upl.
>>>>    #
>>>> -# This test verifies that USDT and pid probes can share underlying probes.
>>>> +# This test verifies various properties of USDT and pid probes sharing
>>>> +# underlying probes.
>>>>      dtrace=$1
>>>> +usdt=$2
>>> Why is there a usdt=$2?  It seems that usdt is always "t" and you'd expect that in test/unittest/usdt.
>> Oh crap. Sorry sorry this is a bug -- I lost the tst.pidprobes.sh I
>> meant to add back. Will fix and repush this patch.
>
> The patch has perhaps already landed.  So, a delta patch might be needed.

Sigh.

>> -- which reads, in entirety
>>
>> exec $(dirname $_test)/pidprobes.sh $1 "" ""
>
> Or, just let runtest.sh call it directly, as it does anyhow.

In which case I could rename it back to tst.pidprobes.sh and have other
things call that (and have it default to the behaviour it had before if
called with no args)... does that sound better?

-- 
NULL && (void)