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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Expanding `imm_value` to `u64` fails to support 64-bit immediates and introduces a critical sign-extension bug for 32-bit constants due to an intermediate 32-bit field.
--

--- Patch [16]: [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 ---
commit df449a8cd3aeb4d61dac5a7e48f7afaddbe8b587
Author: Tengda Wu <[email protected]>

perf annotate-data: Expand type_state_reg imm_value to u64

The imm_value in struct type_state_reg is promoted from u32 to u64 to
support larger immediate values. The print format specifier is adjusted
accordingly.

> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 453e13bbe3e2c..91b83e94c51b9 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;

[Severity: Medium]
Does this change introduce a sign-extension issue for 32-bit constants?

The offset field in struct annotated_op_loc (like src->offset) is currently
a 32-bit signed int. When we assign imm_value = src->offset in
update_insn_state_x86(), a 32-bit immediate like 0xdeadbeef will now be
sign-extended into this new u64 field instead of zero-extended.

Additionally, because the intermediate offset is still an int, does this
fail to support 64-bit immediates? They would be truncated during parsing
when extract_op_location_x86() assigns op_loc->offset = strtol(...).

>  	/*
>  	 * The offset within the struct that the register points to.
>  	 * A value of 0 means the register points to the beginning.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16