[PATCH bpf-next v8 0/2] libbpf: fix USDT SIB argument handling causing unrecognized register error

List overview All Threads
Download

newer

older

Re: [PATCH v14 net-next 00/14]...

[PATCH v2] selftests: arm64: Fix...

Jiawei Zhao

7 Aug 2025 7 Aug '25

2:34 a.m.

When using GCC on x86-64 to compile an usdt prog with -O1 or higher optimization, the compiler will generate SIB addressing mode for global array and PC-relative addressing mode for global variable, e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".

The current USDT implementation in libbpf cannot parse these two formats, causing `bpf_program__attach_usdt()` to fail with -ENOENT (unrecognized register).

This patch series adds support for SIB addressing mode in USDT probes. The main changes include: - add correct handling logic for SIB-addressed arguments in `parse_usdt_arg`. - add an usdt_o2 test case to cover SIB addressing mode.

Testing shows that the SIB probe correctly generates 8@(%rcx,%rax,8) argument spec and passes all validation checks.

The modification history of this patch series: Change since v1: - 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

Change since v3: - force -O2 optimization for usdt.test.o to generate SIB addressing usdt and pass all test cases.

Change since v4: - split the patch into two parts, one for the fix and the other for the test

Change since v5: - Only enable optimization for x86 architecture to generate SIB addressing usdt argument spec.

Change since v6: - Add an usdt_o2 test case to cover SIB addressing mode. - Reinstate the usdt.c test case.

Change since v7: - Add a bpf-next tag to the patch series. - update the commit message of the second commit

Jiawei Zhao (2): libbpf: fix USDT SIB argument handling causing unrecognized register error selftests/bpf: Add an usdt_o2 test case in selftests to cover SIB handling logic

tools/lib/bpf/usdt.bpf.h | 33 ++++++++- tools/lib/bpf/usdt.c | 43 +++++++++-- tools/testing/selftests/bpf/Makefile | 8 +++ .../selftests/bpf/prog_tests/usdt_o2.c | 71 +++++++++++++++++++ .../selftests/bpf/progs/test_usdt_o2.c | 37 ++++++++++ 5 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/usdt_o2.c create mode 100644 tools/testing/selftests/bpf/progs/test_usdt_o2.c

-- 2.43.0

Show replies by date

Jiawei Zhao

7 Aug 7 Aug

2:34 a.m.

New subject: [PATCH bpf-next v8 1/2] libbpf: fix USDT SIB argument handling causing unrecognized register error

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.

Signed-off-by: Jiawei Zhao phoenix500526@163.com --- tools/lib/bpf/usdt.bpf.h | 33 +++++++++++++++++++++++++++++- tools/lib/bpf/usdt.c | 43 ++++++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 7 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: diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c index 4e4a52742b01..1f8b9e1c9819 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,39 @@ 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; - long off; + char reg_name[16] = {0}, idx_reg_name[16] = {0}; + int len, reg_off, idx_reg_off, scale = 1; + long off = 0; + + if (sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^,] , %d ) %n", + arg_sz, &off, reg_name, idx_reg_name, &scale, &len) == 5 || + sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^,] , %d ) %n", + arg_sz, reg_name, idx_reg_name, &scale, &len) == 4 || + sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^)] ) %n", + arg_sz, &off, reg_name, idx_reg_name, &len) == 4 || + sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^)] ) %n", + arg_sz, reg_name, idx_reg_name, &len) == 3 + ) { + /* Scale Index Base case, e.g., 1@-96(%rbp,%rax,8) + * 1@(%rbp,%rax,8) + * 1@-96(%rbp,%rax) + * 1@(%rbp,%rax) + */ + 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;

- if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n", arg_sz, &off, reg_name, &len) == 3) { + 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; @@ -1298,7 +1329,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } else if (sscanf(arg_str, " %d @ ( %%%15[^)] ) %n", arg_sz, reg_name, &len) == 2) { /* Memory dereference case without offset, e.g., 8@(%rsp) */ arg->arg_type = USDT_ARG_REG_DEREF; - arg->val_off = 0; + arg->val_off = off; reg_off = calc_pt_regs_off(reg_name); if (reg_off < 0) return reg_off; @@ -1306,7 +1337,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } 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; - arg->val_off = 0; + arg->val_off = off;

reg_off = calc_pt_regs_off(reg_name); if (reg_off < 0)

-- 2.43.0

Andrii Nakryiko

13 Aug 13 Aug

11:52 p.m.

New subject: [PATCH bpf-next v8 1/2] libbpf: fix USDT SIB argument handling causing unrecognized register error

On Wed, Aug 6, 2025 at 7:35 PM Jiawei Zhao phoenix500526@163.com 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.

Signed-off-by: Jiawei Zhao phoenix500526@163.com

tools/lib/bpf/usdt.bpf.h | 33 +++++++++++++++++++++++++++++- tools/lib/bpf/usdt.c | 43 ++++++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 7 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;

I'd really prefer not to increase the size of __bpf_usdt_arg_spec and not change its layout for all existing BPF_USDT_ARG_* modes just to not have to worry about any backwards/forward compatibility issues.

Scale can be 1, 2,4, 8, is that right? Instead of using 2 bytes for it, we should be able to use just 2 bits to represent bit shift (0, 1, 2, 3 should be enough).

We can carve out at least 3 bytes by making arg_type field into packed single-byte enum (we'd need to be careful with big endian).

Then we can add idx_reg_off:12 and idx_scale_shift:4 somewhere between arg_type and reg_off, taking 2 bytes in total.

We'll still be left with one byte to spare for the future (and there are tricks we can do with arg_signed and arg_bitshift, but I'd not touch them yet).

WDYT?

pw-bot: cr

...

    /* 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);

it might be just how gmail renders it, but please make sure that wrapped argument is aligned with first argument on the previous line

...

          if (err)
                  return err;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
          val >>= arg_spec->arg_bitshift;
#endif break; default: diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c index 4e4a52742b01..1f8b9e1c9819 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,39 @@ 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;
  long off;
  char reg_name[16] = {0}, idx_reg_name[16] = {0};
  int len, reg_off, idx_reg_off, scale = 1;
  long off = 0;
  if (sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^,] , %d ) %n",
                          arg_sz, &off, reg_name, idx_reg_name, &scale, &len) == 5 ||

see comment above about aligning wrapped argument list

...

          sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^,] , %d ) %n",

                          arg_sz, reg_name, idx_reg_name, &scale, &len) == 4 ||

          sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^)] ) %n",

                          arg_sz, &off, reg_name, idx_reg_name, &len) == 4 ||

          sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^)] ) %n",

                          arg_sz, reg_name, idx_reg_name, &len) == 3

```
          ) {
```

          /* Scale Index Base case, e.g., 1@-96(%rbp,%rax,8)

```
           * 1@(%rbp,%rax,8)
```
```
           * 1@-96(%rbp,%rax)
```
```
           * 1@(%rbp,%rax)
```

nit: let's list all variants at the same indentation level (and let's use the more standard multi-level comment format)

/* * Scale-Index-Base case: * - 1@-96(%rbp,%rax,8) * - 1@(%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;
```

  if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n", arg_sz, &off, reg_name, &len) == 3) {

          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;

@@ -1298,7 +1329,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } else if (sscanf(arg_str, " %d @ ( %%%15[^)] ) %n", arg_sz, reg_name, &len) == 2) { /* Memory dereference case without offset, e.g., 8@(%rsp) */ arg->arg_type = USDT_ARG_REG_DEREF;

```
          arg->val_off = 0;
```

          arg->val_off = off;
          reg_off = calc_pt_regs_off(reg_name);
          if (reg_off < 0)
                  return reg_off;

@@ -1306,7 +1337,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } 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;

```
          arg->val_off = 0;
```

```
          arg->val_off = off;
```

why this change? it makes it seem like val_off might not be zero, for no good reason...

...

            reg_off = calc_pt_regs_off(reg_name);
            if (reg_off < 0)

