BPF List
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox