| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <db33a977-c712-48b7-9be1-83721b23635f@intel.com>
Date: Thu, 16 Oct 2025 11:36:32 +0800
From: "Li, Tianyou" <tianyou.li@...el.com>
To: James Clark <james.clark@...aro.org>, Namhyung Kim <namhyung@...nel.org>
CC: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland
<mark.rutland@....com>, Alexander Shishkin
<alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, "Ian
Rogers" <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, "Kan
Liang" <kan.liang@...ux.intel.com>, Ravi Bangoria <ravi.bangoria@....com>,
<wangyang.guo@...el.com>, <pan.deng@...el.com>, <zhiguo.zhou@...el.com>,
<jiebin.sun@...el.com>, <thomas.falcon@...el.com>, <dapeng1.mi@...el.com>,
<linux-perf-users@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] perf tools annotate: fix a crash when annotate the
same symbol with 's' and 'T'
Hi James,
Thanks for your time to review. Please see my comments inlined.
Regards,
Tianyou
On 10/16/2025 1:30 AM, James Clark wrote:
>
>
> On 15/10/2025 6:20 pm, Tianyou Li wrote:
>> When perf report with annotation for a symbol, press 's' and 'T',
>> then exit
>> the annotate browser. Once annotate the same symbol, the annotate
>> browser
>> will crash.
>>
>> The browser.arch was required to be correctly updated when data type
>> feature was enabled by 'T'. Usually it was initialized by
>> symbol__annotate2
>> function. If a symbol has already been correctly annotated at the first
>> time, it should not call the symbol__annotate2 function again, thus the
>> browser.arch will not get initialized. Then at the second time to
>> show the
>> annotate browser, the data type needs to be displayed but the
>> browser.arch
>> is empty.
>>
>> Stack trace as below:
>>
>> Perf: Segmentation fault
>> -------- backtrace --------
>> #0 0x55d365 in ui__signal_backtrace setup.c:0
>> #1 0x7f5ff1a3e930 in __restore_rt libc.so.6[3e930]
>> #2 0x570f08 in arch__is perf[570f08]
>> #3 0x562186 in annotate_get_insn_location perf[562186]
>> #4 0x562626 in __hist_entry__get_data_type annotate.c:0
>> #5 0x56476d in annotation_line__write perf[56476d]
>> #6 0x54e2db in annotate_browser__write annotate.c:0
>> #7 0x54d061 in ui_browser__list_head_refresh perf[54d061]
>> #8 0x54dc9e in annotate_browser__refresh annotate.c:0
>> #9 0x54c03d in __ui_browser__refresh browser.c:0
>> #10 0x54ccf8 in ui_browser__run perf[54ccf8]
>> #11 0x54eb92 in __hist_entry__tui_annotate perf[54eb92]
>> #12 0x552293 in do_annotate hists.c:0
>> #13 0x55941c in evsel__hists_browse hists.c:0
>> #14 0x55b00f in evlist__tui_browse_hists perf[55b00f]
>> #15 0x42ff02 in cmd_report perf[42ff02]
>> #16 0x494008 in run_builtin perf.c:0
>> #17 0x494305 in handle_internal_command perf.c:0
>> #18 0x410547 in main perf[410547]
>> #19 0x7f5ff1a295d0 in __libc_start_call_main libc.so.6[295d0]
>> #20 0x7f5ff1a29680 in __libc_start_main@@GLIBC_2.34
>> libc.so.6[29680]
>> #21 0x410b75 in _start perf[410b75]
>>
>> Fixes: 1d4374afd000 ("perf annotate: Add 'T' hot key to toggle data
>> type display")
>> Reviewed-by: James Clark <james.clark@...aro.org>
>> Signed-off-by: Tianyou Li <tianyou.li@...el.com>
>> ---
>> tools/perf/ui/browsers/annotate.c | 3 +++
>> tools/perf/util/annotate.c | 2 +-
>> tools/perf/util/annotate.h | 2 ++
>> 3 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/ui/browsers/annotate.c
>> b/tools/perf/ui/browsers/annotate.c
>> index 8fe699f98542..3b27ef1e8490 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -1161,6 +1161,9 @@ int __hist_entry__tui_annotate(struct
>> hist_entry *he, struct map_symbol *ms,
>> if (!annotation__has_source(notes))
>> ui__warning("Annotation has no source code.");
>> }
>> + } else if (evsel__get_arch(evsel, &browser.arch)) {
>> + ui__error("Couldn't get architecture for event '%s'",
>> evsel->name);
>> + return -1;
>> }
>
> symbol_annotate() only fails for negative return values of
> evsel__get_arch(), but evsel__get_arch() has at least two positive
> error return values.
>
> If symbol_annotate() is wrong and it should be != 0 like you have,
> then maybe symbol_annotate() should be fixed in another commit in the
> same patchset as this one. Otherwise you have two calls to the same
> thing right next to each other that handle errors differently.
Thanks James. I will give a try on handling the error message with
symbol__strerror_disassemble. I am conservative to change the code in
symbol_annotate, agreed it should be considered in another patch. Would
like to focus this particular issue and get it fixed properly. Thanks.
>
>> /* Copy necessary information when it's called from perf top */
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index a2e34f149a07..39d6594850f1 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -980,7 +980,7 @@ void symbol__calc_percent(struct symbol *sym,
>> struct evsel *evsel)
>> annotation__calc_percent(notes, evsel, symbol__size(sym));
>> }
>> -static int evsel__get_arch(struct evsel *evsel, struct arch **parch)
>> +int evsel__get_arch(struct evsel *evsel, struct arch **parch)
>> {
>> struct perf_env *env = evsel__env(evsel);
>> const char *arch_name = perf_env__arch(env);
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index eaf6c8aa7f47..d4990bff29a7 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -585,4 +585,6 @@ void debuginfo_cache__delete(void);
>> int annotation_br_cntr_entry(char **str, int br_cntr_nr, u64 *br_cntr,
>> int num_aggr, struct evsel *evsel);
>> int annotation_br_cntr_abbr_list(char **str, struct evsel *evsel,
>> bool header);
>> +
>> +int evsel__get_arch(struct evsel *evsel, struct arch **parch);
>> #endif /* __PERF_ANNOTATE_H */
>
>
Powered by blists - more mailing lists