-- 2.43.0

赵佳炜

14 Aug 14 Aug

6:46 a.m.

New subject: [PATCH bpf-next v8 1/2] libbpf: fix USDT SIB argument handling causing unrecognized register error

...

I'd really prefer not to increase the size of __bpf_usdt_arg_spec and not change its layout for all existing BPF_USDT_ARG_* modes just to not have to worry about any backwards/forward compatibility issues.

Scale can be 1, 2,4, 8, is that right? Instead of using 2 bytes for it, we should be able to use just 2 bits to represent bit shift (0, 1, 2, 3 should be enough).

We can carve out at least 3 bytes by making arg_type field into packed single-byte enum (we'd need to be careful with big endian).

Then we can add idx_reg_off:12 and idx_scale_shift:4 somewhere between arg_type and reg_off, taking 2 bytes in total.

We'll still be left with one byte to spare for the future (and there are tricks we can do with arg_signed and arg_bitshift, but I'd not touch them yet).

WDYT?

That's a good idea. I'll modify it in the new patch.

At 2025-08-14 07:52:47, "Andrii Nakryiko" andrii.nakryiko@gmail.com wrote:

...

On Wed, Aug 6, 2025 at 7:35 PM Jiawei Zhao phoenix500526@163.com 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.

Signed-off-by: Jiawei Zhao phoenix500526@163.com

tools/lib/bpf/usdt.bpf.h | 33 +++++++++++++++++++++++++++++- tools/lib/bpf/usdt.c | 43 ++++++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 7 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;
I'd really prefer not to increase the size of __bpf_usdt_arg_spec and not change its layout for all existing BPF_USDT_ARG_* modes just to not have to worry about any backwards/forward compatibility issues.

Scale can be 1, 2,4, 8, is that right? Instead of using 2 bytes for it, we should be able to use just 2 bits to represent bit shift (0, 1, 2, 3 should be enough).

We can carve out at least 3 bytes by making arg_type field into packed single-byte enum (we'd need to be careful with big endian).

Then we can add idx_reg_off:12 and idx_scale_shift:4 somewhere between arg_type and reg_off, taking 2 bytes in total.

We'll still be left with one byte to spare for the future (and there are tricks we can do with arg_signed and arg_bitshift, but I'd not touch them yet).

WDYT?

pw-bot: cr

...
    /* 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);
it might be just how gmail renders it, but please make sure that wrapped argument is aligned with first argument on the previous line

...
          if (err)
                  return err;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
          val >>= arg_spec->arg_bitshift;
#endif break; default: diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c index 4e4a52742b01..1f8b9e1c9819 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,39 @@ 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;
  long off;
  char reg_name[16] = {0}, idx_reg_name[16] = {0};
  int len, reg_off, idx_reg_off, scale = 1;
  long off = 0;
  if (sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^,] , %d ) %n",
                          arg_sz, &off, reg_name, idx_reg_name, &scale, &len) == 5 ||
see comment above about aligning wrapped argument list

...
          sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^,] , %d ) %n",
                          arg_sz, reg_name, idx_reg_name, &scale, &len) == 4 ||
          sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^)] ) %n",
                          arg_sz, &off, reg_name, idx_reg_name, &len) == 4 ||
          sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^)] ) %n",
                          arg_sz, reg_name, idx_reg_name, &len) == 3
          ) {
          /* Scale Index Base case, e.g., 1@-96(%rbp,%rax,8)
           * 1@(%rbp,%rax,8)
           * 1@-96(%rbp,%rax)
           * 1@(%rbp,%rax)
nit: let's list all variants at the same indentation level (and let's use the more standard multi-level comment format)

/*

Scale-Index-Base case:

1@-96(%rbp,%rax,8)

1@(%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;
  if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n", arg_sz, &off, reg_name, &len) == 3) {
          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;
@@ -1298,7 +1329,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } else if (sscanf(arg_str, " %d @ ( %%%15[^)] ) %n", arg_sz, reg_name, &len) == 2) { /* Memory dereference case without offset, e.g., 8@(%rsp) */ arg->arg_type = USDT_ARG_REG_DEREF;
          arg->val_off = 0;
          arg->val_off = off;
          reg_off = calc_pt_regs_off(reg_name);
          if (reg_off < 0)
                  return reg_off;
@@ -1306,7 +1337,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec } 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;
          arg->val_off = 0;
          arg->val_off = off;
why this change? it makes it seem like val_off might not be zero, for no good reason...

...
            reg_off = calc_pt_regs_off(reg_name);
            if (reg_off < 0)
-- 2.43.0

Jiawei Zhao

7 Aug 7 Aug

2:34 a.m.

New subject: [PATCH bpf-next v8 2/2] selftests/bpf: Add an usdt_o2 test case in selftests to cover SIB handling logic

In this patch: - add usdt_o2 test case to cover SIB addressing usdt argument spec handling logic

Signed-off-by: Jiawei Zhao phoenix500526@163.com --- tools/testing/selftests/bpf/Makefile | 8 +++ .../selftests/bpf/prog_tests/usdt_o2.c | 71 +++++++++++++++++++ .../selftests/bpf/progs/test_usdt_o2.c | 37 ++++++++++ 3 files changed, 116 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/usdt_o2.c create mode 100644 tools/testing/selftests/bpf/progs/test_usdt_o2.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 910d8d6402ef..68cf6a9cf05f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) TRUNNER_BPF_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_maps))

+# Use -O2 optimization to generate SIB addressing usdt argument spec +# Only apply on x86 architecture where SIB addressing is relevant +ifeq ($(ARCH), x86) +$(OUTPUT)/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/cpuv4/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/no_alu32/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +endif + # Define test_verifier test runner. # It is much simpler than test_maps/test_progs and sufficiently different from # them (e.g., test.h is using completely pattern), that it's worth just diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c new file mode 100644 index 000000000000..f04b756b3640 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Jiawei Zhao phoenix500526@163.com. */ +#include <test_progs.h> + +#define _SDT_HAS_SEMAPHORES 1 +#include "../sdt.h" +#include "test_usdt_o2.skel.h" + +int lets_test_this(int); + +#define test_value 0xFEDCBA9876543210ULL +#define SEC(name) __attribute__((section(name), used)) + + +static volatile __u64 array[1] = {test_value}; +unsigned short test_usdt1_semaphore SEC(".probes"); + +static __always_inline void trigger_func(void) +{ + /* Base address + offset + (index * scale) */ + if (test_usdt1_semaphore) { + for (volatile int i = 0; i <= 0; i++) + STAP_PROBE1(test, usdt1, array[i]); + } +} + +static void basic_sib_usdt(void) +{ + LIBBPF_OPTS(bpf_usdt_opts, opts); + struct test_usdt_o2 *skel; + struct test_usdt_o2__bss *bss; + int err; + + skel = test_usdt_o2__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + return; + + bss = skel->bss; + bss->my_pid = getpid(); + + err = test_usdt_o2__attach(skel); + if (!ASSERT_OK(err, "skel_attach")) + goto cleanup; + + /* usdt1 won't be auto-attached */ + opts.usdt_cookie = 0xcafedeadbeeffeed; + skel->links.usdt1 = bpf_program__attach_usdt(skel->progs.usdt1, + 0 /*self*/, "/proc/self/exe", + "test", "usdt1", &opts); + if (!ASSERT_OK_PTR(skel->links.usdt1, "usdt1_link")) + goto cleanup; + + trigger_func(); + + ASSERT_EQ(bss->usdt1_called, 1, "usdt1_called"); + ASSERT_EQ(bss->usdt1_cookie, 0xcafedeadbeeffeed, "usdt1_cookie"); + ASSERT_EQ(bss->usdt1_arg_cnt, 1, "usdt1_arg_cnt"); + ASSERT_EQ(bss->usdt1_arg, test_value, "usdt1_arg"); + ASSERT_EQ(bss->usdt1_arg_ret, 0, "usdt1_arg_ret"); + ASSERT_EQ(bss->usdt1_arg_size, sizeof(array[0]), "usdt1_arg_size"); + +cleanup: + test_usdt_o2__destroy(skel); +} + + + +void test_usdt_o2(void) +{ + basic_sib_usdt(); +} diff --git a/tools/testing/selftests/bpf/progs/test_usdt_o2.c b/tools/testing/selftests/bpf/progs/test_usdt_o2.c new file mode 100644 index 000000000000..14602aa54578 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_usdt_o2.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */ + +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/usdt.bpf.h> + +int my_pid; + +int usdt1_called; +u64 usdt1_cookie; +int usdt1_arg_cnt; +int usdt1_arg_ret; +u64 usdt1_arg; +int usdt1_arg_size; + +SEC("usdt") +int usdt1(struct pt_regs *ctx) +{ + long tmp; + + if (my_pid != (bpf_get_current_pid_tgid() >> 32)) + return 0; + + __sync_fetch_and_add(&usdt1_called, 1); + + usdt1_cookie = bpf_usdt_cookie(ctx); + usdt1_arg_cnt = bpf_usdt_arg_cnt(ctx); + + usdt1_arg_ret = bpf_usdt_arg(ctx, 0, &tmp); + usdt1_arg = (u64)tmp; + usdt1_arg_size = bpf_usdt_arg_size(ctx, 0); + + return 0; +} + +char _license[] SEC("license") = "GPL";

