Re: [PATCH v4 3/5] selftest/bpf: Fixing powerpc JIT disassembly failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Truncating 8 bytes on 32-bit PowerPC incorrectly removes a valid instruction from the JIT disassembly output.
--

commit 1ee8cba34b15e2d991f046b6802fda4443b02cd0
Author: Abhishek Dubey <adubey@linux.ibm.com>

selftest/bpf: Fixing powerpc JIT disassembly failure

This commit truncates the last 8 bytes from JITed buffers on PowerPC
architectures prior to disassembly in the BPF selftests. This is intended
to prevent the disassembler from misinterpreting a trailing dummy
trampoline address as a valid instruction.

> diff --git a/tools/testing/selftests/bpf/jit_disasm_helpers.c b/tools/testing/selftests/bpf/jit_disasm_helpers.c
> --- a/tools/testing/selftests/bpf/jit_disasm_helpers.c
> +++ b/tools/testing/selftests/bpf/jit_disasm_helpers.c

[ ... ]

> @@ -216,9 +218,18 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
>  	if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd #2"))
>  		goto out;
>  
> +	/*
> +	 * last 8 bytes contains dummy_trampoline address in JIT
> +	 * output for 64-bit and 32-bit powerpc, which can't
> +	 * disassemble a to valid instruction.
> +	 */
> +	triple = LLVMGetDefaultTargetTriple();
> +	if (strstr(triple, "powerpc"))
> +		trunc_len = 8;

Will this remove a valid instruction on 32-bit PowerPC targets?

The dummy trampoline address is stored as an unsigned long, which occupies 8
bytes on 64-bit PowerPC, but only 4 bytes on 32-bit PowerPC. 

Because the long branch stub ends with a 4-byte bctr instruction followed by
the dummy address, unconditionally truncating 8 bytes on 32-bit PowerPC would
remove both the 4-byte dummy address and the valid 4-byte bctr instruction
from the disassembly.

Does the truncation length need to be determined dynamically based on whether
the target is 32-bit or 64-bit?

>  	for (pc = 0, i = 0; i < jited_funcs; ++i) {
>  		fprintf(text_out, "func #%d:\n", i);
> -		disasm_one_func(text_out, image + pc, func_lens[i]);
> +		disasm_one_func(text_out, image + pc, func_lens[i] - trunc_len);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517214043.12975-1-adubey@linux.ibm.com?part=3