Re: [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump

Hi Shuai, thank you for your time.

On 2026/8/10 21:08, Shuai Xue wrote:
> 
> 
> On 8/8/26 8:23 PM, Tengda Wu wrote:
>> The jump and adrp instructions parsed by libcapstone currently lack
>> symbolic representation and use a '#' prefix for addresses. This
>> format is inconsistent with objdump's output, which causes subsequent
>> parsing in jump__parse() and arm64_mov__parse() to fail.
>>
>> Example mismatch:
>>    Current: b #0xffff8000800114c8
>>    Fix:     b ffff8000800114c8 <el0t_64_sync+0x108>
>>
>>    Current: adrp x18, #0xffff800081f5f000
>>    Fix:     adrp x18, ffff800081f5f000 <this_cpu_vector>
>>
>> Fix this by implementing extended formatting for these arm64
>> instructions during symbol__disassemble_capstone(). This ensures
>> the output matches objdump's expected style, including the raw
>> address and the associated <symbol+offset> suffix.
>>
>> Signed-off-by: Tengda Wu <[email protected]>
>> ---
>>   tools/perf/util/capstone.c | 136 +++++++++++++++++++++++++++++++++----
>>   tools/perf/util/disasm.c   |   5 ++
>>   tools/perf/util/disasm.h   |   1 +
>>   3 files changed, 130 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/util/capstone.c b/tools/perf/util/capstone.c
>> index 74213daf8786..fb8a2bc5558f 100644
>> --- a/tools/perf/util/capstone.c
>> +++ b/tools/perf/util/capstone.c
>> @@ -3,6 +3,7 @@
>>     #include <errno.h>
>>   #include <inttypes.h>
>> +#include <stdlib.h>
>>   #include <string.h>
>>     #include <dlfcn.h>
>> @@ -31,6 +32,10 @@
>>   #define CS_MODE_RISCVC   4
>>   #endif
>>   +#if CS_VERSION_MAJOR < 4
>> +#define ARM64_GRP_BRANCH_RELATIVE  7
> 
> Please add a comment explaining where '7' comes from
> (CS_GRP_BRANCH_RELATIVE in capstone v3), otherwise it reads like an
> arbitrary magic number.
> 

Um, this was done following Namhyung's approach. That said, adding a
comment would certainly make this definition clearer -- will add it.

Also, I couldn't find CS_GRP_BRANCH_RELATIVE in v3. From what I can see,
it was originally introduced in v4, together with ARM64_GRP_BRANCH_RELATIVE
(see https://github.com/capstone-engine/capstone/commit/a09a81813c83).
So the comment might look something like this:

  #define ARM64_GRP_BRANCH_RELATIVE  7  /* = CS_GRP_BRANCH_RELATIVE */

Is this acceptable?

Thanks,
Tengda

>> +#endif
>> +
>>   #ifdef LIBCAPSTONE_DLOPEN
>>   static void *perf_cs_dll_handle(void)
>>   {
>> @@ -225,6 +230,12 @@ static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_
>>            * on x86 by investigating instruction details.
>>            */
>>           perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
>> +    } else if (arch == CS_ARCH_ARM64) {
>> +        /*
>> +         * Same as x86: arm64 needs instruction details to resolve
>> +         * symbolic addresses.
>> +         */
>> +        perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
>>       }
>>         return 0;
>> @@ -299,10 +310,6 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
>>       struct map *map = args->ms->map;
>>       struct symbol *sym;
>>   -    /* TODO: support more architectures */
>> -    if (!arch__is_x86(args->arch))
>> -        return;
>> -
>>       if (insn->detail == NULL)
>>           return;
>>   @@ -354,6 +361,116 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
>>       }
>>   }
>>   +static int print_default_format(struct cs_insn *insn, char *buf, size_t len)
>> +{
>> +    return scnprintf(buf, len, "       %-7s %s",
>> +             insn->mnemonic, insn->op_str);
>> +}
>> +
>> +static void format_capstone_insn_x86(struct cs_insn *insn, char *buf,
>> +                     size_t len, struct annotate_args *args,
>> +                     u64 addr)
>> +{
>> +    int printed;
>> +
>> +    printed = print_default_format(insn, buf, len);
>> +    buf += printed;
>> +    len -= printed;
>> +
>> +    print_capstone_detail(insn, buf, len, args, addr);
>> +}
>> +
>> +static bool is_pc_relative_insn(struct cs_insn *insn)
>> +{
>> +    int i;
>> +
>> +    if (insn->id == ARM64_INS_ADR || insn->id == ARM64_INS_ADRP)
>> +        return true;
>> +
>> +    if (insn->detail == NULL)
>> +        return false;
>> +
>> +    for (i = 0; i < insn->detail->groups_count; i++) {
>> +        if (insn->detail->groups[i] == ARM64_GRP_JUMP ||
>> +            insn->detail->groups[i] == ARM64_GRP_CALL ||
>> +            insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)
>> +            return true;
>> +    }
>> +
>> +    return false;
>> +}
>> +
>> +static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
>> +                       size_t len, struct annotate_args *args)
>> +{
>> +    struct map *map = args->ms->map;
>> +    struct symbol *sym;
>> +    char *last_imm, *endptr;
>> +    u64 orig_addr, addr;
>> +    struct map *found_map = NULL;
>> +
>> +    print_default_format(insn, buf, len);
>> +    /*
>> +     * Adjust instructions to keep the existing behavior with objdump.
>> +     *
>> +     * Example conversion:
>> +     * From: b #0xffff8000800114c8
>> +     * To:   b ffff8000800114c8 <el0t_64_sync+0x108>
>> +     */
>> +    if (is_pc_relative_insn(insn)) {
>> +        /* Extract last immediate value as address */
>> +        last_imm = strrchr(buf, '#');
>> +        if (!last_imm)
>> +            return;
>> +
>> +        orig_addr = strtoull(last_imm + 1, &endptr, 16);
>> +        if (endptr == last_imm + 1)
>> +            return;
>> +
>> +        addr = map__objdump_2mem(map, orig_addr);
>> +
>> +        /* Relocate map that contains the address */
>> +        if (dso__kernel(map__dso(map))) {
>> +            found_map = maps__find(map__kmaps(map), addr);
>> +            if (found_map == NULL)
>> +                return;
>> +            map = found_map;
>> +        }
>> +
>> +        /* Convert it to map-relative address for search */
>> +        addr = map__map_ip(map, addr);
>> +
>> +        sym = map__find_symbol(map, addr);
>> +        if (sym == NULL) {
>> +            map__put(found_map);
>> +            return;
>> +        }
>> +
>> +        /* Symbolize the resolved address */
>> +        len = len - (last_imm - buf);
>> +        if (addr == sym->start) {
>> +            scnprintf(last_imm, len, "%"PRIx64" <%s>",
>> +                  orig_addr, sym->name);
>> +        } else {
>> +            scnprintf(last_imm, len, "%"PRIx64" <%s+%#"PRIx64">",
>> +                  orig_addr, sym->name, addr - sym->start);
>> +        }
>> +        map__put(found_map);
> 
> 
> This whole sequence (objdump_2mem -> kmaps relocation -> map_ip ->
> find_symbol -> scnprintf the "<sym+off>" string) is almost identical to
> what print_capstone_detail() does for x86 RIP-relative operands.  Could
> you factor out a common helper so the two paths don't diverge over time?
> 
> Thanks.
> Shuai