[PATCH v2 07/10] perf annotate: Invalidate register states for unsupported instructions

[PATCH v2 07/10] perf annotate: Invalidate register states for unsupported instructions

[Zecheng Li]

Invalidate register states when encountering unsupported instructions
that modify pointers, to prevent propagating incorrect pointer types.

On x86, the 'xor' instruction may appear in a predecessor basic block
and zero out a register that invalidates the target register state. This
sometimes relates to tagged pointers and normal programs should not
dereference NULL pointers, so we assume such execution paths are invalid
and do not invalidate states for 'xor' instructions.

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

diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index 540b4d0a01af..03df52a5266d 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -413,6 +413,23 @@ 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, "mov", 3))
 		return;
 
-- 
2.51.0.261.g7ce5a0a67e-goog

Re: [PATCH v2 07/10] perf annotate: Invalidate register states for unsupported instructions

[Namhyung Kim]

On Mon, Aug 25, 2025 at 07:57:48PM +0000, Zecheng Li wrote:
> Invalidate register states when encountering unsupported instructions
> that modify pointers, to prevent propagating incorrect pointer types.
> 
> On x86, the 'xor' instruction may appear in a predecessor basic block
> and zero out a register that invalidates the target register state. This
> sometimes relates to tagged pointers and normal programs should not
> dereference NULL pointers, so we assume such execution paths are invalid
> and do not invalidate states for 'xor' instructions.

Probably we can set it to 0 with TSR_KIND_CONST.

Thanks,
Namhyung

> 
> Signed-off-by: Zecheng Li <zecheng@google.com>
> ---
>  tools/perf/arch/x86/annotate/instructions.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
> index 540b4d0a01af..03df52a5266d 100644
> --- a/tools/perf/arch/x86/annotate/instructions.c
> +++ b/tools/perf/arch/x86/annotate/instructions.c
> @@ -413,6 +413,23 @@ 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, "mov", 3))
>  		return;
>  
> -- 
> 2.51.0.261.g7ce5a0a67e-goog
> 

Re: [PATCH v2 07/10] perf annotate: Invalidate register states for unsupported instructions

[Zecheng Li]

On Sat, Aug 30, 2025 at 3:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Mon, Aug 25, 2025 at 07:57:48PM +0000, Zecheng Li wrote:
> > Invalidate register states when encountering unsupported instructions
> > that modify pointers, to prevent propagating incorrect pointer types.
> >
> > On x86, the 'xor' instruction may appear in a predecessor basic block
> > and zero out a register that invalidates the target register state. This
> > sometimes relates to tagged pointers and normal programs should not
> > dereference NULL pointers, so we assume such execution paths are invalid
> > and do not invalidate states for 'xor' instructions.
>
> Probably we can set it to 0 with TSR_KIND_CONST.
>
It seems TSR_KIND_CONST doesn't relate to a type. Although the value
was set to 0, it still has the pointer type. I see regressions that
has this pattern

xorl    %rax, %rax
mov    %rax, (%rsp)

and sometimes

xorl    %rax, %rax
... (some branches)
mov    (%rax), %rbx

Normally NULL pointer dereference should not happen, so I assume such
execution paths are invalid.

Re: [PATCH v2 07/10] perf annotate: Invalidate register states for unsupported instructions

[Namhyung Kim]

On Wed, Sep 03, 2025 at 04:54:39PM -0400, Zecheng Li wrote:
> On Sat, Aug 30, 2025 at 3:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Mon, Aug 25, 2025 at 07:57:48PM +0000, Zecheng Li wrote:
> > > Invalidate register states when encountering unsupported instructions
> > > that modify pointers, to prevent propagating incorrect pointer types.
> > >
> > > On x86, the 'xor' instruction may appear in a predecessor basic block
> > > and zero out a register that invalidates the target register state. This
> > > sometimes relates to tagged pointers and normal programs should not
> > > dereference NULL pointers, so we assume such execution paths are invalid
> > > and do not invalidate states for 'xor' instructions.
> >
> > Probably we can set it to 0 with TSR_KIND_CONST.
> >
> It seems TSR_KIND_CONST doesn't relate to a type. Although the value
> was set to 0, it still has the pointer type. I see regressions that
> has this pattern
> 
> xorl    %rax, %rax
> mov    %rax, (%rsp)
> 
> and sometimes
> 
> xorl    %rax, %rax
> ... (some branches)
> mov    (%rax), %rbx
> 
> Normally NULL pointer dereference should not happen, so I assume such
> execution paths are invalid.

Agreed, I think there should be a place to set the pointer to a valid
value again (before use).

Thanks,
Namhyung