Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221202153816.1180450-1-timo.hunziker@gmx.ch>
In-Reply-To: <20221202153816.1180450-1-timo.hunziker@gmx.ch>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Fri, 2 Dec 2022 16:39:11 -0800
Message-ID: <CAEf4BzbzgDfkvEtcQbCE1wZ4YHOTTWwr3rfX3D63RfD08qGj-Q@mail.gmail.com>
Subject: Re: [PATCH bpf-next] libbpf: parse usdt args without offset on x86
 (e.g. 8@(%rsp))
To:     Timo Hunziker <timo.hunziker@gmx.ch>
Cc:     bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
        andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
        yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
        sdf@google.com, haoluo@google.com, jolsa@google.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Dec 2, 2022 at 7:39 AM Timo Hunziker <timo.hunziker@gmx.ch> wrote:
>
> Parse USDT arguments like "8@(%rsp)" on x86. These are emmited by
> systemtap. The syntax is a mixture between the "memory dereference
> case" and the "register read case" as the offset is zero but the
> register is wrapped in parentheses. We treat them the same as the
> the "register read case".

wait, why? I'd assume this is equivalent to 8@0(%rsp) and that's
actually the USDT_ARG_REG_DEREF case? I.e., we read the value of %rsp
and then use that as a pointer to a memory.

>
> I've tested that this fixes the "unrecognized arg #N spec: 8@(%rsp).."
> error I've run into when attaching to a probe with such an argument.
> Attaching and reading the arguments works.
>
> Something similar might be needed for the other supported
> architectures.
>
> ref: https://github.com/libbpf/libbpf/issues/559
>
> Signed-off-by: Timo Hunziker <timo.hunziker@gmx.ch>
> ---
>  tools/lib/bpf/usdt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index b8daae265f99..5e7ec7ad8ad7 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -1233,6 +1233,14 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
>                 if (reg_off < 0)
>                         return reg_off;
>                 arg->reg_off = reg_off;
> +       } else if (sscanf(arg_str, " %d @ ( %%%15[^)] ) %n", &arg_sz, reg_name, &len) == 2) {
> +               /* Register read case with parentheses, e.g., 8@(%rsp) */
> +               arg->arg_type = USDT_ARG_REG;

while you implemented it as "return %rsp value", it's a very different case


> +               arg->val_off = 0;
> +               reg_off = calc_pt_regs_off(reg_name);
> +               if (reg_off < 0)
> +                       return reg_off;
> +               arg->reg_off = reg_off;
>         } else if (sscanf(arg_str, " %d @ %%%15s %n", &arg_sz, reg_name, &len) == 2) {
>                 /* Register read case, e.g., -4@%eax */
>                 arg->arg_type = USDT_ARG_REG;
> --
> 2.36.2
>