-- 2.43.0

Andrii Nakryiko

14 Aug 14 Aug

12:04 a.m.

New subject: [PATCH bpf-next v8 2/2] selftests/bpf: Add an usdt_o2 test case in selftests to cover SIB handling logic

On Wed, Aug 6, 2025 at 7:35 PM Jiawei Zhao phoenix500526@163.com wrote:

...

When using GCC on x86-64 to compile an usdt prog with -O1 or higher optimization, the compiler will generate SIB addressing mode for global array and PC-relative addressing mode for global variable, e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".

In this patch:

add usdt_o2 test case to cover SIB addressing usdt argument spec handling logic

Signed-off-by: Jiawei Zhao phoenix500526@163.com

tools/testing/selftests/bpf/Makefile | 8 +++ .../selftests/bpf/prog_tests/usdt_o2.c | 71 +++++++++++++++++++ .../selftests/bpf/progs/test_usdt_o2.c | 37 ++++++++++ 3 files changed, 116 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/usdt_o2.c create mode 100644 tools/testing/selftests/bpf/progs/test_usdt_o2.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 910d8d6402ef..68cf6a9cf05f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) TRUNNER_BPF_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_maps))

+# Use -O2 optimization to generate SIB addressing usdt argument spec +# Only apply on x86 architecture where SIB addressing is relevant +ifeq ($(ARCH), x86) +$(OUTPUT)/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/cpuv4/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/no_alu32/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +endif

Have you considered using GCC's __attribute__((optimize("O2"))) attribute. It seems like Clang doesn't have support for something like that, but we'll still have this covered in BPF CI for GCC-built selftests. Then I'd just add this as another subtest to existing usdt tests.

Can you please try that?

...

# Define test_verifier test runner. # It is much simpler than test_maps/test_progs and sufficiently different from # them (e.g., test.h is using completely pattern), that it's worth just diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c new file mode 100644 index 000000000000..f04b756b3640 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Jiawei Zhao phoenix500526@163.com. */ +#include <test_progs.h>

+#define _SDT_HAS_SEMAPHORES 1 +#include "../sdt.h" +#include "test_usdt_o2.skel.h"

+int lets_test_this(int);

+#define test_value 0xFEDCBA9876543210ULL +#define SEC(name) __attribute__((section(name), used))

+static volatile __u64 array[1] = {test_value}; +unsigned short test_usdt1_semaphore SEC(".probes");

Is semaphore essential to this test?

...

