[DTrace-devel] [PATCH v4] bpf, fbt: do not try to get a return value for void function return pronbes
Eugene Loh
eugene.loh at oracle.com
Fri Aug 2 18:59:22 UTC 2024
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
Subject line: s/pronbes/probes/.
Gratuitous '`' in the comment line in both .x files?
Just to talk myself through this, with kprobes, no typed args and so
even args[0] will be rejected. We have other tests to check that this
is working. So skipping this test in the case of kprobes is okay.
One last concern is if something goes wrong and we mistakenly skip the
test. That is, no typed args, even though "there should be." I'm fine
considering that to be beyond the scope of this patch.
On 8/2/24 14:34, Kris Van Hees via DTrace-devel wrote:
> We were trying to access a return value for void functions, causing BPF
> verifier failures. We can determine the return type when getting the
> probe info, and set the return probe argc value accordingly. This then
> drives the code generation in the trampoline.
>
> Reported-by: Alan Maguire <alan.maguire at oracle.com>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_btf.c | 16 +++++++++++++
> libdtrace/dt_btf.h | 2 ++
> libdtrace/dt_prov_fbt.c | 21 +++++++++-------
> .../fbtprovider/err.D_ARGS_IDX.void-void.d | 24 +++++++++++++++++++
> .../fbtprovider/err.D_ARGS_IDX.void-void.r | 2 ++
> .../fbtprovider/err.D_ARGS_IDX.void-void.x | 11 +++++++++
> .../fbtprovider/err.D_ARGS_IDX.void.d | 24 +++++++++++++++++++
> .../fbtprovider/err.D_ARGS_IDX.void.r | 2 ++
> .../fbtprovider/err.D_ARGS_IDX.void.x | 11 +++++++++
> 9 files changed, 104 insertions(+), 9 deletions(-)
> create mode 100644 test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.d
> create mode 100644 test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.r
> create mode 100755 test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.x
> create mode 100644 test/unittest/fbtprovider/err.D_ARGS_IDX.void.d
> create mode 100644 test/unittest/fbtprovider/err.D_ARGS_IDX.void.r
> create mode 100755 test/unittest/fbtprovider/err.D_ARGS_IDX.void.x
>
> diff --git a/libdtrace/dt_btf.c b/libdtrace/dt_btf.c
> index 77366df6..e905580f 100644
> --- a/libdtrace/dt_btf.c
> +++ b/libdtrace/dt_btf.c
> @@ -980,3 +980,19 @@ dt_btf_func_argc(dtrace_hdl_t *dtp, const dt_btf_t *btf, uint32_t id)
>
> return -1;
> }
> +
> +int
> +dt_btf_func_is_void(dtrace_hdl_t *dtp, const dt_btf_t *btf, uint32_t id)
> +{
> + btf_type_t *type = dt_btf_type_by_id(dtp, btf, id);
> +
> + /* For functions, move on to the function prototype. */
> + if (BTF_INFO_KIND(type->info) == BTF_KIND_FUNC)
> + type = dt_btf_type_by_id(dtp, btf, type->type);
> +
> + if (BTF_INFO_KIND(type->info) == BTF_KIND_FUNC_PROTO &&
> + type->type == 0)
> + return 1;
> +
> + return 0;
> +}
> diff --git a/libdtrace/dt_btf.h b/libdtrace/dt_btf.h
> index a7cee464..e05b30d0 100644
> --- a/libdtrace/dt_btf.h
> +++ b/libdtrace/dt_btf.h
> @@ -25,6 +25,8 @@ extern int32_t dt_btf_lookup_name_kind(dtrace_hdl_t *, dt_btf_t *,
> const char *, uint32_t);
> extern int dt_btf_func_argc(dtrace_hdl_t *dtp, const dt_btf_t *btf,
> uint32_t id);
> +extern int dt_btf_func_is_void(dtrace_hdl_t *dtp, const dt_btf_t *btf,
> + uint32_t id);
> extern int dt_btf_get_module_ids(dtrace_hdl_t *);
> extern int dt_btf_module_fd(const dt_module_t *);
>
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 461d5e35..80c3db20 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -199,13 +199,15 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
> emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(0), BPF_REG_0));
>
> /*
> + * Only try to retrieve a return value if we know we can.
> + *
> * The return value is provided by the fexit probe as an
> * argument slot past the last function argument. We can get
> * the number of function arguments using the BTF id that has
> * been stored as the tracepoint event id.
> */
> dmp = dt_module_lookup_by_name(dtp, prp->desc->mod);
> - if (dmp != NULL) {
> + if (dmp && prp->argc == 2) {
> int32_t btf_id = dt_tp_get_event_id(prp);
> int i = dt_btf_func_argc(dtp, dmp->dm_btf, btf_id);
>
> @@ -240,7 +242,9 @@ static int fprobe_probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> dt_tp_set_event_id(prp, btf_id);
>
> if (strcmp(desc->prb, "return") == 0) {
> - argc = 2;
> + /* Void function return probes only provide 1 argument. */
> + argc = dt_btf_func_is_void(dtp, dmp->dm_btf, btf_id) ? 1 : 2;
> +
> argv = dt_calloc(dtp, argc, sizeof(dt_argdesc_t));
> if (argv == NULL)
> return dt_set_errno(dtp, EDT_NOMEM);
> @@ -250,13 +254,12 @@ static int fprobe_probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> argv[0].native = strdup("uint64_t");
> argv[0].xlate = NULL;
>
> - /*
> - * The return type is a generic uint64_t. For void functions
> - * the value of arg1 and argv[1] is undefined.
> - */
> - argv[1].mapping = 1;
> - argv[1].native = strdup("uint64_t");
> - argv[1].xlate = NULL;
> + if (argc == 2) {
> + /* The return type is a generic uint64_t. */
> + argv[1].mapping = 1;
> + argv[1].native = strdup("uint64_t");
> + argv[1].xlate = NULL;
> + }
>
> goto done;
> }
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.d b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.d
> new file mode 100644
> index 00000000..4bdf36cb
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.d
> @@ -0,0 +1,24 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2024, 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.
> + */
> +
> +#pragma D option quiet
> +
> +/*
> + * This should fail compilation due to accessing args[1] (not args[0]) for a
> + * void function without any arguuments.
> + */
> +fbt:vmlinux:oops_enter:return
> +{
> + trace(args[0]);
> + trace(args[1]);
> + exit(0);
> +}
> +
> +ERROR
> +{
> + exit(1);
> +}
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.r b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.r
> new file mode 100644
> index 00000000..8bd3dd7e
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.d: [D_ARGS_IDX] line 17: index 1 is out of range for fbt:vmlinux:oops_enter:return args[ ]
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.x b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.x
> new file mode 100755
> index 00000000..fb503858
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void-void.x
> @@ -0,0 +1,11 @@
> +#!/bin/bash
> +
> +# Skip test if FBT probes do not provide argument datatype info.`
> +types=`$dtrace -lvn fbt::oops_enter:return | awk '/^[ ]*args\[/ { $1 = ""; print }' | sort -u`
> +
> +if [[ -z "$types" ]]; then
> + echo "FBT probes without args[] type info"
> + exit 2
> +fi
> +
> +exit 0
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void.d b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.d
> new file mode 100644
> index 00000000..516d733a
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.d
> @@ -0,0 +1,24 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2024, 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.
> + */
> +
> +#pragma D option quiet
> +
> +/*
> + * This should fail compilation due to accessing args[1] (not args[0]) for a
> + * void function with arguments.
> + */
> +fbt:vmlinux:exit_creds:return
> +{
> + trace(args[0]);
> + trace(args[1]);
> + exit(0);
> +}
> +
> +ERROR
> +{
> + exit(1);
> +}
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void.r b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.r
> new file mode 100644
> index 00000000..720d2f97
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/fbtprovider/err.D_ARGS_IDX.void.d: [D_ARGS_IDX] line 17: index 1 is out of range for fbt:vmlinux:exit_creds:return args[ ]
> diff --git a/test/unittest/fbtprovider/err.D_ARGS_IDX.void.x b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.x
> new file mode 100755
> index 00000000..79677007
> --- /dev/null
> +++ b/test/unittest/fbtprovider/err.D_ARGS_IDX.void.x
> @@ -0,0 +1,11 @@
> +#!/bin/bash
> +
> +# Skip test if FBT probes do not provide argument datatype info.`
> +types=`$dtrace -lvn fbt::exit_creds:return | awk '/^[ ]*args\[/ { $1 = ""; print }' | sort -u`
> +
> +if [[ -z "$types" ]]; then
> + echo "FBT probes without args[] type info"
> + exit 2
> +fi
> +
> +exit 0
More information about the DTrace-devel
mailing list