[DTrace-devel] [PATCH] syscall: fix syscall probe argument handling
Kris Van Hees
kris.van.hees at oracle.com
Tue Feb 3 20:04:10 UTC 2026
On Tue, Feb 03, 2026 at 02:53:03PM -0500, Eugene Loh wrote:
> I'm probably good with this patch but just wanted to ask/say a few tiny
> things:
>
> *) What is the testing situation? Is there a major outage of tests with
> 6.19 kernels?
Yes, this broke pretty much any syscall test for syscalls with a char * arg.
> *) In valid_arg(), s/cmmon/common/.
Thanks.
> *) In valid_arg(), why is there a fldc arg? If what we're trying to do is
> skip __syscall_nr, why not check strcmp(desc,"__syscall_nr")==0? I suppose
> that's a more expensive check, but it's more precise?
Sure, I can do that. I was going by the notion that the provider has knowledge
about the tracing infrastructure (which it has to), so we *know* for certain
that we need to skip the first field we are presented with, regardless of
whether they might change the name. So, I think that using the field count is
actually fine.
> On 2/3/26 11:23, Kris Van Hees via DTrace-devel wrote:
> > Kernel commit a544d9a66bdf
> > ("tracing: Have syscall trace events read user space string") (added in
> > 6.19-rc1) causes special fields to be added to the tracepoint format
> > file for data strings that are faulted in. We need to account for this
> > when determining the number of arguments for syscall tracepoints.
> >
> > We introduce support for a valid_arg() callback function that providers
> > can implement to support skipping initial non-common fields and also to
> > stop processing fields as arguments. The syscall provider uses this to
> > skip the __syscall_nr field, and to stop argument processing when the
> > first dynamic string field is found.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_prov_dtrace.c | 2 +-
> > libdtrace/dt_prov_fbt.c | 2 +-
> > libdtrace/dt_prov_sdt.c | 2 +-
> > libdtrace/dt_prov_syscall.c | 42 +++++++++++++++++++++----
> > libdtrace/dt_provider_tp.c | 63 +++++++++++++++++++++++++------------
> > libdtrace/dt_provider_tp.h | 11 +++++--
> > 6 files changed, 90 insertions(+), 32 deletions(-)
> >
> > diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> > index 9ef001a3..a0bf34c3 100644
> > --- a/libdtrace/dt_prov_dtrace.c
> > +++ b/libdtrace/dt_prov_dtrace.c
> > @@ -255,7 +255,7 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
> > if (f == NULL)
> > return -ENOENT;
> > - rc = dt_tp_probe_info(dtp, f, 0, prp, NULL, NULL);
> > + rc = dt_tp_probe_info(dtp, f, NULL, prp, NULL, NULL);
> > fclose(f);
> > if (rc < 0)
> > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> > index ed1bd93d..59e4583b 100644
> > --- a/libdtrace/dt_prov_fbt.c
> > +++ b/libdtrace/dt_prov_fbt.c
> > @@ -526,7 +526,7 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
> > goto out;
> > /* read event id from format file */
> > - rc = dt_tp_probe_info(dtp, f, 0, prp, NULL, NULL);
> > + rc = dt_tp_probe_info(dtp, f, NULL, prp, NULL, NULL);
> > fclose(f);
> > if (rc < 0)
> > diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> > index f9de2cf8..b1343481 100644
> > --- a/libdtrace/dt_prov_sdt.c
> > +++ b/libdtrace/dt_prov_sdt.c
> > @@ -202,7 +202,7 @@ static int probe_info_tracefs(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> > if (!f)
> > return -ENOENT;
> > - rc = dt_tp_probe_info(dtp, f, 0, prp, argcp, argvp);
> > + rc = dt_tp_probe_info(dtp, f, NULL, prp, argcp, argvp);
> > fclose(f);
> > return rc;
> > diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> > index 4f1e206a..e80c7bcb 100644
> > --- a/libdtrace/dt_prov_syscall.c
> > +++ b/libdtrace/dt_prov_syscall.c
> > @@ -40,11 +40,6 @@ static const char modname[] = "vmlinux";
> > #define SYSCALLSFS EVENTSFS "syscalls/"
> > -/*
> > - * We need to skip over an extra field: __syscall_nr.
> > - */
> > -#define SKIP_EXTRA_FIELDS 1
> > -
> > static const dtrace_pattr_t pattr = {
> > { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON },
> > { DTRACE_STABILITY_PRIVATE, DTRACE_STABILITY_PRIVATE, DTRACE_CLASS_UNKNOWN },
> > @@ -186,6 +181,41 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> > return 0;
> > }
> > +static int valid_arg(int fldc, const char *desc)
> > +{
> > + char *p, *q;
> > + int rc = 1;
> > + size_t l;
> > +
> > + /* Skip the first non-cmmon field (__syscall_nr). */
> > + if (fldc < 1)
> > + return 0;
> > +
> > + /*
> > + * A non-common field with a __data_loc tag and __<name>_val name
> > + * indicates the beginning of the dynamic data area. The previously
> > + * processed field was the last argument.
> > + */
> > + p = strdup(desc);
> > + if (sscanf(p, "__data_loc %[^;]", p) <= 0)
> > + goto ok;
> > +
> > + l = strlen(p);
> > + if (l < 4 || strcmp(&(p[l - 4]), "_val") != 0)
> > + goto ok;
> > +
> > + q = strrchr(p, ' ');
> > + if (q == NULL || strncmp(q, " __", 3) != 0)
> > + goto ok;
> > +
> > + /* Done with processing fields. */
> > + rc = -1;
> > +
> > +ok:
> > + free(p);
> > + return rc;
> > +}
> > +
> > static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> > int *argcp, dt_argdesc_t **argvp)
> > {
> > @@ -209,7 +239,7 @@ static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> > if (!f)
> > return -ENOENT;
> > - rc = dt_tp_probe_info(dtp, f, SKIP_EXTRA_FIELDS, prp, argcp, argvp);
> > + rc = dt_tp_probe_info(dtp, f, valid_arg, prp, argcp, argvp);
> > fclose(f);
> > return rc;
> > diff --git a/libdtrace/dt_provider_tp.c b/libdtrace/dt_provider_tp.c
> > index 1faf5cd4..6e0d908a 100644
> > --- a/libdtrace/dt_provider_tp.c
> > +++ b/libdtrace/dt_provider_tp.c
> > @@ -123,17 +123,16 @@ dt_tp_attach_raw(dtrace_hdl_t *dtp, tp_probe_t *tpp, const char *name,
> > * array dimension specifiers (and those are part of the type).
> > *
> > * All events include a number of common fields that we are not interested
> > - * in and that need to be skipped. Callers of this function can specify an
> > - * additional number of fields to skip (using the 'skip' parameter) before
> > - * we get to the actual arguments.
> > + * in and that need to be skipped. Callers of this function can specify a
> > + * callback function (valid_arg) to validate a non-common field.
> > */
> > int
> > -dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > - int *argcp, dt_argdesc_t **argvp)
> > +dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, dt_valid_arg_f *valid_arg,
> > + tp_probe_t *tpp, int *argcp, dt_argdesc_t **argvp)
> > {
> > char *buf = NULL;
> > size_t bufsz;
> > - int argc, common = 1;
> > + int idx = 0, argc = 0, skip = 0, common = 1;
> > dt_argdesc_t *argv = NULL;
> > tpp->id = 0;
> > @@ -141,10 +140,8 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > /*
> > * Pass 1:
> > * Determine the event id and the number of arguments.
> > - * Skip over how ever many arguments the caller asks us to skip.
> > - * We will skip initial "common fields" as well.
> > + * We will skip initial "common fields".
> > */
> > - argc = -skip;
> > while (getline(&buf, &bufsz, f) >= 0) {
> > char *p = buf;
> > @@ -205,7 +202,31 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > common = 0;
> > }
> > - /* We found a non-common "field:" description. */
> > + /*
> > + * We found a non-common "field:" description.
> > + *
> > + * If the caller provided a validation hook, call it with the
> > + * current non-common field counter and the description text.
> > + * The hook returns
> > + * -1 if the field is not a valid argument, and no additional
> > + * arguments can follow
> > + * 0 if the field should be skipped
> > + * 1 if the field is a valid argument
> > + */
> > + idx++;
> > + if (valid_arg != NULL) {
> > + int rc = valid_arg(idx - 1, p);
> > +
> > + if (rc == -1)
> > + break;
> > + if (rc == 0) {
> > + skip++;
> > + continue;
> > + }
> > + }
> > +
> > + sscanf(p, "__data_loc %[^;]", p);
> > +
> > argc++;
> > }
> > free(buf);
> > @@ -232,7 +253,7 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > * Fill in the actual argument datatype strings.
> > */
> > rewind(f);
> > - argc = -skip;
> > + idx = -skip;
> > while (getline(&buf, &bufsz, f) >= 0) {
> > char *p;
> > size_t l;
> > @@ -249,7 +270,7 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > continue;
> > /* We found a field: description - see if we should skip it. */
> > - if (argc < 0)
> > + if (idx < 0)
> > goto skip;
> > sscanf(p, "__data_loc %[^;]", p);
> > @@ -320,13 +341,15 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
> > }
> > }
> > - argv[argc].mapping = argc;
> > - argv[argc].flags = 0;
> > - argv[argc].native = strdup(strp);
> > - argv[argc].xlate = NULL;
> > + argv[idx].mapping = idx;
> > + argv[idx].flags = 0;
> > + argv[idx].native = strdup(strp);
> > + argv[idx].xlate = NULL;
> > skip:
> > - argc++;
> > + idx++;
> > + if (idx == argc)
> > + break;
> > }
> > done:
> > @@ -413,12 +436,12 @@ dt_tp_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const char *prv,
> > * the argument types for a given probe.
> > */
> > int
> > -dt_tp_probe_info(dtrace_hdl_t *dtp, FILE *f, int skip, const dt_probe_t *prp,
> > - int *argcp, dt_argdesc_t **argvp)
> > +dt_tp_probe_info(dtrace_hdl_t *dtp, FILE *f, dt_valid_arg_f *valid_arg,
> > + const dt_probe_t *prp, int *argcp, dt_argdesc_t **argvp)
> > {
> > tp_probe_t *tpp = prp->prv_data;
> > - return dt_tp_event_info(dtp, f, skip, tpp, argcp, argvp);
> > + return dt_tp_event_info(dtp, f, valid_arg, tpp, argcp, argvp);
> > }
> > /*
> > diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h
> > index 406609d1..bf73d96a 100644
> > --- a/libdtrace/dt_provider_tp.h
> > +++ b/libdtrace/dt_provider_tp.h
> > @@ -33,8 +33,12 @@ extern int dt_tp_attach(dtrace_hdl_t *dtp, tp_probe_t *tpp, int bpf_fd);
> > extern int dt_tp_attach_raw(dtrace_hdl_t *dtp, tp_probe_t *tpp,
> > const char *name, int bpf_fd);
> > extern int dt_tp_has_info(const tp_probe_t *tpp);
> > -extern int dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip,
> > - tp_probe_t *tpp, int *argcp, dt_argdesc_t **argvp);
> > +
> > +typedef int dt_valid_arg_f(int fldc, const char *desc);
> > +extern int dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f,
> > + dt_valid_arg_f *valid_arg, tp_probe_t *tpp,
> > + int *argcp, dt_argdesc_t **argvp);
> > +
> > extern void dt_tp_detach(dtrace_hdl_t *dtp, tp_probe_t *tpp);
> > extern void dt_tp_destroy(dtrace_hdl_t *dtp, tp_probe_t *tpp);
> > extern uint32_t dt_tp_get_id(const tp_probe_t *prp);
> > @@ -44,7 +48,8 @@ extern struct dt_probe *dt_tp_probe_insert(dtrace_hdl_t *dtp,
> > dt_provider_t *prov,
> > const char *prv, const char *mod,
> > const char *fun, const char *prb);
> > -extern int dt_tp_probe_info(dtrace_hdl_t *dtp, FILE *f, int skip,
> > +extern int dt_tp_probe_info(dtrace_hdl_t *dtp, FILE *f,
> > + dt_valid_arg_f *valid_arg,
> > const struct dt_probe *prp, int *argcp,
> > dt_argdesc_t **argvp);
> > extern int dt_tp_probe_has_info(const struct dt_probe *prp);
More information about the DTrace-devel
mailing list