+static __always_inline void trigger_func(void) +{

  /* Base address + offset + (index * scale) */

```
  if (test_usdt1_semaphore) {
```

          for (volatile int i = 0; i <= 0; i++)

                  STAP_PROBE1(test, usdt1, array[i]);

```
  }
```

[...]

赵佳炜

6:48 a.m.

New subject: [PATCH bpf-next v8 2/2] selftests/bpf: Add an usdt_o2 test case in selftests to cover SIB handling logic

...

Have you considered using GCC's __attribute__((optimize("O2"))) attribute. It seems like Clang doesn't have support for something like that, but we'll still have this covered in BPF CI for GCC-built selftests. Then I'd just add this as another subtest to existing usdt tests.

Can you please try that?

Done

...

Is semaphore essential to this test?

It's no essential. I've already removed it in the new patch.

At 2025-08-14 08:04:35, "Andrii Nakryiko" andrii.nakryiko@gmail.com wrote:

...

On Wed, Aug 6, 2025 at 7:35 PM Jiawei Zhao phoenix500526@163.com wrote:

...
When using GCC on x86-64 to compile an usdt prog with -O1 or higher optimization, the compiler will generate SIB addressing mode for global array and PC-relative addressing mode for global variable, e.g. "1@-96(%rbp,%rax,8)" and "-1@4+t1(%rip)".

In this patch:

add usdt_o2 test case to cover SIB addressing usdt argument spec handling logic

Signed-off-by: Jiawei Zhao phoenix500526@163.com

tools/testing/selftests/bpf/Makefile | 8 +++ .../selftests/bpf/prog_tests/usdt_o2.c | 71 +++++++++++++++++++ .../selftests/bpf/progs/test_usdt_o2.c | 37 ++++++++++ 3 files changed, 116 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/usdt_o2.c create mode 100644 tools/testing/selftests/bpf/progs/test_usdt_o2.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 910d8d6402ef..68cf6a9cf05f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -759,6 +759,14 @@ TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) TRUNNER_BPF_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_maps))

+# Use -O2 optimization to generate SIB addressing usdt argument spec +# Only apply on x86 architecture where SIB addressing is relevant +ifeq ($(ARCH), x86) +$(OUTPUT)/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/cpuv4/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +$(OUTPUT)/no_alu32/usdt_o2.test.o: CFLAGS:=$(subst O0,O2,$(CFLAGS)) +endif

Have you considered using GCC's __attribute__((optimize("O2"))) attribute. It seems like Clang doesn't have support for something like that, but we'll still have this covered in BPF CI for GCC-built selftests. Then I'd just add this as another subtest to existing usdt tests.

Can you please try that?

...
# Define test_verifier test runner. # It is much simpler than test_maps/test_progs and sufficiently different from # them (e.g., test.h is using completely pattern), that it's worth just diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o2.c b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c new file mode 100644 index 000000000000..f04b756b3640 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/usdt_o2.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Jiawei Zhao phoenix500526@163.com. */ +#include <test_progs.h>

+#define _SDT_HAS_SEMAPHORES 1 +#include "../sdt.h" +#include "test_usdt_o2.skel.h"

+int lets_test_this(int);

+#define test_value 0xFEDCBA9876543210ULL +#define SEC(name) __attribute__((section(name), used))

+static volatile __u64 array[1] = {test_value}; +unsigned short test_usdt1_semaphore SEC(".probes");

Is semaphore essential to this test?

...
+static __always_inline void trigger_func(void) +{
  /* Base address + offset + (index * scale) */
  if (test_usdt1_semaphore) {
          for (volatile int i = 0; i <= 0; i++)
                  STAP_PROBE1(test, usdt1, array[i]);
  }
+}
[...]

Jiawei Zhao

7 Aug 7 Aug

2:34 a.m.

New subject: [PATCH bpf-next v7 2/2] selftests/bpf: Force -O2 for USDT selftests to cover SIB handling logic

In this patch: - add usdt_o2 test case to cover SIB addressing usdt argument spec handling logic

-- 2.43.0

days inactive

days old

linux-kselftest-mirror@lists.linaro.org

7 comments

participants

tags (0)

participants (3)

Andrii Nakryiko
Jiawei Zhao
赵佳炜