* [PATCH 1/2] fbt: Populate just once
@ 2026-01-13 21:42 eugene.loh
2026-01-13 21:42 ` [PATCH 2/2] rawfbt: prvname is not properly set eugene.loh
2026-01-15 21:35 ` [PATCH 1/2] fbt: Populate just once Kris Van Hees
0 siblings, 2 replies; 9+ messages in thread
From: eugene.loh @ 2026-01-13 21:42 UTC (permalink / raw)
To: dtrace, dtrace-devel
From: Eugene Loh <eugene.loh@oracle.com>
In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"),
the populate() functions for fbt and rawfbt were combined, populating both
providers, but the function was still called twice. That is, dt_open.c
tries to insert each provider twice.
One solution would be to have a different populate() function for each
provider.
Here, we employ another solution: in dt_provider_create(), check to see
if a specified provider has already been inserted.
This also requires a corresponding change in dt_provider_lookup(), so
that it will work even if no providers have yet been inserted.
Also, fix a minor comment in the fbt provider.
Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
libdtrace/dt_prov_fbt.c | 2 +-
libdtrace/dt_provider.c | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index bbe44a842..3feac56ea 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -228,7 +228,7 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
if (!dt_symbol_traceable(sym))
continue;
- /* Function name cannot be synthetic and must match. */
+ /* Function name cannot be synthetic (unless rawfbt) and must match. */
fun = dt_symbol_name(sym);
if ((!rawfbt && strchr(fun, '.')) || !dt_gmatch(fun, pdp->fun))
continue;
diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
index 848fdc132..9d75225fa 100644
--- a/libdtrace/dt_provider.c
+++ b/libdtrace/dt_provider.c
@@ -114,6 +114,8 @@ dt_provider_lookup(dtrace_hdl_t *dtp, const char *name)
return NULL;
strcpy(tmpl.desc.dtvd_name, name);
+ if (dtp->dt_provs == NULL)
+ return NULL;
return dt_htab_lookup(dtp->dt_provs, &tmpl);
}
@@ -124,6 +126,10 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name,
{
dt_provider_t *pvp;
+ pvp = dt_provider_lookup(dtp, name);
+ if (pvp)
+ return pvp;
+
if ((pvp = dt_zalloc(dtp, sizeof(dt_provider_t))) == NULL)
goto nomem;
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-13 21:42 [PATCH 1/2] fbt: Populate just once eugene.loh @ 2026-01-13 21:42 ` eugene.loh 2026-01-15 22:18 ` Kris Van Hees 2026-01-15 21:35 ` [PATCH 1/2] fbt: Populate just once Kris Van Hees 1 sibling, 1 reply; 9+ messages in thread From: eugene.loh @ 2026-01-13 21:42 UTC (permalink / raw) To: dtrace, dtrace-devel From: Eugene Loh <eugene.loh@oracle.com> The char array prvname[] is set for each provider. It is used in the file that implements the provider. It might also be passed to dt_sdt_populate() via a function argument. However, dt_provider_tp.h also defines the macro GROUP_DATA in terms of prvname. In turn, GROUP_DATA is used not only in dt_prov_dtrace.c but then again in dt_prov_fbt.c to define FBT_GROUP_DATA. In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), the fbt and rawfbt providers are combined into a single file. Thus, two uses of prvname collide. The in-file collisions get resolved, but the cascade of macro definitions does not: FBT_GROUP_DATA ends up using "fbt" for both fbt and rawfbt providers. As a result, it is possible for rawfbt probes not to be seen. Notice that GROUP_DATA is always paired with prp->desc->prb. Therefore, simply replace: -#define GROUP_DATA getpid(), prvname +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb This makes the code more compact and relieves the macro definitions from needing prvname. It also makes the FBT_GROUP_DATA macro unnecessary. The format (FMT) macro is similarly renamed and redefined to include the probe info. Add a test to check that a rawfbt probe can be found behind an fbt probe when the probe description has a wildcard provider. Orabug: 38842114 Signed-off-by: Eugene Loh <eugene.loh@oracle.com> --- libdtrace/dt_prov_dtrace.c | 14 ++++++------- libdtrace/dt_prov_fbt.c | 13 ++++++------ libdtrace/dt_prov_rawtp.c | 4 ++-- libdtrace/dt_prov_sdt.c | 4 ++-- libdtrace/dt_provider_tp.h | 12 +++++------ .../providers/rawfbt/tst.wildcard-provider.d | 20 +++++++++++++++++++ .../providers/rawfbt/tst.wildcard-provider.r | 2 ++ 7 files changed, 45 insertions(+), 24 deletions(-) create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.d create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.r diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c index 34b5d8e24..102afd84e 100644 --- a/libdtrace/dt_prov_dtrace.c +++ b/libdtrace/dt_prov_dtrace.c @@ -243,8 +243,8 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) /* add a uprobe */ fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND); if (fd != -1) { - rc = dprintf(fd, "p:" GROUP_FMT "/%s %s\n", - GROUP_DATA, prp->desc->prb, spec); + rc = dprintf(fd, "p:" PROBE_FMT " %s\n", + PROBE_DATA, spec); close(fd); } free(spec); @@ -252,14 +252,14 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) return -ENOENT; /* open format file */ - len = snprintf(NULL, 0, "%s" GROUP_FMT "/%s/format", - EVENTSFS, GROUP_DATA, prp->desc->prb) + 1; + len = snprintf(NULL, 0, "%s" PROBE_FMT "/format", + EVENTSFS, PROBE_DATA) + 1; fn = dt_alloc(dtp, len); if (fn == NULL) return -ENOENT; - snprintf(fn, len, "%s" GROUP_FMT "/%s/format", - EVENTSFS, GROUP_DATA, prp->desc->prb); + snprintf(fn, len, "%s" PROBE_FMT "/format", + EVENTSFS, PROBE_DATA); f = fopen(fn, "r"); dt_free(dtp, fn); if (f == NULL) @@ -300,7 +300,7 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) if (fd == -1) return; - dprintf(fd, "-:" GROUP_FMT "/%s\n", GROUP_DATA, prp->desc->prb); + dprintf(fd, "-:" PROBE_FMT "\n", PROBE_DATA); close(fd); } diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c index 3feac56ea..8dbf9740a 100644 --- a/libdtrace/dt_prov_fbt.c +++ b/libdtrace/dt_prov_fbt.c @@ -53,8 +53,7 @@ static const char prvname[] = "fbt"; #define KPROBE_EVENTS TRACEFS "kprobe_events" -#define FBT_GROUP_FMT GROUP_FMT "_%s" -#define FBT_GROUP_DATA GROUP_DATA, prp->desc->prb +#define FBT_PROBE_FMT "dt_%d_%s_%s" static const dtrace_pattr_t pattr = { { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON }, @@ -508,16 +507,16 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) if (fd == -1) goto out; - rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n", + rc = dprintf(fd, "%c:" FBT_PROBE_FMT "/%s %s\n", prp->desc->prb[0] == 'e' ? 'p' : 'r', - FBT_GROUP_DATA, tpn, fun); + PROBE_DATA, tpn, fun); close(fd); if (rc == -1) goto out; /* create format file name */ - if (asprintf(&fn, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS, - FBT_GROUP_DATA, tpn) == -1) + if (asprintf(&fn, "%s" FBT_PROBE_FMT "/%s/format", EVENTSFS, + PROBE_DATA, tpn) == -1) goto out; /* open format file */ @@ -583,7 +582,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) } } - dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn); + dprintf(fd, "-:" FBT_PROBE_FMT "/%s\n", PROBE_DATA, tpn); close(fd); if (tpn != prp->desc->fun) diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c index 709f7040f..e3269f4b3 100644 --- a/libdtrace/dt_prov_rawtp.c +++ b/libdtrace/dt_prov_rawtp.c @@ -56,7 +56,7 @@ static const dtrace_pattr_t pattr = { /* * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. * We need to ignore these groups: - * - GROUP_FMT (created by DTrace processes) + * - PROBE_SFMT * - kprobes and uprobes * - syscalls (handled by a different provider) * - pid and usdt probes (ditto) @@ -89,7 +89,7 @@ static int populate(dtrace_hdl_t *dtp) *p++ = '\0'; - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { free(str); continue; } diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c index 74555f5be..ee2cc3c27 100644 --- a/libdtrace/dt_prov_sdt.c +++ b/libdtrace/dt_prov_sdt.c @@ -54,7 +54,7 @@ static const dtrace_pattr_t pattr = { /* * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. * We need to ignore these groups: - * - GROUP_FMT (created by DTrace processes) + * - PROBE_SFMT * - kprobes and uprobes * - syscalls (handled by a different provider) * - pid and usdt probes (ditto) @@ -87,7 +87,7 @@ static int populate(dtrace_hdl_t *dtp) *p++ = '\0'; - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { free(str); continue; } diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h index f131b1e02..406609d1b 100644 --- a/libdtrace/dt_provider_tp.h +++ b/libdtrace/dt_provider_tp.h @@ -18,13 +18,13 @@ extern "C" { * Tracepoint group naming format for DTrace providers. Providers may append * to this format string as needed. * - * GROUP_DATA provides the necessary data items to populate the format string - * (PID of the dtrace process and the provider name). GROUP_SFMT is like - * GROUP_FMT, but for sscanf(). + * PROBE_DATA provides the necessary data items to populate the format string. + * PROBE_FMT formats that data. + * PROBE_SFMT is a format string for recognizing PROBE_FMT data in sscanf(). */ -#define GROUP_FMT "dt_%d_%s" -#define GROUP_SFMT "dt_%d_%ms" -#define GROUP_DATA getpid(), prvname +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb +#define PROBE_FMT "dt_%d_%s/%s" +#define PROBE_SFMT "dt_%d_%ms" typedef struct tp_probe tp_probe_t; diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d new file mode 100644 index 000000000..ad9f4c779 --- /dev/null +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.d @@ -0,0 +1,20 @@ +/* + * Oracle Linux DTrace. + * Copyright (c) 2026, 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: rawfbt probes can be found even when a wildcard provider + * description also allows fbt probes. + */ + +#pragma D option quiet + +BEGIN, +*fbt:vmlinux:do_sys_open*:entry +{ + printf("success\n"); + exit(0); +} diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.r b/test/unittest/providers/rawfbt/tst.wildcard-provider.r new file mode 100644 index 000000000..b34a52d2f --- /dev/null +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.r @@ -0,0 +1,2 @@ +success + -- 2.47.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-13 21:42 ` [PATCH 2/2] rawfbt: prvname is not properly set eugene.loh @ 2026-01-15 22:18 ` Kris Van Hees 2026-01-16 5:18 ` Eugene Loh 0 siblings, 1 reply; 9+ messages in thread From: Kris Van Hees @ 2026-01-15 22:18 UTC (permalink / raw) To: eugene.loh; +Cc: dtrace, dtrace-devel On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh@oracle.com wrote: > From: Eugene Loh <eugene.loh@oracle.com> > > The char array prvname[] is set for each provider. It is used in the > file that implements the provider. It might also be passed to > dt_sdt_populate() via a function argument. > > However, dt_provider_tp.h also defines the macro GROUP_DATA in terms of > prvname. In turn, GROUP_DATA is used not only in dt_prov_dtrace.c but > then again in dt_prov_fbt.c to define FBT_GROUP_DATA. > > In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), > the fbt and rawfbt providers are combined into a single file. Thus, two > uses of prvname collide. The in-file collisions get resolved, but the > cascade of macro definitions does not: FBT_GROUP_DATA ends up using > "fbt" for both fbt and rawfbt providers. As a result, it is possible > for rawfbt probes not to be seen. > > Notice that GROUP_DATA is always paired with prp->desc->prb. Therefore, > simply replace: > -#define GROUP_DATA getpid(), prvname > +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb > This makes the code more compact and relieves the macro definitions from > needing prvname. It also makes the FBT_GROUP_DATA macro unnecessary. > > The format (FMT) macro is similarly renamed and redefined to include the > probe info. > > Add a test to check that a rawfbt probe can be found behind an fbt probe > when the probe description has a wildcard provider. > > Orabug: 38842114 > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> > --- > libdtrace/dt_prov_dtrace.c | 14 ++++++------- > libdtrace/dt_prov_fbt.c | 13 ++++++------ > libdtrace/dt_prov_rawtp.c | 4 ++-- > libdtrace/dt_prov_sdt.c | 4 ++-- > libdtrace/dt_provider_tp.h | 12 +++++------ > .../providers/rawfbt/tst.wildcard-provider.d | 20 +++++++++++++++++++ > .../providers/rawfbt/tst.wildcard-provider.r | 2 ++ > 7 files changed, 45 insertions(+), 24 deletions(-) > create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.d > create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.r > > diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c > index 34b5d8e24..102afd84e 100644 > --- a/libdtrace/dt_prov_dtrace.c > +++ b/libdtrace/dt_prov_dtrace.c > @@ -243,8 +243,8 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > /* add a uprobe */ > fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND); > if (fd != -1) { > - rc = dprintf(fd, "p:" GROUP_FMT "/%s %s\n", > - GROUP_DATA, prp->desc->prb, spec); > + rc = dprintf(fd, "p:" PROBE_FMT " %s\n", > + PROBE_DATA, spec); > close(fd); > } > free(spec); > @@ -252,14 +252,14 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > return -ENOENT; > > /* open format file */ > - len = snprintf(NULL, 0, "%s" GROUP_FMT "/%s/format", > - EVENTSFS, GROUP_DATA, prp->desc->prb) + 1; > + len = snprintf(NULL, 0, "%s" PROBE_FMT "/format", > + EVENTSFS, PROBE_DATA) + 1; > fn = dt_alloc(dtp, len); > if (fn == NULL) > return -ENOENT; > > - snprintf(fn, len, "%s" GROUP_FMT "/%s/format", > - EVENTSFS, GROUP_DATA, prp->desc->prb); > + snprintf(fn, len, "%s" PROBE_FMT "/format", > + EVENTSFS, PROBE_DATA); How about also changing this to use asprintf() rather than doing the douoble snprintf (one to get the langth, the alloc, and then actually populate). > f = fopen(fn, "r"); > dt_free(dtp, fn); > if (f == NULL) > @@ -300,7 +300,7 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) > if (fd == -1) > return; > > - dprintf(fd, "-:" GROUP_FMT "/%s\n", GROUP_DATA, prp->desc->prb); > + dprintf(fd, "-:" PROBE_FMT "\n", PROBE_DATA); > close(fd); > } > > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c > index 3feac56ea..8dbf9740a 100644 > --- a/libdtrace/dt_prov_fbt.c > +++ b/libdtrace/dt_prov_fbt.c > @@ -53,8 +53,7 @@ static const char prvname[] = "fbt"; > > #define KPROBE_EVENTS TRACEFS "kprobe_events" > > -#define FBT_GROUP_FMT GROUP_FMT "_%s" > -#define FBT_GROUP_DATA GROUP_DATA, prp->desc->prb > +#define FBT_PROBE_FMT "dt_%d_%s_%s" > > static const dtrace_pattr_t pattr = { > { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON }, > @@ -508,16 +507,16 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > if (fd == -1) > goto out; > > - rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n", > + rc = dprintf(fd, "%c:" FBT_PROBE_FMT "/%s %s\n", > prp->desc->prb[0] == 'e' ? 'p' : 'r', > - FBT_GROUP_DATA, tpn, fun); > + PROBE_DATA, tpn, fun); > close(fd); > if (rc == -1) > goto out; > > /* create format file name */ > - if (asprintf(&fn, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS, > - FBT_GROUP_DATA, tpn) == -1) > + if (asprintf(&fn, "%s" FBT_PROBE_FMT "/%s/format", EVENTSFS, > + PROBE_DATA, tpn) == -1) > goto out; > > /* open format file */ > @@ -583,7 +582,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) > } > } > > - dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn); > + dprintf(fd, "-:" FBT_PROBE_FMT "/%s\n", PROBE_DATA, tpn); > close(fd); > > if (tpn != prp->desc->fun) > diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c > index 709f7040f..e3269f4b3 100644 > --- a/libdtrace/dt_prov_rawtp.c > +++ b/libdtrace/dt_prov_rawtp.c > @@ -56,7 +56,7 @@ static const dtrace_pattr_t pattr = { > /* > * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. > * We need to ignore these groups: > - * - GROUP_FMT (created by DTrace processes) > + * - PROBE_SFMT > * - kprobes and uprobes > * - syscalls (handled by a different provider) > * - pid and usdt probes (ditto) > @@ -89,7 +89,7 @@ static int populate(dtrace_hdl_t *dtp) > > *p++ = '\0'; > > - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { > + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { > free(str); > continue; > } > diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c > index 74555f5be..ee2cc3c27 100644 > --- a/libdtrace/dt_prov_sdt.c > +++ b/libdtrace/dt_prov_sdt.c > @@ -54,7 +54,7 @@ static const dtrace_pattr_t pattr = { > /* > * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. > * We need to ignore these groups: > - * - GROUP_FMT (created by DTrace processes) > + * - PROBE_SFMT > * - kprobes and uprobes > * - syscalls (handled by a different provider) > * - pid and usdt probes (ditto) > @@ -87,7 +87,7 @@ static int populate(dtrace_hdl_t *dtp) > > *p++ = '\0'; > > - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { > + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { > free(str); > continue; > } > diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h > index f131b1e02..406609d1b 100644 > --- a/libdtrace/dt_provider_tp.h > +++ b/libdtrace/dt_provider_tp.h > @@ -18,13 +18,13 @@ extern "C" { > * Tracepoint group naming format for DTrace providers. Providers may append > * to this format string as needed. > * > - * GROUP_DATA provides the necessary data items to populate the format string > - * (PID of the dtrace process and the provider name). GROUP_SFMT is like > - * GROUP_FMT, but for sscanf(). > + * PROBE_DATA provides the necessary data items to populate the format string. > + * PROBE_FMT formats that data. > + * PROBE_SFMT is a format string for recognizing PROBE_FMT data in sscanf(). > */ > -#define GROUP_FMT "dt_%d_%s" > -#define GROUP_SFMT "dt_%d_%ms" > -#define GROUP_DATA getpid(), prvname > +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb > +#define PROBE_FMT "dt_%d_%s/%s" > +#define PROBE_SFMT "dt_%d_%ms" > > typedef struct tp_probe tp_probe_t; > > diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d > new file mode 100644 > index 000000000..ad9f4c779 > --- /dev/null > +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.d > @@ -0,0 +1,20 @@ > +/* > + * Oracle Linux DTrace. > + * Copyright (c) 2026, 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: rawfbt probes can be found even when a wildcard provider > + * description also allows fbt probes. > + */ > + > +#pragma D option quiet > + > +BEGIN, > +*fbt:vmlinux:do_sys_open*:entry > +{ > + printf("success\n"); > + exit(0); > +} I am confused how this test accomplishes verification of the assertion. If a regular FBT probe satisfies the probe description, wouldn't this pass also? Wouldn't a -l based test be better here, where you can test that the output does indeed report both the fbt probe and the rawfbt probe that match? > diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.r b/test/unittest/providers/rawfbt/tst.wildcard-provider.r > new file mode 100644 > index 000000000..b34a52d2f > --- /dev/null > +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.r > @@ -0,0 +1,2 @@ > +success > + > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-15 22:18 ` Kris Van Hees @ 2026-01-16 5:18 ` Eugene Loh 2026-01-16 22:02 ` Kris Van Hees 0 siblings, 1 reply; 9+ messages in thread From: Eugene Loh @ 2026-01-16 5:18 UTC (permalink / raw) To: Kris Van Hees; +Cc: dtrace, dtrace-devel On 1/15/26 17:18, Kris Van Hees wrote: > On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh@oracle.com wrote: >> From: Eugene Loh <eugene.loh@oracle.com> >> >> The char array prvname[] is set for each provider. It is used in the >> file that implements the provider. It might also be passed to >> dt_sdt_populate() via a function argument. >> >> However, dt_provider_tp.h also defines the macro GROUP_DATA in terms of >> prvname. In turn, GROUP_DATA is used not only in dt_prov_dtrace.c but >> then again in dt_prov_fbt.c to define FBT_GROUP_DATA. >> >> In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), >> the fbt and rawfbt providers are combined into a single file. Thus, two >> uses of prvname collide. The in-file collisions get resolved, but the >> cascade of macro definitions does not: FBT_GROUP_DATA ends up using >> "fbt" for both fbt and rawfbt providers. As a result, it is possible >> for rawfbt probes not to be seen. >> >> Notice that GROUP_DATA is always paired with prp->desc->prb. Therefore, >> simply replace: >> -#define GROUP_DATA getpid(), prvname >> +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb >> This makes the code more compact and relieves the macro definitions from >> needing prvname. It also makes the FBT_GROUP_DATA macro unnecessary. >> >> The format (FMT) macro is similarly renamed and redefined to include the >> probe info. >> >> Add a test to check that a rawfbt probe can be found behind an fbt probe >> when the probe description has a wildcard provider. >> >> Orabug: 38842114 >> Signed-off-by: Eugene Loh <eugene.loh@oracle.com> >> --- >> libdtrace/dt_prov_dtrace.c | 14 ++++++------- >> libdtrace/dt_prov_fbt.c | 13 ++++++------ >> libdtrace/dt_prov_rawtp.c | 4 ++-- >> libdtrace/dt_prov_sdt.c | 4 ++-- >> libdtrace/dt_provider_tp.h | 12 +++++------ >> .../providers/rawfbt/tst.wildcard-provider.d | 20 +++++++++++++++++++ >> .../providers/rawfbt/tst.wildcard-provider.r | 2 ++ >> 7 files changed, 45 insertions(+), 24 deletions(-) >> create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.d >> create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.r >> >> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c >> index 34b5d8e24..102afd84e 100644 >> --- a/libdtrace/dt_prov_dtrace.c >> +++ b/libdtrace/dt_prov_dtrace.c >> @@ -243,8 +243,8 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) >> /* add a uprobe */ >> fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND); >> if (fd != -1) { >> - rc = dprintf(fd, "p:" GROUP_FMT "/%s %s\n", >> - GROUP_DATA, prp->desc->prb, spec); >> + rc = dprintf(fd, "p:" PROBE_FMT " %s\n", >> + PROBE_DATA, spec); >> close(fd); >> } >> free(spec); >> @@ -252,14 +252,14 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) >> return -ENOENT; >> >> /* open format file */ >> - len = snprintf(NULL, 0, "%s" GROUP_FMT "/%s/format", >> - EVENTSFS, GROUP_DATA, prp->desc->prb) + 1; >> + len = snprintf(NULL, 0, "%s" PROBE_FMT "/format", >> + EVENTSFS, PROBE_DATA) + 1; >> fn = dt_alloc(dtp, len); >> if (fn == NULL) >> return -ENOENT; >> >> - snprintf(fn, len, "%s" GROUP_FMT "/%s/format", >> - EVENTSFS, GROUP_DATA, prp->desc->prb); >> + snprintf(fn, len, "%s" PROBE_FMT "/format", >> + EVENTSFS, PROBE_DATA); > How about also changing this to use asprintf() rather than doing the douoble > snprintf (one to get the langth, the alloc, and then actually populate). Sure, sounds good. On the other hand, this is in dt_prov_dtrace, while the main point of the patch is an fbt problem. Yeah, I know, not a big deal. But since we've been chasing these asprintf() sites opportunistically over a couple of patches and there are several (4?) more sites to go, how about I do all these asprintf() conversions in a separate patch? >> f = fopen(fn, "r"); >> dt_free(dtp, fn); >> if (f == NULL) >> @@ -300,7 +300,7 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) >> if (fd == -1) >> return; >> >> - dprintf(fd, "-:" GROUP_FMT "/%s\n", GROUP_DATA, prp->desc->prb); >> + dprintf(fd, "-:" PROBE_FMT "\n", PROBE_DATA); >> close(fd); >> } >> >> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c >> index 3feac56ea..8dbf9740a 100644 >> --- a/libdtrace/dt_prov_fbt.c >> +++ b/libdtrace/dt_prov_fbt.c >> @@ -53,8 +53,7 @@ static const char prvname[] = "fbt"; >> >> #define KPROBE_EVENTS TRACEFS "kprobe_events" >> >> -#define FBT_GROUP_FMT GROUP_FMT "_%s" >> -#define FBT_GROUP_DATA GROUP_DATA, prp->desc->prb >> +#define FBT_PROBE_FMT "dt_%d_%s_%s" >> >> static const dtrace_pattr_t pattr = { >> { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON }, >> @@ -508,16 +507,16 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) >> if (fd == -1) >> goto out; >> >> - rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n", >> + rc = dprintf(fd, "%c:" FBT_PROBE_FMT "/%s %s\n", >> prp->desc->prb[0] == 'e' ? 'p' : 'r', >> - FBT_GROUP_DATA, tpn, fun); >> + PROBE_DATA, tpn, fun); >> close(fd); >> if (rc == -1) >> goto out; >> >> /* create format file name */ >> - if (asprintf(&fn, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS, >> - FBT_GROUP_DATA, tpn) == -1) >> + if (asprintf(&fn, "%s" FBT_PROBE_FMT "/%s/format", EVENTSFS, >> + PROBE_DATA, tpn) == -1) >> goto out; >> >> /* open format file */ >> @@ -583,7 +582,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) >> } >> } >> >> - dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn); >> + dprintf(fd, "-:" FBT_PROBE_FMT "/%s\n", PROBE_DATA, tpn); >> close(fd); >> >> if (tpn != prp->desc->fun) >> diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c >> index 709f7040f..e3269f4b3 100644 >> --- a/libdtrace/dt_prov_rawtp.c >> +++ b/libdtrace/dt_prov_rawtp.c >> @@ -56,7 +56,7 @@ static const dtrace_pattr_t pattr = { >> /* >> * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. >> * We need to ignore these groups: >> - * - GROUP_FMT (created by DTrace processes) >> + * - PROBE_SFMT >> * - kprobes and uprobes >> * - syscalls (handled by a different provider) >> * - pid and usdt probes (ditto) >> @@ -89,7 +89,7 @@ static int populate(dtrace_hdl_t *dtp) >> >> *p++ = '\0'; >> >> - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { >> + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { >> free(str); >> continue; >> } >> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c >> index 74555f5be..ee2cc3c27 100644 >> --- a/libdtrace/dt_prov_sdt.c >> +++ b/libdtrace/dt_prov_sdt.c >> @@ -54,7 +54,7 @@ static const dtrace_pattr_t pattr = { >> /* >> * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. >> * We need to ignore these groups: >> - * - GROUP_FMT (created by DTrace processes) >> + * - PROBE_SFMT >> * - kprobes and uprobes >> * - syscalls (handled by a different provider) >> * - pid and usdt probes (ditto) >> @@ -87,7 +87,7 @@ static int populate(dtrace_hdl_t *dtp) >> >> *p++ = '\0'; >> >> - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { >> + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { >> free(str); >> continue; >> } >> diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h >> index f131b1e02..406609d1b 100644 >> --- a/libdtrace/dt_provider_tp.h >> +++ b/libdtrace/dt_provider_tp.h >> @@ -18,13 +18,13 @@ extern "C" { >> * Tracepoint group naming format for DTrace providers. Providers may append >> * to this format string as needed. >> * >> - * GROUP_DATA provides the necessary data items to populate the format string >> - * (PID of the dtrace process and the provider name). GROUP_SFMT is like >> - * GROUP_FMT, but for sscanf(). >> + * PROBE_DATA provides the necessary data items to populate the format string. >> + * PROBE_FMT formats that data. >> + * PROBE_SFMT is a format string for recognizing PROBE_FMT data in sscanf(). >> */ >> -#define GROUP_FMT "dt_%d_%s" >> -#define GROUP_SFMT "dt_%d_%ms" >> -#define GROUP_DATA getpid(), prvname >> +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb >> +#define PROBE_FMT "dt_%d_%s/%s" >> +#define PROBE_SFMT "dt_%d_%ms" >> >> typedef struct tp_probe tp_probe_t; >> >> diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d >> new file mode 100644 >> index 000000000..ad9f4c779 >> --- /dev/null >> +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.d >> @@ -0,0 +1,20 @@ >> +/* >> + * Oracle Linux DTrace. >> + * Copyright (c) 2026, 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: rawfbt probes can be found even when a wildcard provider >> + * description also allows fbt probes. >> + */ >> + >> +#pragma D option quiet >> + >> +BEGIN, >> +*fbt:vmlinux:do_sys_open*:entry >> +{ >> + printf("success\n"); >> + exit(0); >> +} > I am confused how this test accomplishes verification of the assertion. If > a regular FBT probe satisfies the probe description, wouldn't this pass also? > Wouldn't a -l based test be better here, where you can test that the output > does indeed report both the fbt probe and the rawfbt probe that match? It appears that -l is not an issue; a -l test does not show the problem. The test in the patch is derived from the user bug report that motivated this patch. The whole prvname problem is that the bad prvname value makes GROUP_DATA bad, which makes FBT_GROUP_DATA bad, which is an issue only in kprobe_attach (and detach). So, we need kprobe_attach() to see the problem. >> diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.r b/test/unittest/providers/rawfbt/tst.wildcard-provider.r >> new file mode 100644 >> index 000000000..b34a52d2f >> --- /dev/null >> +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.r >> @@ -0,0 +1,2 @@ >> +success >> + >> -- >> 2.47.3 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-16 5:18 ` Eugene Loh @ 2026-01-16 22:02 ` Kris Van Hees 2026-01-16 22:43 ` Eugene Loh 0 siblings, 1 reply; 9+ messages in thread From: Kris Van Hees @ 2026-01-16 22:02 UTC (permalink / raw) To: Eugene Loh; +Cc: dtrace, dtrace-devel On Fri, Jan 16, 2026 at 12:18:51AM -0500, Eugene Loh wrote: > On 1/15/26 17:18, Kris Van Hees wrote: > > > On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh@oracle.com wrote: > > > From: Eugene Loh <eugene.loh@oracle.com> > > > > > > The char array prvname[] is set for each provider. It is used in the > > > file that implements the provider. It might also be passed to > > > dt_sdt_populate() via a function argument. > > > > > > However, dt_provider_tp.h also defines the macro GROUP_DATA in terms of > > > prvname. In turn, GROUP_DATA is used not only in dt_prov_dtrace.c but > > > then again in dt_prov_fbt.c to define FBT_GROUP_DATA. > > > > > > In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), > > > the fbt and rawfbt providers are combined into a single file. Thus, two > > > uses of prvname collide. The in-file collisions get resolved, but the > > > cascade of macro definitions does not: FBT_GROUP_DATA ends up using > > > "fbt" for both fbt and rawfbt providers. As a result, it is possible > > > for rawfbt probes not to be seen. > > > > > > Notice that GROUP_DATA is always paired with prp->desc->prb. Therefore, > > > simply replace: > > > -#define GROUP_DATA getpid(), prvname > > > +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb > > > This makes the code more compact and relieves the macro definitions from > > > needing prvname. It also makes the FBT_GROUP_DATA macro unnecessary. > > > > > > The format (FMT) macro is similarly renamed and redefined to include the > > > probe info. > > > > > > Add a test to check that a rawfbt probe can be found behind an fbt probe > > > when the probe description has a wildcard provider. > > > > > > Orabug: 38842114 > > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> > > > --- > > > libdtrace/dt_prov_dtrace.c | 14 ++++++------- > > > libdtrace/dt_prov_fbt.c | 13 ++++++------ > > > libdtrace/dt_prov_rawtp.c | 4 ++-- > > > libdtrace/dt_prov_sdt.c | 4 ++-- > > > libdtrace/dt_provider_tp.h | 12 +++++------ > > > .../providers/rawfbt/tst.wildcard-provider.d | 20 +++++++++++++++++++ > > > .../providers/rawfbt/tst.wildcard-provider.r | 2 ++ > > > 7 files changed, 45 insertions(+), 24 deletions(-) > > > create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.d > > > create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.r > > > > > > diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c > > > index 34b5d8e24..102afd84e 100644 > > > --- a/libdtrace/dt_prov_dtrace.c > > > +++ b/libdtrace/dt_prov_dtrace.c > > > @@ -243,8 +243,8 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > > > /* add a uprobe */ > > > fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND); > > > if (fd != -1) { > > > - rc = dprintf(fd, "p:" GROUP_FMT "/%s %s\n", > > > - GROUP_DATA, prp->desc->prb, spec); > > > + rc = dprintf(fd, "p:" PROBE_FMT " %s\n", > > > + PROBE_DATA, spec); > > > close(fd); > > > } > > > free(spec); > > > @@ -252,14 +252,14 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > > > return -ENOENT; > > > /* open format file */ > > > - len = snprintf(NULL, 0, "%s" GROUP_FMT "/%s/format", > > > - EVENTSFS, GROUP_DATA, prp->desc->prb) + 1; > > > + len = snprintf(NULL, 0, "%s" PROBE_FMT "/format", > > > + EVENTSFS, PROBE_DATA) + 1; > > > fn = dt_alloc(dtp, len); > > > if (fn == NULL) > > > return -ENOENT; > > > - snprintf(fn, len, "%s" GROUP_FMT "/%s/format", > > > - EVENTSFS, GROUP_DATA, prp->desc->prb); > > > + snprintf(fn, len, "%s" PROBE_FMT "/format", > > > + EVENTSFS, PROBE_DATA); > > How about also changing this to use asprintf() rather than doing the douoble > > snprintf (one to get the langth, the alloc, and then actually populate). > > Sure, sounds good. On the other hand, this is in dt_prov_dtrace, while the > main point of the patch is an fbt problem. Yeah, I know, not a big deal. > But since we've been chasing these asprintf() sites opportunistically over a > couple of patches and there are several (4?) more sites to go, how about I > do all these asprintf() conversions in a separate patch? Sounds good (and thank you for already posting those). > > > f = fopen(fn, "r"); > > > dt_free(dtp, fn); > > > if (f == NULL) > > > @@ -300,7 +300,7 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) > > > if (fd == -1) > > > return; > > > - dprintf(fd, "-:" GROUP_FMT "/%s\n", GROUP_DATA, prp->desc->prb); > > > + dprintf(fd, "-:" PROBE_FMT "\n", PROBE_DATA); > > > close(fd); > > > } > > > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c > > > index 3feac56ea..8dbf9740a 100644 > > > --- a/libdtrace/dt_prov_fbt.c > > > +++ b/libdtrace/dt_prov_fbt.c > > > @@ -53,8 +53,7 @@ static const char prvname[] = "fbt"; > > > #define KPROBE_EVENTS TRACEFS "kprobe_events" > > > -#define FBT_GROUP_FMT GROUP_FMT "_%s" > > > -#define FBT_GROUP_DATA GROUP_DATA, prp->desc->prb > > > +#define FBT_PROBE_FMT "dt_%d_%s_%s" > > > static const dtrace_pattr_t pattr = { > > > { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON }, > > > @@ -508,16 +507,16 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd) > > > if (fd == -1) > > > goto out; > > > - rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n", > > > + rc = dprintf(fd, "%c:" FBT_PROBE_FMT "/%s %s\n", > > > prp->desc->prb[0] == 'e' ? 'p' : 'r', > > > - FBT_GROUP_DATA, tpn, fun); > > > + PROBE_DATA, tpn, fun); > > > close(fd); > > > if (rc == -1) > > > goto out; > > > /* create format file name */ > > > - if (asprintf(&fn, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS, > > > - FBT_GROUP_DATA, tpn) == -1) > > > + if (asprintf(&fn, "%s" FBT_PROBE_FMT "/%s/format", EVENTSFS, > > > + PROBE_DATA, tpn) == -1) > > > goto out; > > > /* open format file */ > > > @@ -583,7 +582,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp) > > > } > > > } > > > - dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn); > > > + dprintf(fd, "-:" FBT_PROBE_FMT "/%s\n", PROBE_DATA, tpn); > > > close(fd); > > > if (tpn != prp->desc->fun) > > > diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c > > > index 709f7040f..e3269f4b3 100644 > > > --- a/libdtrace/dt_prov_rawtp.c > > > +++ b/libdtrace/dt_prov_rawtp.c > > > @@ -56,7 +56,7 @@ static const dtrace_pattr_t pattr = { > > > /* > > > * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. > > > * We need to ignore these groups: > > > - * - GROUP_FMT (created by DTrace processes) > > > + * - PROBE_SFMT > > > * - kprobes and uprobes > > > * - syscalls (handled by a different provider) > > > * - pid and usdt probes (ditto) > > > @@ -89,7 +89,7 @@ static int populate(dtrace_hdl_t *dtp) > > > *p++ = '\0'; > > > - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { > > > + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { > > > free(str); > > > continue; > > > } > > > diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c > > > index 74555f5be..ee2cc3c27 100644 > > > --- a/libdtrace/dt_prov_sdt.c > > > +++ b/libdtrace/dt_prov_sdt.c > > > @@ -54,7 +54,7 @@ static const dtrace_pattr_t pattr = { > > > /* > > > * The PROBE_LIST file lists all tracepoints in a <group>:<name> format. > > > * We need to ignore these groups: > > > - * - GROUP_FMT (created by DTrace processes) > > > + * - PROBE_SFMT > > > * - kprobes and uprobes > > > * - syscalls (handled by a different provider) > > > * - pid and usdt probes (ditto) > > > @@ -87,7 +87,7 @@ static int populate(dtrace_hdl_t *dtp) > > > *p++ = '\0'; > > > - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) { > > > + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) { > > > free(str); > > > continue; > > > } > > > diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h > > > index f131b1e02..406609d1b 100644 > > > --- a/libdtrace/dt_provider_tp.h > > > +++ b/libdtrace/dt_provider_tp.h > > > @@ -18,13 +18,13 @@ extern "C" { > > > * Tracepoint group naming format for DTrace providers. Providers may append > > > * to this format string as needed. > > > * > > > - * GROUP_DATA provides the necessary data items to populate the format string > > > - * (PID of the dtrace process and the provider name). GROUP_SFMT is like > > > - * GROUP_FMT, but for sscanf(). > > > + * PROBE_DATA provides the necessary data items to populate the format string. > > > + * PROBE_FMT formats that data. > > > + * PROBE_SFMT is a format string for recognizing PROBE_FMT data in sscanf(). > > > */ > > > -#define GROUP_FMT "dt_%d_%s" > > > -#define GROUP_SFMT "dt_%d_%ms" > > > -#define GROUP_DATA getpid(), prvname > > > +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb > > > +#define PROBE_FMT "dt_%d_%s/%s" > > > +#define PROBE_SFMT "dt_%d_%ms" > > > typedef struct tp_probe tp_probe_t; > > > diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d > > > new file mode 100644 > > > index 000000000..ad9f4c779 > > > --- /dev/null > > > +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.d > > > @@ -0,0 +1,20 @@ > > > +/* > > > + * Oracle Linux DTrace. > > > + * Copyright (c) 2026, 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: rawfbt probes can be found even when a wildcard provider > > > + * description also allows fbt probes. > > > + */ > > > + > > > +#pragma D option quiet > > > + > > > +BEGIN, > > > +*fbt:vmlinux:do_sys_open*:entry > > > +{ > > > + printf("success\n"); > > > + exit(0); > > > +} > > I am confused how this test accomplishes verification of the assertion. If > > a regular FBT probe satisfies the probe description, wouldn't this pass also? > > Wouldn't a -l based test be better here, where you can test that the output > > does indeed report both the fbt probe and the rawfbt probe that match? > > It appears that -l is not an issue; a -l test does not show the problem. > The test in the patch is derived from the user bug report that motivated > this patch. > > The whole prvname problem is that the bad prvname value makes GROUP_DATA > bad, which makes FBT_GROUP_DATA bad, which is an issue only in kprobe_attach > (and detach). So, we need kprobe_attach() to see the problem. Ok, so yes, -l is not sufficient. But still, even if the test is derived from the original reported problem, it still looks wrong to me. Or rather, the test can pass when fbt:vmlinux:do_sys+open*:entry exists rawfbt:vmlinux:do_sys+open*:entry does NOT exist because as long as 1 probe matches the probe specification, compilation will succeed (and at runtime, the probe firing will complete the success criterium). But that is not a valid PASS result for the assertion that is posed for this test. If you are specifically looking for a rawfbt probe using a wildcard, you might want to use r*fbt:vmlinux:do_sys+open*:entry instead perhaps? > > > diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.r b/test/unittest/providers/rawfbt/tst.wildcard-provider.r > > > new file mode 100644 > > > index 000000000..b34a52d2f > > > --- /dev/null > > > +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.r > > > @@ -0,0 +1,2 @@ > > > +success > > > + > > > -- > > > 2.47.3 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-16 22:02 ` Kris Van Hees @ 2026-01-16 22:43 ` Eugene Loh 2026-01-26 21:11 ` Kris Van Hees 0 siblings, 1 reply; 9+ messages in thread From: Eugene Loh @ 2026-01-16 22:43 UTC (permalink / raw) To: Kris Van Hees; +Cc: dtrace, dtrace-devel On 1/16/26 17:02, Kris Van Hees wrote: > On Fri, Jan 16, 2026 at 12:18:51AM -0500, Eugene Loh wrote: >> On 1/15/26 17:18, Kris Van Hees wrote: >>> On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh@oracle.com wrote: >>>> From: Eugene Loh <eugene.loh@oracle.com> >>>> >>>> [...] >>>> >>>> Add a test to check that a rawfbt probe can be found behind an fbt probe >>>> when the probe description has a wildcard provider. >>>> >>>> Orabug: 38842114 >>>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com> >>>> >>>> diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d >>>> new file mode 100644 >>>> @@ -0,0 +1,20 @@ >>>> +/* >>>> + * ASSERTION: rawfbt probes can be found even when a wildcard provider >>>> + * description also allows fbt probes. >>>> + */ >>>> + >>>> +#pragma D option quiet >>>> + >>>> +BEGIN, >>>> +*fbt:vmlinux:do_sys_open*:entry >>>> +{ >>>> + printf("success\n"); >>>> + exit(0); >>>> +} >>> I am confused how this test accomplishes verification of the assertion. If >>> a regular FBT probe satisfies the probe description, wouldn't this pass also? >>> Wouldn't a -l based test be better here, where you can test that the output >>> does indeed report both the fbt probe and the rawfbt probe that match? >> It appears that -l is not an issue; a -l test does not show the problem. >> The test in the patch is derived from the user bug report that motivated >> this patch. >> >> The whole prvname problem is that the bad prvname value makes GROUP_DATA >> bad, which makes FBT_GROUP_DATA bad, which is an issue only in kprobe_attach >> (and detach). So, we need kprobe_attach() to see the problem. > Ok, so yes, -l is not sufficient. But still, even if the test is derived from > the original reported problem, it still looks wrong to me. Or rather, the test > can pass when > > fbt:vmlinux:do_sys+open*:entry exists > rawfbt:vmlinux:do_sys+open*:entry does NOT exist > > because as long as 1 probe matches the probe specification, compilation will > succeed (and at runtime, the probe firing will complete the success criterium). > > But that is not a valid PASS result for the assertion that is posed for this > test. If you are specifically looking for a rawfbt probe using a wildcard, you > might want to use r*fbt:vmlinux:do_sys+open*:entry instead perhaps? This test is admittedly not the most stringent; it could well pass even if some other bugs lurk somewhere. The test simply reports a problem with the old bits and then passes with the fix. It is a confirmation -- however weak -- that the fix is doing something right. Meanwhile, I do not understand your concerns with regards to the assertion. Specifically, using r*fbt would preclude any fbt probes; so there would be nothing for the rawfbt probe to "hide behind." And, fwiw, using r*fbt makes the test pass even without the fix. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] rawfbt: prvname is not properly set 2026-01-16 22:43 ` Eugene Loh @ 2026-01-26 21:11 ` Kris Van Hees 0 siblings, 0 replies; 9+ messages in thread From: Kris Van Hees @ 2026-01-26 21:11 UTC (permalink / raw) To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel On Fri, Jan 16, 2026 at 05:43:24PM -0500, Eugene Loh wrote: > On 1/16/26 17:02, Kris Van Hees wrote: > > > On Fri, Jan 16, 2026 at 12:18:51AM -0500, Eugene Loh wrote: > > > On 1/15/26 17:18, Kris Van Hees wrote: > > > > On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh@oracle.com wrote: > > > > > From: Eugene Loh <eugene.loh@oracle.com> > > > > > > > > > > [...] > > > > > > > > > > Add a test to check that a rawfbt probe can be found behind an fbt probe > > > > > when the probe description has a wildcard provider. > > > > > > > > > > Orabug: 38842114 > > > > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> > > > > > > > > > > diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d > > > > > new file mode 100644 > > > > > @@ -0,0 +1,20 @@ > > > > > +/* > > > > > + * ASSERTION: rawfbt probes can be found even when a wildcard provider > > > > > + * description also allows fbt probes. > > > > > + */ > > > > > + > > > > > +#pragma D option quiet > > > > > + > > > > > +BEGIN, > > > > > +*fbt:vmlinux:do_sys_open*:entry > > > > > +{ > > > > > + printf("success\n"); > > > > > + exit(0); > > > > > +} > > > > I am confused how this test accomplishes verification of the assertion. If > > > > a regular FBT probe satisfies the probe description, wouldn't this pass also? > > > > Wouldn't a -l based test be better here, where you can test that the output > > > > does indeed report both the fbt probe and the rawfbt probe that match? > > > It appears that -l is not an issue; a -l test does not show the problem. > > > The test in the patch is derived from the user bug report that motivated > > > this patch. > > > > > > The whole prvname problem is that the bad prvname value makes GROUP_DATA > > > bad, which makes FBT_GROUP_DATA bad, which is an issue only in kprobe_attach > > > (and detach). So, we need kprobe_attach() to see the problem. > > Ok, so yes, -l is not sufficient. But still, even if the test is derived from > > the original reported problem, it still looks wrong to me. Or rather, the test > > can pass when > > > > fbt:vmlinux:do_sys+open*:entry exists > > rawfbt:vmlinux:do_sys+open*:entry does NOT exist > > > > because as long as 1 probe matches the probe specification, compilation will > > succeed (and at runtime, the probe firing will complete the success criterium). > > > > But that is not a valid PASS result for the assertion that is posed for this > > test. If you are specifically looking for a rawfbt probe using a wildcard, you > > might want to use r*fbt:vmlinux:do_sys+open*:entry instead perhaps? > > This test is admittedly not the most stringent; it could well pass even if > some other bugs lurk somewhere. The test simply reports a problem with the > old bits and then passes with the fix. It is a confirmation -- however weak > -- that the fix is doing something right. > > Meanwhile, I do not understand your concerns with regards to the assertion. > Specifically, using r*fbt would preclude any fbt probes; so there would be > nothing for the rawfbt probe to "hide behind." And, fwiw, using r*fbt makes > the test pass even without the fix. Well, if we have a test it should validate the problem that is being fixed, and I do not see that happening here. If the problem occurs with the actual attach and it requires both fbt and rawfbt to match, then you should perhaps remove the quiet option, and make the expected output also show the number of probes that match the probe spec. And add a comment in the test that the problem validation is a combination of (1) validating that both the FBT and rawfbt probes are found to match, and (2) validating that the script executes which means that dtrace was able to attach to the matched probes. That seems like it would satisfy the assertion in a way that cannot happen if either of the *fbt probes is missing. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbt: Populate just once 2026-01-13 21:42 [PATCH 1/2] fbt: Populate just once eugene.loh 2026-01-13 21:42 ` [PATCH 2/2] rawfbt: prvname is not properly set eugene.loh @ 2026-01-15 21:35 ` Kris Van Hees 2026-01-15 22:07 ` Eugene Loh 1 sibling, 1 reply; 9+ messages in thread From: Kris Van Hees @ 2026-01-15 21:35 UTC (permalink / raw) To: eugene.loh; +Cc: dtrace, dtrace-devel On Tue, Jan 13, 2026 at 04:42:04PM -0500, eugene.loh@oracle.com wrote: > From: Eugene Loh <eugene.loh@oracle.com> > > In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), > the populate() functions for fbt and rawfbt were combined, populating both > providers, but the function was still called twice. That is, dt_open.c > tries to insert each provider twice. Where do you see evidence that the populate function is being called twice? Only the providers listed in the static initial list have populate() called, and while &dt_fbt is in that list, &dt_rawfbt is not. Also, adding debugging output to populate() in dt_prov_fbt.c (or enabling DTRACE_DEBUG) shows that it is only called once. > One solution would be to have a different populate() function for each > provider. > > Here, we employ another solution: in dt_provider_create(), check to see > if a specified provider has already been inserted. This change is not needed but it is also harmless and can avoid future problems so I would not object having this as a safety net. > This also requires a corresponding change in dt_provider_lookup(), so > that it will work even if no providers have yet been inserted. > > Also, fix a minor comment in the fbt provider. Thanks for this fix. > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> > --- > libdtrace/dt_prov_fbt.c | 2 +- > libdtrace/dt_provider.c | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c > index bbe44a842..3feac56ea 100644 > --- a/libdtrace/dt_prov_fbt.c > +++ b/libdtrace/dt_prov_fbt.c > @@ -228,7 +228,7 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) > if (!dt_symbol_traceable(sym)) > continue; > > - /* Function name cannot be synthetic and must match. */ > + /* Function name cannot be synthetic (unless rawfbt) and must match. */ > fun = dt_symbol_name(sym); > if ((!rawfbt && strchr(fun, '.')) || !dt_gmatch(fun, pdp->fun)) > continue; > diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c > index 848fdc132..9d75225fa 100644 > --- a/libdtrace/dt_provider.c > +++ b/libdtrace/dt_provider.c > @@ -114,6 +114,8 @@ dt_provider_lookup(dtrace_hdl_t *dtp, const char *name) > return NULL; > > strcpy(tmpl.desc.dtvd_name, name); > + if (dtp->dt_provs == NULL) > + return NULL; > return dt_htab_lookup(dtp->dt_provs, &tmpl); > } > > @@ -124,6 +126,10 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name, > { > dt_provider_t *pvp; > > + pvp = dt_provider_lookup(dtp, name); > + if (pvp) > + return pvp; > + > if ((pvp = dt_zalloc(dtp, sizeof(dt_provider_t))) == NULL) > goto nomem; > > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbt: Populate just once 2026-01-15 21:35 ` [PATCH 1/2] fbt: Populate just once Kris Van Hees @ 2026-01-15 22:07 ` Eugene Loh 0 siblings, 0 replies; 9+ messages in thread From: Eugene Loh @ 2026-01-15 22:07 UTC (permalink / raw) To: Kris Van Hees; +Cc: dtrace, dtrace-devel On 1/15/26 16:35, Kris Van Hees wrote: > On Tue, Jan 13, 2026 at 04:42:04PM -0500, eugene.loh@oracle.com wrote: >> From: Eugene Loh <eugene.loh@oracle.com> >> >> In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"), >> the populate() functions for fbt and rawfbt were combined, populating both >> providers, but the function was still called twice. That is, dt_open.c >> tries to insert each provider twice. > Where do you see evidence that the populate function is being called twice? > Only the providers listed in the static initial list have populate() called, > and while &dt_fbt is in that list, &dt_rawfbt is not. Also, adding debugging > output to populate() in dt_prov_fbt.c (or enabling DTRACE_DEBUG) shows that > it is only called once. I agree with you. Thanks. I'll resubmit the patch, keeping only the "harmless safety net" and typo correction, along with an appropriately reworded subject line. >> One solution would be to have a different populate() function for each >> provider. >> >> Here, we employ another solution: in dt_provider_create(), check to see >> if a specified provider has already been inserted. > This change is not needed but it is also harmless and can avoid future problems > so I would not object having this as a safety net. > >> This also requires a corresponding change in dt_provider_lookup(), so >> that it will work even if no providers have yet been inserted. >> >> Also, fix a minor comment in the fbt provider. > Thanks for this fix. > >> Signed-off-by: Eugene Loh <eugene.loh@oracle.com> >> --- >> libdtrace/dt_prov_fbt.c | 2 +- >> libdtrace/dt_provider.c | 6 ++++++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c >> index bbe44a842..3feac56ea 100644 >> --- a/libdtrace/dt_prov_fbt.c >> +++ b/libdtrace/dt_prov_fbt.c >> @@ -228,7 +228,7 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) >> if (!dt_symbol_traceable(sym)) >> continue; >> >> - /* Function name cannot be synthetic and must match. */ >> + /* Function name cannot be synthetic (unless rawfbt) and must match. */ >> fun = dt_symbol_name(sym); >> if ((!rawfbt && strchr(fun, '.')) || !dt_gmatch(fun, pdp->fun)) >> continue; >> diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c >> index 848fdc132..9d75225fa 100644 >> --- a/libdtrace/dt_provider.c >> +++ b/libdtrace/dt_provider.c >> @@ -114,6 +114,8 @@ dt_provider_lookup(dtrace_hdl_t *dtp, const char *name) >> return NULL; >> >> strcpy(tmpl.desc.dtvd_name, name); >> + if (dtp->dt_provs == NULL) >> + return NULL; >> return dt_htab_lookup(dtp->dt_provs, &tmpl); >> } >> >> @@ -124,6 +126,10 @@ dt_provider_create(dtrace_hdl_t *dtp, const char *name, >> { >> dt_provider_t *pvp; >> >> + pvp = dt_provider_lookup(dtp, name); >> + if (pvp) >> + return pvp; >> + >> if ((pvp = dt_zalloc(dtp, sizeof(dt_provider_t))) == NULL) >> goto nomem; >> >> -- >> 2.47.3 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-01-26 21:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-13 21:42 [PATCH 1/2] fbt: Populate just once eugene.loh 2026-01-13 21:42 ` [PATCH 2/2] rawfbt: prvname is not properly set eugene.loh 2026-01-15 22:18 ` Kris Van Hees 2026-01-16 5:18 ` Eugene Loh 2026-01-16 22:02 ` Kris Van Hees 2026-01-16 22:43 ` Eugene Loh 2026-01-26 21:11 ` Kris Van Hees 2026-01-15 21:35 ` [PATCH 1/2] fbt: Populate just once Kris Van Hees 2026-01-15 22:07 ` Eugene Loh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox