* [PATCH REVIEWED v3 1/6] error: add missing EDT_PRINT entry
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-30 12:08 ` [PATCH v3 2/6] usdt: get arg types and xlations into DTrace from the DOF Nick Alcock
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel; +Cc: Alan Maguire, Kris Van Hees
The lack of this made it impossible to print out the last error
in the list (which is EDT_PRINT itself).
Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
---
libdtrace/dt_error.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libdtrace/dt_error.c b/libdtrace/dt_error.c
index 82e114a5b292..213f0d9e1385 100644
--- a/libdtrace/dt_error.c
+++ b/libdtrace/dt_error.c
@@ -97,7 +97,8 @@ static const struct {
{ EDT_OBJIO, "Cannot read object file or modules.dep" },
{ EDT_READMAXSTACK, "Cannot read kernel param perf_event_max_stack" },
{ EDT_TRACEMEM, "Missing or corrupt tracemem() record" },
- { EDT_PCAP, "Missing or corrupt pcap() record" }
+ { EDT_PCAP, "Missing or corrupt pcap() record" },
+ { EDT_PRINT, "Missing or corrupt print() record" }
};
static const int _dt_nerr = sizeof(_dt_errlist) / sizeof(_dt_errlist[0]);
--
2.46.0.278.g36e3a12567
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/6] usdt: get arg types and xlations into DTrace from the DOF
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
2024-10-30 12:08 ` [PATCH REVIEWED v3 1/6] error: add missing EDT_PRINT entry Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-30 12:08 ` [PATCH REVIEWED v3 3/6] dtprobed: stop skipping zero-tracepoint probes in dof_stash.c Nick Alcock
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel
This change propagates native and xlated arg types and remapping 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 | 59 ++++++++++++++++
test/utils/showUSDT.c | 5 +-
7 files changed, 259 insertions(+), 51 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 fdcdee14f851..cdb08a65205e 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -795,16 +795,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..8eb8a1708d04 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 remapped 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; /* remapped 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..dd3a6cd2b887 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 remapping 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 62060b5921b6..e4dc5368897d 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,47 @@ 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;
+
+ 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 +1011,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) {
diff --git a/test/utils/showUSDT.c b/test/utils/showUSDT.c
index f42aaa21103a..73117d700a54 100644
--- a/test/utils/showUSDT.c
+++ b/test/utils/showUSDT.c
@@ -111,8 +111,9 @@ static void processProbe(int id, dof_probe_t *probe, const char *name,
printf(" Probe %d: %s:%s:%s:%s\n",
id, name, "",
strtab + probe->dofpr_func, strtab + probe->dofpr_name);
- printf(" argc %d noffs %d nenoffs %d\n",
- probe->dofpr_nargc, probe->dofpr_noffs, probe->dofpr_nenoffs);
+ printf(" nargc %d xargc %d argidx %d noffs %d nenoffs %d\n",
+ probe->dofpr_nargc, probe->dofpr_xargc, probe->dofpr_argidx,
+ probe->dofpr_noffs, probe->dofpr_nenoffs);
offs = &offtab[probe->dofpr_offidx];
for (i = 0; i < probe->dofpr_noffs; i++)
--
2.46.0.278.g36e3a12567
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH REVIEWED v3 3/6] dtprobed: stop skipping zero-tracepoint probes in dof_stash.c
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
2024-10-30 12:08 ` [PATCH REVIEWED v3 1/6] error: add missing EDT_PRINT entry Nick Alcock
2024-10-30 12:08 ` [PATCH v3 2/6] usdt: get arg types and xlations into DTrace from the DOF Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-30 12:08 ` [PATCH v3 4/6] cg: add argument mapping in the trampoline Nick Alcock
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel; +Cc: Kris Van Hees
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 4/6] cg: add argument mapping in the trampoline
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
` (2 preceding siblings ...)
2024-10-30 12:08 ` [PATCH REVIEWED v3 3/6] dtprobed: stop skipping zero-tracepoint probes in dof_stash.c Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-30 12:08 ` [PATCH v3 5/6] usdt: typed args and arg mapping Nick Alcock
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel
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 | 36 +++++++++++++++++++++++++++++++++++-
libdtrace/dt_cg.h | 1 +
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 39c27ab0ec0b..959be5601ea8 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -831,6 +831,35 @@ dt_cg_tramp_restore_args(dt_pcb_t *pcb)
}
}
+/*
+ * Remap arguments according to the array of mappings given. The original
+ * arguments are overwritten (but saved). In effect, this changes the native
+ * arguments: the caller should adjust the mappings array accordingly, and
+ * shuffle the native arg types to match.
+ *
+ * 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, uint8_t *mappings, size_t nmappings)
+{
+ dt_irlist_t *dlp = &pcb->pcb_ir;
+ int i;
+
+ /*
+ * Work from saved args so we don't need to worry about overwriting regs
+ * we're about to remap.
+ */
+ dt_cg_tramp_save_args(pcb);
+
+ for (i = 0; i < nmappings; i++) {
+ if (mappings[i] != i) {
+ emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_ORIG_ARG(mappings[i])));
+ 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;
@@ -5060,7 +5089,12 @@ dt_cg_array_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
* array index according to the static argument mapping (if any),
* 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 dt_cg_tramp_remap_args is in use, you should apply the remapping
+ * at the probe_info stage, since the effective "native" arg positions
+ * will be changed.
+ *
+ * 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..8f66b7a7cb98 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, uint8_t *mappings, size_t nmappings);
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 5/6] usdt: typed args and arg mapping
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
` (3 preceding siblings ...)
2024-10-30 12:08 ` [PATCH v3 4/6] cg: add argument mapping in the trampoline Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-30 12:08 ` [PATCH REVIEWED v3 6/6] usdt: fix create_underlying error path Nick Alcock
2024-10-31 18:56 ` [PATCH v3 0/6] usdt typed args, translators and arg remapping Kris Van Hees
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel
This change propagates the arg type and mapping info we just got into the
pid_probespec_t onward into the underlying dt_uprobe_t, shuffles the native
types before handing off a 1:1 mapping to the dt_probe_t, and calls
dt_cg_tramp_map_args() to apply the requested mappings. With this, USDT
probes should now have visible types which obey the :-notation translators
specified in their .d fils (this is now tested).
The probe info 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 do all
the arg reshuffling 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. (We intentionally do not shuffle the args in
dt_cg_tramp_map_args specifically so that we are not sensitive to the
relative call order of trampoline() versus probe_info(), but there's no
escaping the requirement for USDT probe discovery to predate all of this:
but this was already a requirement in any case.)
We make a couple of assumptions for simplicity, noted by TODOs in the source:
- overlying probes all have the same native types and mappings, so we don't
need to look at more than one underlying probe to figure out what they
are. DTrace always generates such a thing, but DOF specifying multiple
overlapping probes seems perfectly possible for other programs to generate.
This should probably be validated in dof_parser.c in future.
- any given underlying probe locus is either a PID or a USDT probe, never
both, so we can just map the args at the start of the trampoline and then
never touch them again. The both-at the-same-locus case, while in theory
possible, is not yet implemented, and significant changes need to be made
to create_underlying() to fix this.
Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
---
libdtrace/dt_cg.c | 16 +-
libdtrace/dt_cg.h | 2 +-
libdtrace/dt_pid.c | 1 +
libdtrace/dt_prov_uprobe.c | 166 +++++++++++++++++-
test/triggers/usdt-tst-argmap-prov.d | 5 +-
test/triggers/usdt-tst-argmap.c | 5 +-
.../dtrace-util/tst.ListProbesArgsUSDT.r | 34 ++++
.../dtrace-util/tst.ListProbesArgsUSDT.r.p | 2 +
.../dtrace-util/tst.ListProbesArgsUSDT.sh | 83 +++++++++
test/unittest/usdt/err.argmap-null.d | 40 +++++
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 | 3 +-
16 files changed, 470 insertions(+), 20 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/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 959be5601ea8..1a5384ae8a4f 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -832,16 +832,16 @@ dt_cg_tramp_restore_args(dt_pcb_t *pcb)
}
/*
- * Remap arguments according to the array of mappings given. The original
- * arguments are overwritten (but saved). In effect, this changes the native
- * arguments: the caller should adjust the mappings array accordingly, and
- * shuffle the native arg types to match.
+ * Remap arguments according to the mappings in the array of arguments given.
+ * The original arguments are overwritten if necessary (but saved). In effect,
+ * this changes the native arguments: the caller should adjust the mappings
+ * array accordingly, and shuffle the native arg types to match.
*
* 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, uint8_t *mappings, size_t nmappings)
+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;
@@ -852,9 +852,9 @@ dt_cg_tramp_map_args(dt_pcb_t *pcb, uint8_t *mappings, size_t nmappings)
*/
dt_cg_tramp_save_args(pcb);
- for (i = 0; i < nmappings; i++) {
- if (mappings[i] != i) {
- emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_7, DMST_ORIG_ARG(mappings[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));
}
}
diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
index 8f66b7a7cb98..fb26c125adbc 100644
--- a/libdtrace/dt_cg.h
+++ b/libdtrace/dt_cg.h
@@ -34,7 +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, uint8_t *mappings, size_t nmappings);
+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);
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index e4dc5368897d..d7c93afab1ec 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -950,6 +950,7 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, dt_proc_t *dpr,
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))
diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 481159b01c24..8187d832ff1e 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -55,7 +55,11 @@ 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 in */
+ char *nargvbuf; /* array of native args */
+ char *xargvbuf; /* array of xlated args */
+ dt_list_t probes; /* pid/USDT probes triggered by it */
} dt_uprobe_t;
typedef struct list_probe {
@@ -112,6 +116,9 @@ 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->nargvbuf);
+ dt_free(dtp, upp->xargvbuf);
dt_free(dtp, upp);
}
@@ -123,6 +130,86 @@ static void probe_destroy(dtrace_hdl_t *dtp, void *datap)
free_probe_list(dtp, datap);
}
+/*
+ * Populate args for an underlying probe. This really relates to the overlying
+ * probe (all underlying probes associated with a given underlying probe must
+ * have the same args), but the overlying probe doesn't exist at the point this
+ * is populated, so we must put it in the underlying probe instead and pull it
+ * out when the overlying probe info is called for.
+ *
+ * Move it into dt_argdesc_t's for use later on. The char *'s in that
+ * structure are pointers into the nargvbuf and xargvbuf arrays, which
+ * are straight copies of the nargc/xargc 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.
+ */
+ if (upp->argc == 0 || psp->pps_xargv == NULL || psp->pps_nargv == NULL
+ || psp->pps_xargvlen == 0 || psp->pps_nargvlen == 0)
+ return 0;
+
+ upp->xargvbuf = dt_alloc(dtp, psp->pps_xargvlen);
+ if(upp->xargvbuf == NULL)
+ return -1;
+ memcpy(upp->xargvbuf, psp->pps_xargv, psp->pps_xargvlen);
+ xptr = upp->xargvbuf;
+
+ upp->nargvbuf = dt_alloc(dtp, psp->pps_nargvlen);
+ if(upp->nargvbuf == NULL)
+ return -1;
+ memcpy(upp->nargvbuf, psp->pps_nargv, psp->pps_nargvlen);
+ nptr = upp->nargvbuf;
+
+ 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.
+ */
+
+ for (i = 0; i < upp->argc; i++) {
+ int map_arg = psp->pps_argmap[i];
+
+ if (xptr) {
+ upp->args[i].native = nargv[map_arg];
+ upp->args[i].xlate = xptr;
+ xptr += strlen(xptr) + 1;
+ } else {
+ upp->args[i].native = nptr;
+ nptr += strlen(nptr) + 1;
+ }
+ upp->args[i].mapping = map_arg;
+ upp->args[i].flags = 0;
+ }
+
+ free(nargv);
+ return 0;
+
+ fail:
+ free(nargv);
+ return -1;
+}
/*
* Look up or create an underlying (real) probe, corresponding directly to a
@@ -138,7 +225,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;
int is_enabled = 0;
/*
@@ -201,8 +288,11 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
if (upp->tp == NULL)
goto fail;
+ if (populate_args(dtp, psp, upp) < 0)
+ goto fail;
+
uprp = dt_probe_insert(dtp, pvp, pd.prv, pd.mod, pd.fun, pd.prb,
- upp);
+ upp);
if (uprp == NULL)
goto fail;
} else
@@ -224,6 +314,8 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
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;
}
@@ -413,6 +505,9 @@ static void enable_usdt(dtrace_hdl_t *dtp, dt_probe_t *prp)
*
* The trampoline will first populate a dt_dctx_t struct. It will then emulate
* the firing of all dependent pid* probes and their clauses.
+ *
+ * uprobe trampolines will also arrange for argument shuffling if the argmap
+ * array calls for it.
*/
static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
{
@@ -437,6 +532,23 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
dt_cg_tramp_copy_args_from_regs(pcb,
!(upp->flags & PP_IS_FUNCALL));
+ /*
+ * Remap args, if needed.
+ */
+
+ /*
+ * TODO: pid probes are never mapped, so a mapped set of args
+ * for UDST probes need saving/restoring to reverse the mapping
+ * for pid probes at the same locus -- but right now pid and
+ * usdt probes hitting the same locus simply end up as whichever
+ * registered first because create_underlying cannot handle this
+ * case, so we can avoid it here too. dt_cg_tramp_map_args
+ * already saves regs: all we need to do is restore them before
+ * any pid probe is hit.
+ */
+ if (upp->args != NULL)
+ dt_cg_tramp_map_args(pcb, upp->args, upp->argc);
+
/*
* Retrieve the PID of the process that caused the probe to fire.
*/
@@ -710,8 +822,49 @@ 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;
+ 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)) {
+ 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);
+ }
+ }
+
+done:
+ *argcp = argc;
+ *argvp = argv;
return 0;
}
@@ -779,7 +932,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,
};
@@ -794,7 +946,6 @@ dt_provimpl_t dt_uprobe_is_enabled = {
.load_prog = &dt_bpf_prog_load,
.trampoline = &trampoline_is_enabled,
.attach = &attach,
- .probe_info = &probe_info,
.detach = &detach,
.probe_destroy = &probe_destroy_underlying,
};
@@ -818,5 +969,6 @@ dt_provimpl_t dt_usdt = {
.prog_type = BPF_PROG_TYPE_UNSPEC,
.provide_probe = &provide_usdt_probe,
.enable = &enable_usdt,
+ .probe_info = &probe_info,
.probe_destroy = &probe_destroy,
};
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..8bcfa92ff6b0
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.r
@@ -0,0 +1,34 @@
+ ID PROVIDER MODULE FUNCTION NAME
+ 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
+
+ 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..ad8dc2e7c267
--- /dev/null
+++ b/test/unittest/dtrace-util/tst.ListProbesArgsUSDT.sh
@@ -0,0 +1,83 @@
+#!/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);
+};
+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");
+ 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..8b805cfcfd5f
--- /dev/null
+++ b/test/unittest/usdt/err.argmap-null.d
@@ -0,0 +1,40 @@
+/*
+ * 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 with no args at all 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..ff34ffba5054
--- /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 23: 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..b672df3470d6
--- /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 remap 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..14e7e0992045
--- /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 remapped,
+ * 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..ef5b32d1d372
--- /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 remapped.
+ */
+
+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..9cd3425bf8e0 100644
--- a/test/unittest/usdt/tst.argmap.d
+++ b/test/unittest/usdt/tst.argmap.d
@@ -1,11 +1,10 @@
/*
* 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 */
--
2.46.0.278.g36e3a12567
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH REVIEWED v3 6/6] usdt: fix create_underlying error path
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
` (4 preceding siblings ...)
2024-10-30 12:08 ` [PATCH v3 5/6] usdt: typed args and arg mapping Nick Alcock
@ 2024-10-30 12:08 ` Nick Alcock
2024-10-31 18:56 ` [PATCH v3 0/6] usdt typed args, translators and arg remapping Kris Van Hees
6 siblings, 0 replies; 8+ messages in thread
From: Nick Alcock @ 2024-10-30 12:08 UTC (permalink / raw)
To: dtrace, dtrace-devel; +Cc: Kris Van Hees
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 8187d832ff1e..f9c2027935b7 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -316,7 +316,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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/6] usdt typed args, translators and arg remapping
2024-10-30 12:08 [PATCH v3 0/6] usdt typed args, translators and arg remapping Nick Alcock
` (5 preceding siblings ...)
2024-10-30 12:08 ` [PATCH REVIEWED v3 6/6] usdt: fix create_underlying error path Nick Alcock
@ 2024-10-31 18:56 ` Kris Van Hees
6 siblings, 0 replies; 8+ messages in thread
From: Kris Van Hees @ 2024-10-31 18:56 UTC (permalink / raw)
To: Nick Alcock; +Cc: dtrace, dtrace-devel
Can you please rebase on github's devel? Eugene's USDT work is on there and
that impacts patches 5/6 and 6/6.
Also, 1/6 is already on devel.
Please also searc hfor any occurences of /remap/ and fix those to use the
more consistent "map" and "mapping" language.
On Wed, Oct 30, 2024 at 12:08:44PM +0000, Nick Alcock wrote:
> 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
> remappings 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 remap not only arg[N] but
> also argN references (the single existing test for USDT arg remapping
> verifies that this works), but the SDT machinery depends on argN being
> unremapped! So we do the remapping in dt_prov_uprobe.c while reshuffling
> things into dt_argdesc_t's, and tell the higher layers that no arg remapping
> is ever being performed, then do the remapping 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 fix a couple of tiny error-related bugs encountered in the
> course of development, one in usdt, one related to the print action.
>
> 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):
> error: add missing EDT_PRINT entry
> 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: fix create_underlying error path
>
> dtprobed/dof_stash.c | 21 +--
> dtprobed/dtprobed.c | 10 +-
> include/dtrace/pid.h | 7 +
> libcommon/dof_parser.c | 150 ++++++++++++----
> libcommon/dof_parser.h | 64 ++++++-
> libdtrace/dt_cg.c | 36 +++-
> libdtrace/dt_cg.h | 1 +
> libdtrace/dt_error.c | 3 +-
> libdtrace/dt_pid.c | 60 +++++++
> libdtrace/dt_prov_uprobe.c | 168 +++++++++++++++++-
> test/triggers/usdt-tst-argmap-prov.d | 5 +-
> test/triggers/usdt-tst-argmap.c | 5 +-
> .../dtrace-util/tst.ListProbesArgsUSDT.r | 34 ++++
> .../dtrace-util/tst.ListProbesArgsUSDT.r.p | 2 +
> .../dtrace-util/tst.ListProbesArgsUSDT.sh | 83 +++++++++
> test/unittest/usdt/err.argmap-null.d | 40 +++++
> 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 | 3 +-
> test/utils/showUSDT.c | 5 +-
> 23 files changed, 759 insertions(+), 71 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
>
>
> base-commit: 8a1264bf0e818c8624be250eb5174714b62ed93c
> --
> 2.46.0.278.g36e3a12567
>
^ permalink raw reply [flat|nested] 8+ messages in thread