summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIan Rogers <irogers@google.com>2026-07-22 21:59:46 -0700
committerNamhyung Kim <namhyung@kernel.org>2026-07-27 17:13:08 -0700
commitae89153d5e50d22e04a084829f74dc5a9ae6eb6d (patch)
tree4009d68e04bf0c9bf2259e98b051420685064be1
parent08b0cd02e91d4602e8224c301b47c9d670a04c0d (diff)
downloadlinux-next-ae89153d5e50d22e04a084829f74dc5a9ae6eb6d.tar.gz
linux-next-ae89153d5e50d22e04a084829f74dc5a9ae6eb6d.zip
perf ui hists: In report UI ensure thread is set with reference counting
Populates the map_symbol thread in the UI code to fix the e_machine lookup for cross-platform disassembly when using the annotate action. At the same time, refactor the UI options and actions generation to comply with the perf subsystem's strict reference counting abstraction requirements for 'struct map_symbol' and 'struct thread'. Introduce explicit reference acquiring via map_symbol__copy() and thread__get() for menu items, safely clean them up between menu iterations using a new free_popup_actions() helper, and utilize insulated temporary action variables for direct hotkey handler execution. Fixes: 0e26ba5a8774 ("perf disasm: Refactor arch__find and initialization of arch structs") Reported-by: Josh Stone <jistone@redhat.com> Assisted-by: Antigravity:gemini-3.5-flash Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
-rw-r--r--tools/perf/ui/browsers/hists.c135
1 files changed, 87 insertions, 48 deletions
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f5e13fc125c6..684df550ba49 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2357,6 +2357,16 @@ static int hists_browser__scnprintf_title(struct hist_browser *browser, char *bf
return printed;
}
+struct popup_action {
+ unsigned long time;
+ struct thread *thread;
+ int (*fn)(struct hist_browser *browser, struct popup_action *act);
+ struct map_symbol ms;
+ int socket;
+ enum rstype rstype;
+
+};
+
static inline void free_popup_options(char **options, int n)
{
int i;
@@ -2365,6 +2375,16 @@ static inline void free_popup_options(char **options, int n)
zfree(&options[i]);
}
+static inline void free_popup_actions(struct popup_action *actions, int n)
+{
+ int i;
+
+ for (i = 0; i < n; ++i) {
+ map_symbol__exit(&actions[i].ms);
+ memset(&actions[i], 0, sizeof(struct popup_action));
+ }
+}
+
/*
* Only runtime switching of perf data file will make "input_name" point
* to a malloced buffer. So add "is_input_name_malloced" flag to decide
@@ -2454,16 +2474,6 @@ close_file_and_continue:
return ret;
}
-struct popup_action {
- unsigned long time;
- struct thread *thread;
- int (*fn)(struct hist_browser *browser, struct popup_action *act);
- struct map_symbol ms;
- int socket;
- enum rstype rstype;
-
-};
-
static int
do_annotate(struct hist_browser *browser, struct popup_action *act)
{
@@ -2541,7 +2551,7 @@ add_annotate_opt(struct popup_action *act, char **optstr,
if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
return 0;
- act->ms = *ms;
+ map_symbol__copy(&act->ms, ms);
act->fn = do_annotate;
return 1;
}
@@ -2573,7 +2583,7 @@ add_annotate_type_opt(struct popup_action *act, char **optstr,
static int
do_zoom_thread(struct hist_browser *browser, struct popup_action *act)
{
- struct thread *thread = act->thread;
+ struct thread *thread = act->ms.thread;
if ((!hists__has(browser->hists, thread) &&
!hists__has(browser->hists, comm)) || thread == NULL)
@@ -2628,7 +2638,7 @@ add_thread_opt(struct hist_browser *browser, struct popup_action *act,
if (ret < 0)
return 0;
- act->thread = thread;
+ act->ms.thread = thread__get(thread);
act->fn = do_zoom_thread;
return 1;
}
@@ -2675,7 +2685,7 @@ add_dso_opt(struct hist_browser *browser, struct popup_action *act,
__map__is_kernel(map) ? "the Kernel" : dso__short_name(map__dso(map))) < 0)
return 0;
- act->ms.map = map;
+ act->ms.map = map__get(map);
act->fn = do_zoom_dso;
return 1;
}
@@ -2720,7 +2730,7 @@ add_map_opt(struct hist_browser *browser,
if (asprintf(optstr, "Browse map details") < 0)
return 0;
- act->ms.map = map;
+ act->ms.map = map__get(map);
act->fn = do_browse_map;
return 1;
}
@@ -2734,8 +2744,8 @@ do_run_script(struct hist_browser *browser,
int n = 0;
len = 100;
- if (act->thread)
- len += strlen(thread__comm_str(act->thread));
+ if (act->ms.thread)
+ len += strlen(thread__comm_str(act->ms.thread));
else if (act->ms.sym)
len += strlen(act->ms.sym->name);
script_opt = malloc(len);
@@ -2743,9 +2753,9 @@ do_run_script(struct hist_browser *browser,
return -1;
script_opt[0] = 0;
- if (act->thread) {
+ if (act->ms.thread) {
n = scnprintf(script_opt, len, " -c %s ",
- thread__comm_str(act->thread));
+ thread__comm_str(act->ms.thread));
} else if (act->ms.sym) {
n = scnprintf(script_opt, len, " -S %s ",
act->ms.sym->name);
@@ -2800,7 +2810,7 @@ add_script_opt_2(struct popup_action *act, char **optstr,
return 0;
}
- act->thread = thread;
+ act->ms.thread = thread__get(thread);
act->ms.sym = sym;
act->fn = do_run_script;
return 1;
@@ -3097,6 +3107,8 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
key = 0; // reset key
do_hotkey: // key came straight from options ui__popup_menu()
+ free_popup_options(options, MAX_OPTIONS);
+ free_popup_actions(actions, MAX_OPTIONS);
choice = nr_options = 0;
key = hist_browser__run(browser, helpline, warn_lost_event, key);
@@ -3156,24 +3168,40 @@ do_hotkey: // key came straight from options ui__popup_menu()
}
if (!browser->selection->sym) {
+ struct map_symbol source_ms;
+
if (!browser->he_selection)
continue;
+ memset(&source_ms, 0, sizeof(source_ms));
+
if (sort__mode == SORT_MODE__BRANCH) {
bi = browser->he_selection->branch_info;
if (!bi || !bi->to.ms.map)
continue;
- actions->ms.sym = symbol__new_unresolved(bi->to.al_addr, bi->to.ms.map);
- actions->ms.map = bi->to.ms.map;
+ source_ms.sym =
+ symbol__new_unresolved(
+ bi->to.al_addr,
+ bi->to.ms.map);
+ source_ms.thread = bi->to.ms.thread;
+ source_ms.map = bi->to.ms.map;
} else {
- actions->ms.sym = symbol__new_unresolved(browser->he_selection->ip,
- browser->selection->map);
- actions->ms.map = browser->selection->map;
+ source_ms.sym =
+ symbol__new_unresolved(
+ browser->he_selection->ip,
+ browser->selection->map);
+ source_ms.thread = browser->selection->thread;
+ source_ms.map = browser->selection->map;
}
- if (!actions->ms.sym)
+ if (!source_ms.sym)
continue;
+
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ map_symbol__copy(&hotkey_act.ms, &source_ms);
+ do_annotate(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
} else {
if (symbol__annotation(browser->selection->sym)->src == NULL) {
ui_browser__warning(&browser->b, delay_secs * 2,
@@ -3183,18 +3211,20 @@ do_hotkey: // key came straight from options ui__popup_menu()
continue;
}
- actions->ms.map = browser->selection->map;
- actions->ms.sym = browser->selection->sym;
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ map_symbol__copy(&hotkey_act.ms, browser->selection);
+ do_annotate(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
}
-
- do_annotate(browser, actions);
continue;
case 'P':
hist_browser__dump(browser);
continue;
case 'd':
- actions->ms.map = map;
- do_zoom_dso(browser, actions);
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ hotkey_act.ms.map = map__get(map);
+ do_zoom_dso(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
continue;
case 'k':
if (browser->selection != NULL)
@@ -3209,12 +3239,16 @@ do_hotkey: // key came straight from options ui__popup_menu()
verbose);
continue;
case 't':
- actions->thread = thread;
- do_zoom_thread(browser, actions);
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ hotkey_act.ms.thread = thread__get(thread);
+ do_zoom_thread(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
continue;
case 'S':
- actions->socket = socked_id;
- do_zoom_socket(browser, actions);
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ hotkey_act.socket = socked_id;
+ do_zoom_socket(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
continue;
case '/':
if (ui_browser__input_window("Symbol to show",
@@ -3232,9 +3266,11 @@ do_hotkey: // key came straight from options ui__popup_menu()
continue;
case 'r':
if (is_report_browser(hbt)) {
- actions->thread = NULL;
- actions->ms.sym = NULL;
- do_run_script(browser, actions);
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
+ hotkey_act.ms.thread = NULL;
+ hotkey_act.ms.sym = NULL;
+ do_run_script(browser, &hotkey_act);
+ map_symbol__exit(&hotkey_act.ms);
}
continue;
case 's':
@@ -3306,20 +3342,19 @@ do_hotkey: // key came straight from options ui__popup_menu()
continue;
}
- actions->ms.map = map;
+ memset(&hotkey_act, 0, sizeof(hotkey_act));
top = pstack__peek(browser->pstack);
if (top == &browser->hists->dso_filter) {
- /*
- * No need to set actions->dso here since
- * it's just to remove the current filter.
- */
- do_zoom_dso(browser, actions);
+ hotkey_act.ms.map = map__get(map);
+ do_zoom_dso(browser, &hotkey_act);
} else if (top == &browser->hists->thread_filter) {
- actions->thread = thread;
- do_zoom_thread(browser, actions);
+ hotkey_act.ms.thread = thread__get(thread);
+ do_zoom_thread(browser, &hotkey_act);
} else if (top == &browser->hists->socket_filter) {
- do_zoom_socket(browser, actions);
+ hotkey_act.socket = socked_id;
+ do_zoom_socket(browser, &hotkey_act);
}
+ map_symbol__exit(&hotkey_act.ms);
continue;
}
case 'q':
@@ -3456,9 +3491,13 @@ skip_scripting:
if (key == K_SWITCH_INPUT_DATA)
break;
+
+ free_popup_options(options, MAX_OPTIONS);
+ free_popup_actions(actions, MAX_OPTIONS);
}
out_free_stack:
pstack__delete(browser->pstack);
+ free_popup_actions(actions, MAX_OPTIONS);
out:
hist_browser__delete(browser);
free_popup_options(options, MAX_OPTIONS);