[DTrace-devel] [PATCH] Allow probes not to attach if they were not "needed"
Eugene Loh
eugene.loh at oracle.com
Wed Sep 17 20:04:57 UTC 2025
On 9/17/25 13:43, Kris Van Hees wrote:
> I think this is actually counter-intuitive.
I certainly agree. And I wrestled with this a lot.
A "default" is overridden if any alternative is specified. In the case
of this patch, if a probe comes from two probe descriptions, one that
requires the probe and one that does not, then the "required" case
should prevail. So it is not a default. There is something non-default
about needing a probe to attach.
Another complication is that when we walk probes based on a
user-specified probe description, which is when we decide whether a
probe will have to attach, we are simply "inserting" probes; this is
well before probes are enabled. Probes are enabled later, when we no
longer are dealing with user-specified probe descriptions. (If I
remember correctly.) So we have to mark probes well before we enable them.
I agree there is something counterintuitive going on here, but I find
the second list of enablings to be even harder to fathom.
Further, a second list of enablings strikes me as a funny creature.
Today, a dt_probe_t is really just a dt_list_t with a bunch of extra
"probe" stuff hanging onto it. That is, a dt_probe_t is an item on a
single, particular linked list. Clearly, we could change it so that a
probe could exist on two different lists, but that gets disruptive.
I'm not saying that the patch is the only way of solving the underlying
problem. I do think, though, that having a reasonable patch that solves
the problem and tests well is attractive, and there is no need to
explore other solutions. "Good enough" is good enough, and it is not
worth spending time exploring other solutions. There is so much to do
and so little time. I vote for the patch that is working and reasonable.
> 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