[DTrace-devel] [PATCH v2 6/6] usdt: provide all underlying probes in a pid once per pid
Nick Alcock
nick.alcock at oracle.com
Wed May 17 19:53:03 UTC 2023
Another bug report revealed that USDT probes were sometimes failing to
be provided when there were multiple probes in the D script. The
underlying cause of this is that dt_pid_create_usdt_probes() gets called
once per dt_setcontext() of a USDT probe, but every call checks each
underlying probe to see if it matches the overlying probe we're trying
to provide, provides it if so, and then calls dt_pid_fix_mod() on that
underlying probe... which changes the module name, and now it no longer
matches in subsequent calls.
Since these are in any case *underlying* probes, and thus kept disabled
unless an overlying probe enables them, there's no reason not to just
provide all underlying probes in a pid the first time
dt_pid_create_usdt_probes() is called for a given pid, and pay no
attention to the probe description passed down from dt_setcontext() at
all.
The existing change whereby all DTrace uprobes in the system are
remembered on the first call to dt_pid_create_usdt_probes() is still
beneficial and is retained: this just simplifies things and speeds them
up even more. As a side benefit this means that some wildcard USDT
probes now work: just provider wildcards, not other wildcards, but
that's better than nothing.
We soup up test/unittest/usdt/tst.multiple.sh to identify this failure
(which means not using -p or -c, since those trigger additional uprobe
scans in the old implementation and catch more probes by accident, and
also probing multiple different probes as well as one probe with
multiple loci; we also add a probe that isn't probed just to make sure
that non-enabled underlying probes really don't fire).
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libdtrace/dt_pid.c | 94 ++++++++---------------------
libdtrace/dt_proc.h | 1 +
test/unittest/pid/tst.provregex3.sh | 4 +-
test/unittest/usdt/tst.multiple.r | 2 +
test/unittest/usdt/tst.multiple.sh | 26 +++++++-
5 files changed, 52 insertions(+), 75 deletions(-)
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 627587b6bb51b..b0689aa8a4955 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -51,8 +51,6 @@ typedef struct dt_pid_probe {
uint_t dpp_last_taken;
} dt_pid_probe_t;
-static char *dt_pid_de_pid(char *buf, const char *prv);
-
/*
* Compose the lmid and object name into the canonical representation. We
* omit the lmid for the default link map for convenience.
@@ -761,15 +759,11 @@ out:
* If dpr is set, just set up probes relating to mappings found in that one
* process. (dpr must in this case be locked.)
*
- * If pdp is set, create overlying USDT probes for the specified probe
- * description.
- *
* Return 0 on success or -1 on error. (Failure to create specific underlying
* probes is not an error.)
*/
static int
-dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr,
- dtrace_probedesc_t *pdp, dt_pcb_t *pcb)
+dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dt_pcb_t *pcb)
{
const dt_provider_t *pvp;
size_t i;
@@ -823,56 +817,24 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr,
}
/*
- * Does this match the overlying probe we are meant to be
- * creating? If so, create an overlying probe and let
- * provide_probe() create the underlying probe for us. Among
- * other things, this ensures that the PID part of the provider
- * name gets stuck into the overlying probe properly.
+ * Create an underlying probe using psp, if not already present.
*
- * TODO: wildcard probes get handled here.
+ * Complain if any probe cannot be created: at this stage we
+ * cannot reliably tell whether a corresponding overlying probe
+ * will be created (since dt_setcontext only calls us for the
+ * first one in any given provider).
*/
- if (pdp) {
- char de_pid_prov[DTRACE_PROVNAMELEN + 1];
-
- if ((pdp->prv[0] != 0 &&
- strcmp(dt_pid_de_pid(de_pid_prov, pdp->prv),
- psp->pps_prv) != 0) ||
- (pdp->mod[0] != 0 && strcmp(pdp->mod, psp->pps_mod) != 0) ||
- (pdp->fun[0] != 0 && strcmp(pdp->fun, psp->pps_fun) != 0) ||
- (pdp->prb[0] != 0 && strcmp(pdp->prb, psp->pps_prb) != 0)) {
- dt_dprintf("%s:%s:%s:%s -> %s:%s:%s:%s: match failure\n",
- pdp->prv, pdp->mod, pdp->fun, pdp->prb,
- psp->pps_prv, psp->pps_mod, psp->pps_fun,
- psp->pps_prb);
- continue;
- }
- }
- /*
- * Create a probe using psp, if not already present.
- *
- * If we are creating an overlying probe, complain about it if
- * probe creation fails. Otherwise, this is just an underlying
- * probe: we'll complain later if we use it for anything.
- */
-
- dt_dprintf("providing %s:%s:%s:%s\n", pdp->prv, pdp->mod,
- pdp->fun, pdp->prb);
- if (pvp->impl->provide_probe(dtp, psp) < 0 && pdp) {
+ dt_dprintf("providing %s:%s:%s:%s\n", psp->pps_prv, psp->pps_mod,
+ psp->pps_fun, psp->pps_prb);
+ if (pvp->impl->provide_probe(dtp, psp) < 0) {
dt_pid_error(dtp, pcb, dpr, D_PROC_USDT,
"failed to instantiate %sprobe %s for pid %d: %s",
psp->pps_type == DTPPT_IS_ENABLED ?
- "is-enabled ": "", pdp->prb, dpr->dpr_pid,
+ "is-enabled ": "", psp->pps_prb, dpr->dpr_pid,
dtrace_errmsg(dtp, dtrace_errno(dtp)));
ret = -1;
}
-
- if (pdp && dpr) {
- /*
- * Put the module name in its canonical form.
- */
- dt_pid_fix_mod(NULL, pdp, dtp, dpr->dpr_pid);
- }
}
return ret;
@@ -970,23 +932,6 @@ dt_pid_get_pid(const dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb,
return pid;
}
-/*
- * Return a USDT provider name with the pid removed.
- */
-static char *
-dt_pid_de_pid(char *buf, const char *prv)
-{
- const char *c;
- char *p = buf;
-
- for (c = prv; *c != '\0'; c++) {
- if (!isdigit(*c))
- *p++ = *c;
- }
- *p = 0;
- return buf;
-}
-
int
dt_pid_create_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb)
{
@@ -1029,7 +974,15 @@ dt_pid_create_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_t *pcb)
dpr = dt_proc_lookup(dtp, pid);
assert(dpr != NULL);
- err = dt_pid_create_usdt_probes(dtp, dpr, pdp, pcb);
+ if (!dpr->dpr_usdt) {
+ err = dt_pid_create_usdt_probes(dtp, dpr, pcb);
+ dpr->dpr_usdt = B_TRUE;
+ }
+
+ /*
+ * Put the module name in its canonical form.
+ */
+ dt_pid_fix_mod(NULL, pdp, dtp, dpr->dpr_pid);
dt_proc_release_unlock(dtp, pid);
}
@@ -1073,9 +1026,12 @@ dt_pid_create_probes_module(dtrace_hdl_t *dtp, dt_proc_t *dpr)
* If it's not strictly a pid provider, we might match
* a USDT provider.
*/
- if (strcmp(provname, pdp->prv) != 0 &&
- dt_pid_create_usdt_probes(dtp, dpr, pdp, NULL) < 0)
- ret = 1;
+ if (strcmp(provname, pdp->prv) != 0) {
+ if (dt_pid_create_usdt_probes(dtp, dpr, NULL) < 0)
+ ret = 1;
+ else
+ dt_pid_fix_mod(NULL, pdp, dtp, dpr->dpr_pid);
+ }
free((char *)pd.fun);
}
diff --git a/libdtrace/dt_proc.h b/libdtrace/dt_proc.h
index 0ec1caf08fbfa..128003ede10bd 100644
--- a/libdtrace/dt_proc.h
+++ b/libdtrace/dt_proc.h
@@ -39,6 +39,7 @@ typedef struct dt_proc {
uint_t dpr_refs; /* reference count */
uint8_t dpr_stop; /* stop mask: see flag bits below */
uint8_t dpr_done; /* done flag: ctl thread has exited */
+ uint8_t dpr_usdt; /* usdt flag: usdt probes created */
uint8_t dpr_created; /* proc flag: true if we created this
process, false if we grabbed it */
uint8_t dpr_monitoring; /* true if we should background-monitor
diff --git a/test/unittest/pid/tst.provregex3.sh b/test/unittest/pid/tst.provregex3.sh
index 89ca4aec46a5b..6e1afd3e493ee 100755
--- a/test/unittest/pid/tst.provregex3.sh
+++ b/test/unittest/pid/tst.provregex3.sh
@@ -1,15 +1,13 @@
#!/bin/bash
#
# Oracle Linux DTrace.
-# Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2023, 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 a regex in the provider name will match
# USDT probes as well as pid probes (e.g., p*d$target matches both
# pid$target and pyramid$target.)
-#
-# @@xfail: dtv2 (wildcarded usdt probes not yet implemented)
if [ $# != 1 ]; then
echo expected one argument: '<'dtrace-path'>'
diff --git a/test/unittest/usdt/tst.multiple.r b/test/unittest/usdt/tst.multiple.r
index 5e826192fed19..ab9cf62d77196 100644
--- a/test/unittest/usdt/tst.multiple.r
+++ b/test/unittest/usdt/tst.multiple.r
@@ -2,4 +2,6 @@ test:main:go
test:main:go
test:main:go
test:main:go
+test:main:stop
+test:main:stop
diff --git a/test/unittest/usdt/tst.multiple.sh b/test/unittest/usdt/tst.multiple.sh
index eb0a7ccf5e4d3..b31e8eff705d3 100755
--- a/test/unittest/usdt/tst.multiple.sh
+++ b/test/unittest/usdt/tst.multiple.sh
@@ -1,7 +1,7 @@
#!/bin/bash
#
# Oracle Linux DTrace.
-# Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2006, 2023, 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.
#
@@ -21,6 +21,8 @@ cd $DIRNAME
cat > prov.d <<EOF
provider test_prov {
probe go();
+ probe stop();
+ probe ignore();
};
EOF
@@ -32,15 +34,20 @@ fi
cat > test.c <<EOF
#include <sys/types.h>
+#include <unistd.h>
#include "prov.h"
int
main(int argc, char **argv)
{
+ sleep(5); /* Until proper synchronization is implemented. */
TEST_PROV_GO();
TEST_PROV_GO();
TEST_PROV_GO();
TEST_PROV_GO();
+ TEST_PROV_IGNORE();
+ TEST_PROV_STOP();
+ TEST_PROV_STOP();
return 0;
}
@@ -63,11 +70,24 @@ if [ $? -ne 0 ]; then
fi
script() {
- $dtrace -c ./test -qs /dev/stdin <<EOF
- test_prov\$target:::
+ # Wait for full startup, passing into main(), to prevent
+ # rtld activity monitoring from triggering repeated
+ # probe scans (checking for problems doing one-off scans of
+ # already-running processes while probing multiple probes).
+ ./test & { WAIT=$!; sleep 1; $dtrace -qs /dev/stdin $WAIT <<EOF; }
+ test_prov\$1:::go
{
printf("%s:%s:%s\n", probemod, probefunc, probename);
}
+ test_prov\$1:::stop
+ {
+ printf("%s:%s:%s\n", probemod, probefunc, probename);
+ }
+ test_prov\$1:::stop
+ / ++i > 1 /
+ {
+ exit(0);
+ }
EOF
}
--
2.39.1.268.g9de2f9a303
More information about the DTrace-devel
mailing list