Re: [PATCH v1 3/3] perf annotate: Be robust to annotating without a thread

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Architecture-specific disassembly parsers for LoongArch and s390 dereference `ms->thread` without checking for NULL, causing segmentation faults when annotating without a thread.
--

--- Patch [3]: [PATCH v1 3/3] perf annotate: Be robust to annotating without a thread ---
commit 64d4504d663ab9115662531cbaf640b34642de60
Author: Ian Rogers <irogers@google.com>

perf annotate: Be robust to annotating without a thread

If a thread isn't given to thread__get_arch try harder to determine
the arch for disassembly. Do this by passing an entire map_symbol and
using fallback paths such as reading the e_machine from a map's dso. At the
same time ensure some other uses of map_symbol thread don't assume it is
non-NULL to avoid segvs.

[ ... ]

> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 0a1a7e9cf3efe..8cd90a0a0f8ae 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -271,7 +271,7 @@ static int call__parse(const struct arch *arch, struct ins_operands *ops, struct
>  		.addr = map__objdump_2mem(map, ops->target.addr),
>  	};
>  
> -	if (maps__find_ams(thread__maps(ms->thread), &target) == 0 &&
> +	if (ms->thread && maps__find_ams(thread__maps(ms->thread), &target) == 0 &&

[Severity: Medium]
Since ms->thread is now allowed to be NULL, do the architecture-specific
parsers need similar updates?

For example, looking at loongarch_call__parse() in
tools/perf/util/annotate-arch/annotate-loongarch.c:

	if (maps__find_ams(thread__maps(ms->thread), &target) == 0 &&
	    map__rip_2objdump(target.ms.map, map__map_ip(target.ms.map, target.addr)) == ops->target.addr)

It appears to unconditionally call thread__maps(ms->thread). The same
pattern is also present in loongarch_jump__parse() and s390_call__parse().

Could this regression cause a segmentation fault when running perf annotate
without a thread on LoongArch or s390 binaries?

>  	    map__rip_2objdump(target.ms.map, map__map_ip(target.ms.map, target.addr)) == ops->target.addr)
>  		ops->target.sym = target.ms.sym;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708234646.1040003-1-irogers@google.com?part=3