Re: [PATCH v3] libbpf: fix USDT SIB argument handling causing unrecognized register error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxx/):
- 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@xxxxxxx>
---
  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@xxxxxxxxx/
===
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;





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux