[DTrace-devel] [PATCH] Allow probes not to attach if they were not "needed"
Kris Van Hees
kris.van.hees at oracle.com
Wed Sep 17 17:43:20 UTC 2025
I think this is actually counter-intuitive. In general, probes are expected
to be able to be attached to or else an error is to be raised. There are
exceptional cases where a failure to attach should not be considered an error.
So, I believe that the implementation should by default expect that failure to
attach is an error. Introducing a flag is certainly an option (though I would
expect it a flag to mark the probe as *optional*), though another approach that
might be worth exploring here is to introduce a 2nd list of enablings, named
dt_opt_enablings, and add optional probes to this list. That way you could
refactor the construction and loading of programs into functions called for a
given probe, and just loop through the mandatory enablings first, and then the
optional ones. That avoids adding a flag and also differentiates the two types
of enablings explicitly.
On Thu, Sep 04, 2025 at 03:02:26PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Various kernels prevent the placement of uprobes on particular
> instructions -- e.g., on x86 on hlt or multi-byte nops, or on aarch64 on
> autiasp. This means, for example, that one cannot place a pid probe on
> every possible instruction. If a user explicitly places a probe on a
> forbidden instruction, an error message indicates there is a problem.
>
> On the other hand, if the user specifies a wildcard in a pid probe
> description, we should skip instructions where no uprobe can be placed.
>
> The problem is that we do not find out an instruction is problematic
> until we try to attach, which is long after probe descriptions have been
> processed. And when we walk probe descriptions, we do not have
> information about which kernels and instructions are problematic.
>
> The motivating problem pertains to pid probes, but we would like
> a provider-agnostic solution.
>
> Introduce a "NEEDED" probe flag. Set the flag when a probe is explicitly
> needed (say, it was specifically named). We use two such flags:
>
> * In dt_probe.h, DT_PROBE_FLAG_NEEDED is used for the "flags"
> member of dt_probe_t.
>
> * For pid-style probes (including USDT and stapsdt), in
> include/dtrace/pid.h, DT_PID_PSP_FLAG_NEEDED is used for
> the pps_flags member of pid_probespec_t.
>
> Set the "NEEDED" flag for a probe in these situations:
>
> * In dt_tp_probe_insert(), set the flag for each probe we insert.
> This covers the dtrace, fbt, rawtp, sdt, and syscall providers.
>
> * In dt_sdt_populate(), set the flag for each probe we insert.
> This covers, e.g., the io, ip, lockstat, proc, sched, and tcp
> providers.
>
> * For the cpc and profile providers, set the flag for probes created
> with the "provide" functions (explicitly requested by the user)
> but not with the "populate" functions (chosen by default by dtrace).
>
> * For providers that will sit on underlying probes, set the NEEDED
> flag for pid_probespec_t. Specifically,
>
> * In dt_pid_per_sym(), set the NEEDED flag to cover entry,
> return, and explicit offsets. Unset the flag for glob offsets.
> In dt_pid_create_usdt_probes_proc() and dt_stapsdt_parse(),
> set the flag.
>
> * In dt_prov_uprobe(), pass the pid_probespec_t NEEDED flag
> to dt_probe_t, for both the overlying and underlying probe.
>
> In dt_bpf.c, if the return code from the attach function indicates
> failure, return a failure only if the probe was NEEDED.
>
> Remove the old check on "hlt" instructions, since it was not sufficiently
> general for other kernels and other problematic instructions.
>
> Existing tests that expose the problem are
>
> test/unittest/pid/tst.coverage.d
> test/unittest/pid/tst.emptystack.d
> test/unittest/pid/tst.entry_off0.sh
> hlt instruction on x86
>
> test/unittest/pid/tst.float.d
> multi-byte nop on x86 on newer platforms
>
> test/unittest/pid/tst.entry_off0.sh
> autiasp instruction on aarch64 on newer platforms
> depending on how the trigger process was built
>
> A new test is added to check that a probe set explicitly on a
> problematic instruction will still lead to an error.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> include/dtrace/pid.h | 3 +++
> libdtrace/dt_bpf.c | 2 +-
> libdtrace/dt_pid.c | 10 ++++------
> libdtrace/dt_probe.h | 3 +++
> libdtrace/dt_prov_cpc.c | 10 ++++++++--
> libdtrace/dt_prov_profile.c | 10 ++++++++--
> libdtrace/dt_prov_uprobe.c | 18 ++++++++++++++++++
> libdtrace/dt_provider_sdt.c | 8 ++++++--
> libdtrace/dt_provider_tp.c | 7 ++++++-
> test/unittest/pid/err.needed.aarch64.x | 3 +++
> test/unittest/pid/err.needed.sh | 23 +++++++++++++++++++++++
> test/unittest/pid/err.needed.x86_64.x | 6 ++++++
> test/unittest/pid/tst.entry_off0.sh | 4 ++--
> 13 files changed, 91 insertions(+), 16 deletions(-)
> create mode 100755 test/unittest/pid/err.needed.aarch64.x
> create mode 100755 test/unittest/pid/err.needed.sh
> create mode 100755 test/unittest/pid/err.needed.x86_64.x
>
> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
> index 8ddb1167e..7829e47c0 100644
> --- a/include/dtrace/pid.h
> +++ b/include/dtrace/pid.h
> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
> size_t pps_xargvlen; /* (high estimate of) length of array */
> int8_t *pps_argmap; /* mapped arg indexes */
> char *pps_sargv; /* list of arg sources */
> + int pps_flags; /* flags */
>
> /*
> * Fields below this point do not apply to underlying probes.
> @@ -55,4 +56,6 @@ typedef struct pid_probespec {
> uint64_t pps_nameoff; /* offset to use for name */
> } pid_probespec_t;
>
> +#define DT_PID_PSP_FLAG_NEEDED 1 /* probe is needed */
> +
> #endif /* _DTRACE_PID_H */
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 31781ac9f..dc3d47f56 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -1385,7 +1385,7 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> if (prp->prov->impl->attach)
> rc = prp->prov->impl->attach(dtp, prp, fd);
>
> - if (rc < 0) {
> + if (rc < 0 && (prp->flags & DT_PROBE_FLAG_NEEDED)) {
> close(fd);
> dt_attach_error(dtp, rc,
> prp->desc->prv, prp->desc->mod,
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index 4af9141aa..6e7fba287 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -184,6 +184,7 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> psp->pps_fun = (char *) func;
> psp->pps_nameoff = 0;
> psp->pps_off = symp->st_value - pp->dpp_vaddr;
> + psp->pps_flags |= DT_PID_PSP_FLAG_NEEDED;
>
> /*
> * The special function "-" means the probe name is an absolute
> @@ -386,15 +387,10 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
> #define disasm(x, y) 4
> #endif
>
> + psp->pps_flags &= ~DT_PID_PSP_FLAG_NEEDED;
> for (off = 0; off < symp->st_size; off += disasm(off, &disasm_info)) {
> char offstr[32];
>
> -#if defined(__amd64)
> - /* Newer kernels do not allow uprobes on "hlt" instructions. */
> - if ((unsigned int)disasm_info.buffer[off] == 0xf4)
> - continue;
> -#endif
> -
> snprintf(offstr, sizeof(offstr), "%lx", off);
> if (!gmatch(offstr, pp->dpp_name))
> continue;
> @@ -1064,6 +1060,7 @@ dt_pid_create_usdt_probes_proc(dtrace_hdl_t *dtp, pid_t pid, dt_proc_t *dpr,
> psp.pps_pid = dpr->dpr_pid;
> psp.pps_off = tp->tracepoint.addr - pmp->pr_file->first_segment->pr_vaddr;
> psp.pps_nameoff = 0;
> + psp.pps_flags |= DT_PID_PSP_FLAG_NEEDED;
>
> if (nargv) {
> psp.pps_nargc = probe->probe.nargc;
> @@ -1421,6 +1418,7 @@ dt_stapsdt_parse(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t *pdp,
> psp.pps_inum = pmp->pr_inum;
> psp.pps_pid = dpr->dpr_pid;
> psp.pps_nameoff = 0;
> + psp.pps_flags |= DT_PID_PSP_FLAG_NEEDED;
>
> if (pvp->impl->provide_probe(dtp, &psp) < 0) {
> dt_pid_error(dtp, pcb, dpr, D_PROC_USDT,
> diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
> index 2a78cb9ca..ffd994844 100644
> --- a/libdtrace/dt_probe.h
> +++ b/libdtrace/dt_probe.h
> @@ -53,10 +53,13 @@ typedef struct dt_probe {
> uint8_t *mapping; /* translated argument mapping */
> dtrace_typeinfo_t *argv; /* output argument types */
> int argc; /* output argument count */
> + int flags; /* flags for the probe */
> dt_probe_instance_t *pr_inst; /* list of functions and offsets */
> dtrace_difo_t *difo; /* BPF probe program */
> } dt_probe_t;
>
> +#define DT_PROBE_FLAG_NEEDED 1 /* probe is needed */
> +
> extern dt_probe_t *dt_probe_lookup2(dt_provider_t *, const char *);
> extern dt_probe_t *dt_probe_create(dtrace_hdl_t *, dt_ident_t *, int,
> dt_node_t *, uint_t, dt_node_t *, uint_t);
> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> index 57b11b135..9fb165cb6 100644
> --- a/libdtrace/dt_prov_cpc.c
> +++ b/libdtrace/dt_prov_cpc.c
> @@ -347,6 +347,7 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> {
> dt_provider_t *prv;
> struct perf_event_attr attr;
> + dt_probe_t *prp;
>
> prv = dt_provider_lookup(dtp, prvname);
> if (!prv)
> @@ -358,17 +359,22 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> return 0;
>
> /* return if we already have this probe */
> - if (dt_probe_lookup(dtp, pdp))
> + prp = dt_probe_lookup(dtp, pdp);
> + if (prp) {
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> return 0;
> + }
>
> /* check if the probe name can be decoded */
> if (decode_probename(&attr, prv, pdp->prb) == -1)
> return 0;
>
> /* try to add this probe */
> - if (cpc_probe_insert(dtp, prv, pdp->prb) == NULL)
> + prp = cpc_probe_insert(dtp, prv, pdp->prb);
> + if (prp == NULL)
> return 0;
>
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> return 1;
> }
>
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index e1369ca9b..ec32ce83b 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -160,6 +160,7 @@ static uint64_t get_period(const char *name)
> static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> {
> dt_provider_t *prv;
> + dt_probe_t *prp;
> int kind;
> uint64_t period;
>
> @@ -176,8 +177,11 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> return 0;
>
> /* return if we already have this probe */
> - if (dt_probe_lookup(dtp, pdp))
> + prp = dt_probe_lookup(dtp, pdp);
> + if (prp) {
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> return 0;
> + }
>
> /* get the provider - should have been created in populate() */
> prv = dt_provider_lookup(dtp, prvname);
> @@ -189,9 +193,11 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
> return 0;
>
> /* try to add this probe */
> - if (profile_probe_insert(dtp, prv, pdp->prb, kind, period) == NULL)
> + prp = profile_probe_insert(dtp, prv, pdp->prb, kind, period);
> + if (prp == NULL)
> return 0;
>
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> return 1;
> }
>
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 07b26f20f..2396522ce 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -960,6 +960,12 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
> } else
> upp = uprp->prv_data;
>
> + /*
> + * Pass flag info from psp to uprp
> + */
> + if (psp->pps_flags & DT_PID_PSP_FLAG_NEEDED)
> + uprp->flags |= DT_PROBE_FLAG_NEEDED;
> +
> /*
> * Only one USDT probe can correspond to each underlying probe.
> */
> @@ -1049,6 +1055,12 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> /* Look up the overlying probe. */
> prp = dt_probe_lookup(dtp, &pd);
> if (prp != NULL) {
> + /*
> + * Pass flag info from psp to uprp
> + */
> + if (psp->pps_flags & DT_PID_PSP_FLAG_NEEDED)
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> +
> /*
> * Probe already exists. If it's already in the underlying
> * probe's probe list, there is nothing left to do.
> @@ -1091,6 +1103,12 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> return -1;
> }
>
> + /*
> + * Pass flag info from psp to uprp
> + */
> + if (psp->pps_flags & DT_PID_PSP_FLAG_NEEDED)
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> +
> /*
> * Add the overlying probe to the list of probes for the underlying probe.
> */
> diff --git a/libdtrace/dt_provider_sdt.c b/libdtrace/dt_provider_sdt.c
> index 962848485..f9a51fa25 100644
> --- a/libdtrace/dt_provider_sdt.c
> +++ b/libdtrace/dt_provider_sdt.c
> @@ -48,10 +48,14 @@ dt_sdt_populate(dtrace_hdl_t *dtp, const char *prvname, const char *modname,
> * entries to identify the probe names.
> */
> for (arg = &probe_args[0]; arg->name != NULL; arg++) {
> + dt_probe_t *prp;
> +
> if (arg->argno == 0 &&
> - dt_probe_insert(dtp, prv, prvname, modname, "", arg->name,
> - NULL))
> + (prp = dt_probe_insert(dtp, prv, prvname, modname,
> + "", arg->name, NULL))) {
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> n++;
> + }
> }
>
> return n;
> diff --git a/libdtrace/dt_provider_tp.c b/libdtrace/dt_provider_tp.c
> index 3d4ec1e03..4531a88a8 100644
> --- a/libdtrace/dt_provider_tp.c
> +++ b/libdtrace/dt_provider_tp.c
> @@ -368,12 +368,17 @@ dt_tp_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const char *prv,
> const char *mod, const char *fun, const char *prb)
> {
> tp_probe_t *tpp;
> + dt_probe_t *prp;
>
> tpp = dt_tp_alloc(dtp);
> if (tpp == NULL)
> return NULL;
>
> - return dt_probe_insert(dtp, prov, prv, mod, fun, prb, tpp);
> + prp = dt_probe_insert(dtp, prov, prv, mod, fun, prb, tpp);
> + if (prp)
> + prp->flags |= DT_PROBE_FLAG_NEEDED;
> +
> + return prp;
> }
>
> /*
> diff --git a/test/unittest/pid/err.needed.aarch64.x b/test/unittest/pid/err.needed.aarch64.x
> new file mode 100755
> index 000000000..bd5b41a36
> --- /dev/null
> +++ b/test/unittest/pid/err.needed.aarch64.x
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +echo "skip on aarch64"
> +exit 2
> diff --git a/test/unittest/pid/err.needed.sh b/test/unittest/pid/err.needed.sh
> new file mode 100755
> index 000000000..d6b041b60
> --- /dev/null
> +++ b/test/unittest/pid/err.needed.sh
> @@ -0,0 +1,23 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +
> +# We should not be able to trace on a hlt instruction.
> +#
> +# (When the probe is "needed" -- that is, explicitly requested by the user
> +# -- we should incur an error. Test tst.entry_off0.sh should be okay,
> +# since dtrace will silently skip over such problematic instructions.)
> +
> +dtrace=$1
> +trig=`pwd`/test/triggers/ustack-tst-basic
> +off=`${OBJDUMP} -d $trig | awk '/hlt/ {sub(/:/, ""); print $1}'`
> +
> +$dtrace $dt_flags -c $trig -n 'pid$target:a.out:-:'$off'
> +{
> + trace("hlt instruction");
> + exit(0);
> +}'
> diff --git a/test/unittest/pid/err.needed.x86_64.x b/test/unittest/pid/err.needed.x86_64.x
> new file mode 100755
> index 000000000..158336d98
> --- /dev/null
> +++ b/test/unittest/pid/err.needed.x86_64.x
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +if ! ${OBJDUMP} -d test/triggers/ustack-tst-basic | grep hlt; then
> + echo "did not find hlt instruction in trigger"
> + exit 2
> +fi
> +exit 0
> diff --git a/test/unittest/pid/tst.entry_off0.sh b/test/unittest/pid/tst.entry_off0.sh
> index 1a4c3d207..304be4568 100755
> --- a/test/unittest/pid/tst.entry_off0.sh
> +++ b/test/unittest/pid/tst.entry_off0.sh
> @@ -1,14 +1,14 @@
> #!/bin/bash
> #
> # Oracle Linux DTrace.
> -# Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved.
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
> #
>
> # Although the D script takes only "one second," it takes a long time to
> # shut down. Until that has been solved, increase the timeout for the test.
> -# @@timeout: 120
> +# @@timeout: 240
>
> dtrace=$1
>
> --
> 2.47.3
>
More information about the DTrace-devel
mailing list