Re: [PATCH bpf] bpf, bpftool: Fix incorrect disasm pc

[Stanislav Fomichev <stfomichev@gmail.com>]

On 10/30, Leon Hwang wrote:
> 
> 
> On 2024/10/30 17:47, Leon Hwang wrote:
> > From: Leon Hwang <leon.hwang@linux.dev>
> > 
> > This patch addresses the bpftool issue "Wrong callq address displayed"[0].
> > 
> > The issue stemmed from an incorrect program counter (PC) value used during
> > disassembly with LLVM or libbfd. To calculate the correct address for
> > relative calls, the PC argument must reflect the actual address in the
> > kernel.
> > 
> > [0] https://github.com/libbpf/bpftool/issues/109
> > 
> > Fixes: e1947c750ffe ("bpftool: Refactor disassembler for JIT-ed programs")
> > Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> > ---
> >  tools/bpf/bpftool/jit_disasm.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
> > index 7b8d9ec89ebd3..fe8fabba4b05f 100644
> > --- a/tools/bpf/bpftool/jit_disasm.c
> > +++ b/tools/bpf/bpftool/jit_disasm.c
> > @@ -114,8 +114,7 @@ disassemble_insn(disasm_ctx_t *ctx, unsigned char *image, ssize_t len, int pc)
> 
> It seems we should update the type of pc from int to __u64, as the type
> of func_ksym is __u64 and the type of pc argument in disassemble
> function of LLVM and libbfd is __u64 for 64 bit arch.

I'm assuming u32 is fine as long as the prog size is under 4G?

> >  	char buf[256];
> >  	int count;
> >  

[..]

> > -	count = LLVMDisasmInstruction(*ctx, image + pc, len - pc, pc,
> > -				      buf, sizeof(buf));
> > +	count = LLVMDisasmInstruction(*ctx, image, len, pc, buf, sizeof(buf));

For my understanding, another way to fix it would be:
	count = LLVMDisasmInstruction(*ctx, image + pc, len - pc, 0,
				      buf, sizeof(buf));

IOW, in the original code, using 0 instead of pc should fix it as well?
Or am I missing something?