Re: [PATCHv6 08/13] selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete update of USDT_NOP causes selftest failures and silent loss of libbpf uprobe optimization.
- [Medium] The x86 uprobes subsystem does not support optimizing `nop10` instructions, contradicting the commit message's claim.
--

commit 0e159bff3c4759d25ce27fa625273c5dc59c418b
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Emit nop,nop10 instructions combo for x86_64 arch

This commit updates the USDT_NOP macro for x86_64 to emit a nop,nop10
instruction sequence instead of nop,nop5. This syncs the latest changes
from libbpf usdt.h, leaving it up to the library to use the desirable
nop instruction.

Link: https://github.com/libbpf/usdt/commit/9018f82577d1dad7ed628d9efdaffc09f8f2241b

> Now that we have nop10 optimization support in kernel, let's emit
> nop,nop10 for usdt probe.

[Severity: Medium]
Is this description accurate regarding kernel support for nop10 optimization?

Looking at x86 uprobes in arch/x86/kernel/uprobes.c:can_optimize(), the kernel
explicitly restricts optimizations to 5-byte instructions:

    if (!insn->x86_64 || insn->length != 5)
        return false;

Does the current kernel actually support optimizing a 10-byte nop, or will
this fail the instruction length check and refuse to optimize?

> diff --git a/tools/testing/selftests/bpf/usdt.h b/tools/testing/selftests/bpf/usdt.h
> index c71e21df38b32..75687f50f4e21 100644
> --- a/tools/testing/selftests/bpf/usdt.h
> +++ b/tools/testing/selftests/bpf/usdt.h
[ ... ]
> @@ -313,7 +313,7 @@ struct usdt_sema { volatile unsigned short active; };
>  #if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
>  #define USDT_NOP			nop 0
>  #elif defined(__x86_64__)
> -#define USDT_NOP                       .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
> +#define USDT_NOP                       .byte 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 /* nop, nop10 */

[Severity: High]
Will this incomplete update cause selftest failures and a silent loss of
uprobe optimization in libbpf?

The selftest suite in tools/testing/selftests/bpf/prog_tests/usdt.c expects
the old 6-byte sequence and will fail an assertion when it cannot find it
in subtest_optimized_attach():

    addr_2 = find_instr(usdt_2, nop1_nop5_combo, 6);
    if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop5_combo"))

Additionally, libbpf hardcodes the old 6-byte sequence in
tools/lib/bpf/usdt.c:has_nop_combo():

    unsigned char nop_combo[6] = {
        0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 /* nop,nop5 */
    };

Without updating these locations, won't libbpf fail to detect the new
11-byte sequence, causing it to fall back to the 1-byte nop which cannot
be optimized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703114917.238144-9-jolsa@kernel.org?part=1