Re: [PATCH bpf-next v2] bpftool: Enable aarch64 ISA extensions for JIT disassembly

[Leon Hwang <leon.hwang@linux.dev>]

Yonghong Song wrote:
> 
> 
> On 3/10/26 4:05 PM, bot+bpf-ci@kernel.org wrote:
>>> diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/
>>> jit_disasm.c
>>> --- a/tools/bpf/bpftool/jit_disasm.c
>>> +++ b/tools/bpf/bpftool/jit_disasm.c
>>> @@ -93,7 +93,16 @@ init_context(disasm_ctx_t *ctx, const char *arch,
>>>           p_err("Failed to retrieve triple");
>>>           return -1;
>>>       }
>>> -    *ctx = LLVMCreateDisasm(triple, NULL, 0, NULL,
>>> symbol_lookup_callback);
>>> +
>>> +    /*
>>> +     * Enable all aarch64 ISA extensions so the disassembler can
>>> handle any
>>> +     * instruction the kernel JIT might emit (e.g. ARM64 LSE atomics).
>>> +     */
>>> +    if (strncmp(triple, "aarch64", 7))
>>> +        *ctx = LLVMCreateDisasmCPUFeatures(triple, "", "+all", NULL,
>>> 0, NULL,
>>> +                           symbol_lookup_callback);
>>> +    else
>>> +        *ctx = LLVMCreateDisasm(triple, NULL, 0, NULL,
>>> symbol_lookup_callback);
>> Isn't this condition inverted? strncmp() returns 0 when the
>> strings match, so when triple starts with "aarch64" the condition
>> is false and the else branch runs, which calls the old
>> LLVMCreateDisasm() without any features enabled.
>>
>> Conversely, for non-aarch64 targets (e.g. x86_64), strncmp()
>> returns non-zero, making the condition true, and
>> LLVMCreateDisasmCPUFeatures() with "+all" gets called on the
>> wrong architecture.
>>
>> It looks like this should be:
>>
>>     if (!strncmp(triple, "aarch64", 7))
>>
>> The v1 of this patch used strstr(triple, "aarch64") which has the
>> correct polarity (returns non-NULL on match). The switch to
>> strncmp() for v2 appears to have lost the negation.
> 
> AI is correct. The condition should be
>     if (strncmp(triple, "aarch64", 7) == 0)
> 

Agreed.

After addressing it,

Acked-by: Leon Hwang <leon.hwang@linux.dev>

[...]