All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Jiawei Zhao <Phoenix500526@163.com>, andrii@kernel.org
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH v3] libbpf: fix USDT SIB argument handling causing unrecognized register error
Date: Tue, 29 Jul 2025 15:54:31 -0700	[thread overview]
Message-ID: <23caf44c-48e7-41b2-bc9a-e286f93bd96f@linux.dev> (raw)
In-Reply-To: <20250729161722.35462-1-Phoenix500526@163.com>



On 7/29/25 9:17 AM, Jiawei Zhao wrote:
> On x86-64, USDT arguments can be specified using Scale-Index-Base (SIB)
> addressing, e.g. "1@-96(%rbp,%rax,8)". The current USDT implementation
> in libbpf cannot parse this format, causing `bpf_program__attach_usdt()`
> to fail with -ENOENT (unrecognized register).
>
> This patch fixes this by implementing the necessary changes:
> - add correct handling for SIB-addressed arguments in `bpf_usdt_arg`.
> - add adaptive support to `__bpf_usdt_arg_type` and
> `__bpf_usdt_arg_spec` to represent SIB addressing parameters.
>
> Change since v1(https://lore.kernel.org/lkml/20250729125244.28364-1-Phoenix500526@163.com/):
> - refactor the code to make it more readable
> - modify the commit message to explain why and how
>
> Change since v2:
> - fix the `scale` uninitialized error
>
> Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
> ---
>   tools/lib/bpf/usdt.bpf.h | 33 ++++++++++++++++++++++++++++++++-
>   tools/lib/bpf/usdt.c     | 26 +++++++++++++++++++++++---
>   2 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h
> index 2a7865c8e3fe..246513088c3a 100644
> --- a/tools/lib/bpf/usdt.bpf.h
> +++ b/tools/lib/bpf/usdt.bpf.h
> @@ -34,6 +34,7 @@ enum __bpf_usdt_arg_type {
>   	BPF_USDT_ARG_CONST,
>   	BPF_USDT_ARG_REG,
>   	BPF_USDT_ARG_REG_DEREF,
> +	BPF_USDT_ARG_SIB,
>   };
>   
>   struct __bpf_usdt_arg_spec {
> @@ -43,6 +44,10 @@ struct __bpf_usdt_arg_spec {
>   	enum __bpf_usdt_arg_type arg_type;
>   	/* offset of referenced register within struct pt_regs */
>   	short reg_off;
> +	/* offset of index register in pt_regs, only used in SIB mode */
> +	short idx_reg_off;
> +	/* scale factor for index register, only used in SIB mode */
> +	short scale;
>   	/* whether arg should be interpreted as signed value */
>   	bool arg_signed;
>   	/* number of bits that need to be cleared and, optionally,
> @@ -149,7 +154,7 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
>   {
>   	struct __bpf_usdt_spec *spec;
>   	struct __bpf_usdt_arg_spec *arg_spec;
> -	unsigned long val;
> +	unsigned long val, idx;
>   	int err, spec_id;
>   
>   	*res = 0;
> @@ -202,6 +207,32 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
>   			return err;
>   #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>   		val >>= arg_spec->arg_bitshift;
> +#endif
> +		break;
> +	case BPF_USDT_ARG_SIB:
> +		/* Arg is in memory addressed by SIB (Scale-Index-Base) mode
> +		 * (e.g., "-1@-96(%rbp,%rax,8)" in USDT arg spec). Register
> +		 * is identified like with BPF_USDT_ARG_SIB case, the offset
> +		 * is in arg_spec->val_off, the scale factor is in arg_spec->scale.
> +		 * Firstly, we fetch the base register contents and the index
> +		 * register contents from pt_regs. Secondly, we multiply the
> +		 * index register contents by the scale factor, then add the
> +		 * base address and the offset to get the final address. Finally,
> +		 * we do another user-space probe read to fetch argument value
> +		 * itself.
> +		 */
> +		err = bpf_probe_read_kernel(&val, sizeof(val), (void *)ctx + arg_spec->reg_off);
> +		if (err)
> +			return err;
> +		err = bpf_probe_read_kernel(&idx, sizeof(idx), (void *)ctx + arg_spec->idx_reg_off);
> +		if (err)
> +			return err;
> +		err = bpf_probe_read_user(&val, sizeof(val),
> +				(void *)val + idx * arg_spec->scale + arg_spec->val_off);
> +		if (err)
> +			return err;
> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +		val >>= arg_spec->arg_bitshift;
>   #endif
>   		break;
>   	default:

If possible, could you add some tests which actually trigger a usdt pattern
like "-1@-96(%rbp,%rax,8)"?

There are much more usdt patterns e.g. in
   https://lore.kernel.org/bpf/b3ce39f0-c52b-4787-980c-973bd4228349@linux.dev/
===
with -O2 and with gcc14 on x86:

    stapsdt              0x00000087       NT_STAPSDT (SystemTap probe descriptors)
      Provider: test
      Name: usdt12
      Location: 0x000000000000258f, Base: 0x0000000000000000, Semaphore: 0x0000000000000006
      Arguments: -4@$2 -4@$3 -8@$42 -8@$44 -4@$5 -8@$6 8@%rdx 8@%rsi -4@$-9 -2@%cx -2@nums(%rax,%rax) -1@t1+4(%rip)
    ...
===

But we didn't add those '-2@nums(%rax,%rax)' '-1@t1+4(%rip)' as
they are very rare.

> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 4e4a52742b01..260211e896d5 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -200,6 +200,7 @@ enum usdt_arg_type {
>   	USDT_ARG_CONST,
>   	USDT_ARG_REG,
>   	USDT_ARG_REG_DEREF,
> +	USDT_ARG_SIB,
>   };
>   
>   /* should match exactly struct __bpf_usdt_arg_spec from usdt.bpf.h */
> @@ -207,6 +208,8 @@ struct usdt_arg_spec {
>   	__u64 val_off;
>   	enum usdt_arg_type arg_type;
>   	short reg_off;
> +	short idx_reg_off;
> +	short scale;
>   	bool arg_signed;
>   	char arg_bitshift;
>   };
> @@ -1283,11 +1286,28 @@ static int calc_pt_regs_off(const char *reg_name)
>   
>   static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg, int *arg_sz)
>   {
> -	char reg_name[16];
> -	int len, reg_off;
> +	char reg_name[16], idx_reg_off, idx_reg_name[16];
> +	int len, reg_off, scale;
>   	long off;
>   
> -	if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n", arg_sz, &off, reg_name, &len) == 3) {
> +	if (sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^,] , %d ) %n",
> +				arg_sz, &off, reg_name, idx_reg_name, &scale, &len) == 5) {
> +		/* Scale Index Base case, e.g., 1@-96(%rbp,%rax,8)*/
> +		arg->arg_type = USDT_ARG_SIB;
> +		arg->val_off = off;
> +		arg->scale = scale;
> +
> +		reg_off = calc_pt_regs_off(reg_name);
> +		if (reg_off < 0)
> +			return reg_off;
> +		arg->reg_off = reg_off;
> +
> +		idx_reg_off = calc_pt_regs_off(idx_reg_name);
> +		if (idx_reg_off < 0)
> +			return idx_reg_off;
> +		arg->idx_reg_off = idx_reg_off;
> +	} else if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n",
> +				arg_sz, &off, reg_name, &len) == 3) {
>   		/* Memory dereference case, e.g., -4@-20(%rbp) */
>   		arg->arg_type = USDT_ARG_REG_DEREF;
>   		arg->val_off = off;


      reply	other threads:[~2025-07-29 22:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-29 16:17 [PATCH v3] libbpf: fix USDT SIB argument handling causing unrecognized register error Jiawei Zhao
2025-07-29 22:54 ` Yonghong Song [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23caf44c-48e7-41b2-bc9a-e286f93bd96f@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=Phoenix500526@163.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.