Re: [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64

On 8/8/26 8:23 PM, Tengda Wu wrote:
> The imm_value in struct type_state_reg is defined as u32, which limits
> the size of values it can pass.
> 
> Promote imm_value from u32 to u64 and adjust the print format specifier
> in pr_debug_dtp() accordingly.
> 
> Signed-off-by: Tengda Wu <[email protected]>
> ---
>   tools/perf/util/annotate-arch/annotate-x86.c | 2 +-
>   tools/perf/util/annotate-data.h              | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index ee4e3e7f3209..eec3d8ce00b8 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c
> @@ -540,7 +540,7 @@ static void update_insn_state_x86(struct type_state *state,
>   			tsr->offset = 0;
>   			tsr->ok = true;
>   
> -			pr_debug_dtp("mov [%x] imm=%#x -> reg%d\n",
> +			pr_debug_dtp("mov [%x] imm=%#"PRIx64" -> reg%d\n",
>   				     insn_offset, tsr->imm_value, dst->reg1);
>   			return;
>   		}
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 453e13bbe3e2..91b83e94c51b 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -173,7 +173,7 @@ extern struct annotated_data_stat ann_data_stat;
>    */
>   struct type_state_reg {
>   	Dwarf_Die type;
> -	u32 imm_value;
> +	u64 imm_value;
>   	/*
One subtlety this introduces: annotated_op_loc.offset is an int, and
immediates are parsed into it via strtol(), so assignments like
tsr->imm_value = src->offset now sign-extend instead of preserving
the 32-bit bit pattern. mov $0xdeadbeef used to track 0xdeadbeef,
now it tracks 0xffffffffdeadbeef, while the register actually holds
the zero-extended value. (Canonical kernel addresses happen to
sign-extend back to the right value, which masks this in the common
case.)

Relatedly, this widening doesn't actually help 64-bit immediates on
the strtol() path - movabs is still truncated to int at parse time.
The real consumers that need u64 are the arm64 adrp and stack paths,
which feed imm_value from ops.source.addr / stack state directly, so
the change is justified. But maybe worth spelling that out, and
considering a follow-up that parses immediates with strtoull() into a
dedicated u64 field instead of overloading the signed offset field.

Thanks,