[PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions

Zecheng Li posted 10 patches 4 months, 3 weeks ago
There is a newer version of this series
[PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions
Posted by Zecheng Li 4 months, 3 weeks ago
When tracking variable types, instructions that modify a pointer value
in an untracked way can lead to incorrect type propagation. To prevent
this, invalidate the register state when encountering such instructions.

This change invalidates pointer types for various arithmetic and bitwise
operations that current pointer offset tracking doesn't support, like
imul, shl, and, inc, etc.

A special case is added for 'xor reg, reg', which is a common idiom for
zeroing a register. For this, the register state is updated to be a
constant with a value of 0.

This could introduce slight regressions if a variable is zeroed and then
reused. This can be addressed in the future by using all DWARF locations
for instruction tracking instead of only the first one.

Signed-off-by: Zecheng Li <zecheng@google.com>
---
 tools/perf/arch/x86/annotate/instructions.c | 29 +++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index 709c6f7efe82..3c98f72c423f 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -411,6 +411,35 @@ static void update_insn_state_x86(struct type_state *state,
 		return;
 	}
 
+	/* Invalidate register states for other ops which may change pointers */
+	if (has_reg_type(state, dst->reg1) && !dst->mem_ref &&
+	    dwarf_tag(&state->regs[dst->reg1].type) == DW_TAG_pointer_type) {
+		if (!strncmp(dl->ins.name, "imul", 4) || !strncmp(dl->ins.name, "mul", 3) ||
+		    !strncmp(dl->ins.name, "idiv", 4) || !strncmp(dl->ins.name, "div", 3) ||
+		    !strncmp(dl->ins.name, "shl", 3)  || !strncmp(dl->ins.name, "shr", 3) ||
+		    !strncmp(dl->ins.name, "sar", 3)  || !strncmp(dl->ins.name, "and", 3) ||
+		    !strncmp(dl->ins.name, "or", 2)   || !strncmp(dl->ins.name, "neg", 3) ||
+		    !strncmp(dl->ins.name, "inc", 3)  || !strncmp(dl->ins.name, "dec", 3)) {
+			pr_debug_dtp("%s [%x] invalidate reg%d\n",
+						dl->ins.name, insn_offset, dst->reg1);
+			state->regs[dst->reg1].ok = false;
+			state->regs[dst->reg1].copied_from = -1;
+			return;
+		}
+
+		if (!strncmp(dl->ins.name, "xor", 3) && dst->reg1 == src->reg1) {
+			/* xor reg, reg clears the register */
+			pr_debug_dtp("xor [%x] clear reg%d\n",
+				     insn_offset, dst->reg1);
+
+			state->regs[dst->reg1].kind = TSR_KIND_CONST;
+			state->regs[dst->reg1].imm_value = 0;
+			state->regs[dst->reg1].ok = false;
+			state->regs[dst->reg1].copied_from = -1;
+			return;
+		}
+	}
+
 	if (strncmp(dl->ins.name, "mov", 3))
 		return;
 
-- 
2.51.0.384.g4c02a37b29-goog
Re: [PATCH v3 06/10] perf annotate: Invalidate register states for untracked instructions
Posted by Namhyung Kim 4 months, 1 week ago
On Wed, Sep 17, 2025 at 07:58:04PM +0000, Zecheng Li wrote:
> When tracking variable types, instructions that modify a pointer value
> in an untracked way can lead to incorrect type propagation. To prevent
> this, invalidate the register state when encountering such instructions.
> 
> This change invalidates pointer types for various arithmetic and bitwise
> operations that current pointer offset tracking doesn't support, like
> imul, shl, and, inc, etc.
> 
> A special case is added for 'xor reg, reg', which is a common idiom for
> zeroing a register. For this, the register state is updated to be a
> constant with a value of 0.
> 
> This could introduce slight regressions if a variable is zeroed and then
> reused. This can be addressed in the future by using all DWARF locations
> for instruction tracking instead of only the first one.
> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/arch/x86/annotate/instructions.c | 29 +++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index 709c6f7efe82..3c98f72c423f 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -411,6 +411,35 @@ static void update_insn_state_x86(struct type_state *state,
>  		return;
>  	}
>  
> +	/* Invalidate register states for other ops which may change pointers */
> +	if (has_reg_type(state, dst->reg1) && !dst->mem_ref &&
> +	    dwarf_tag(&state->regs[dst->reg1].type) == DW_TAG_pointer_type) {
> +		if (!strncmp(dl->ins.name, "imul", 4) || !strncmp(dl->ins.name, "mul", 3) ||
> +		    !strncmp(dl->ins.name, "idiv", 4) || !strncmp(dl->ins.name, "div", 3) ||
> +		    !strncmp(dl->ins.name, "shl", 3)  || !strncmp(dl->ins.name, "shr", 3) ||
> +		    !strncmp(dl->ins.name, "sar", 3)  || !strncmp(dl->ins.name, "and", 3) ||
> +		    !strncmp(dl->ins.name, "or", 2)   || !strncmp(dl->ins.name, "neg", 3) ||
> +		    !strncmp(dl->ins.name, "inc", 3)  || !strncmp(dl->ins.name, "dec", 3)) {
> +			pr_debug_dtp("%s [%x] invalidate reg%d\n",
> +						dl->ins.name, insn_offset, dst->reg1);
> +			state->regs[dst->reg1].ok = false;
> +			state->regs[dst->reg1].copied_from = -1;
> +			return;
> +		}
> +
> +		if (!strncmp(dl->ins.name, "xor", 3) && dst->reg1 == src->reg1) {
> +			/* xor reg, reg clears the register */
> +			pr_debug_dtp("xor [%x] clear reg%d\n",
> +				     insn_offset, dst->reg1);
> +
> +			state->regs[dst->reg1].kind = TSR_KIND_CONST;
> +			state->regs[dst->reg1].imm_value = 0;
> +			state->regs[dst->reg1].ok = false;

I think .ok should be 'true'.

Thanks,
Namhyung


> +			state->regs[dst->reg1].copied_from = -1;
> +			return;
> +		}
> +	}
> +
>  	if (strncmp(dl->ins.name, "mov", 3))
>  		return;
>  
> -- 
> 2.51.0.384.g4c02a37b29-goog
>