Until now, the --code-with-type option is available only on stdio.
But it was an artifical limitation because of an implemention issue.
Implement the same logic in annotation_line__write() for stdio2/TUI.
Make disasm_line__write() return the number of printed characters so
that it can skip unnecessary operations when the screen is full.
Remove the limitation and update the man page.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-annotate.txt | 1 -
tools/perf/builtin-annotate.c | 5 --
tools/perf/ui/browsers/annotate.c | 6 +++
tools/perf/util/annotate.c | 61 +++++++++++++++++++---
4 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index 46090c5b42b4762f..547f1a2680185e3c 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -170,7 +170,6 @@ include::itrace.txt[]
--code-with-type::
Show data type info in code annotation (for memory instructions only).
- Currently it only works with --stdio option.
SEE ALSO
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 9833c2c82a2fee46..6debd725392db4a4 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
symbol_conf.annotate_data_sample = true;
} else if (annotate_opts.code_with_type) {
symbol_conf.annotate_data_member = true;
-
- if (!annotate.use_stdio) {
- pr_err("--code-with-type only works with --stdio.\n");
- goto out_delete;
- }
}
setup_browser(true);
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 23bea5b165774ae7..cdee1969f3131a7c 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -4,6 +4,7 @@
#include "../ui.h"
#include "../../util/annotate.h"
#include "../../util/debug.h"
+#include "../../util/debuginfo.h"
#include "../../util/dso.h"
#include "../../util/hist.h"
#include "../../util/sort.h"
@@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
ui_helpline__push("Press ESC to exit");
+ if (annotate_opts.code_with_type)
+ browser.dbg = debuginfo__new(dso__long_name(dso));
+
browser.b.width = notes->src->widths.max_line_len;
browser.b.nr_entries = notes->src->nr_entries;
browser.b.entries = ¬es->src->source;
@@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
ret = annotate_browser__run(&browser, evsel, hbt);
+ if (annotate_opts.code_with_type)
+ debuginfo__delete(browser.dbg);
if (not_annotated && !notes->src->tried_source)
annotated_source__purge(notes->src);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d69e406c1bc289cd..06ddc7a9f58722a4 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
};
struct annotation_line *al;
+ if (annotate_opts.code_with_type) {
+ evsel__get_arch(apd->evsel, &apd->arch);
+ apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
+ }
+
list_for_each_entry(al, ¬es->src->source, node) {
if (annotation_line__filter(al))
continue;
@@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
wops.first_line = false;
}
+ if (annotate_opts.code_with_type)
+ debuginfo__delete(apd->dbg);
+
return 0;
}
@@ -1743,7 +1751,7 @@ static double annotation_line__max_percent(struct annotation_line *al,
return percent_max;
}
-static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
+static int disasm_line__write(struct disasm_line *dl, struct annotation *notes,
void *obj, char *bf, size_t size,
void (*obj__printf)(void *obj, const char *fmt, ...),
void (*obj__write_graph)(void *obj, int graph))
@@ -1771,8 +1779,8 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
obj__printf(obj, " ");
}
- disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
- notes->src->widths.max_ins_name);
+ return disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
+ notes->src->widths.max_ins_name);
}
static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
@@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
width -= printed + 3;
- disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
+ printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
+ obj__printf, obj__write_graph);
+
+ obj__printf(obj, "%s", bf);
+ width -= printed;
+
+ if (annotate_opts.code_with_type && apd->dbg) {
+ struct annotated_data_type *data_type;
+ int offset = 0;
+
+ data_type = __hist_entry__get_data_type(apd->he, apd->arch,
+ apd->dbg,
+ disasm_line(al),
+ &offset);
+ if (data_type && data_type != NO_TYPE) {
+ char member[256];
+
+ printed = scnprintf(bf, sizeof(bf),
+ "\t\t# data-type: %s",
+ data_type->self.type_name);
- obj__printf(obj, "%-*s", width, bf);
+ if (data_type != &stackop_type &&
+ data_type != &canary_type &&
+ sizeof(bf) > (size_t)printed) {
+ printed += scnprintf(bf + printed,
+ sizeof(bf) - printed,
+ " +%#x", offset);
+ }
+
+ if (annotated_data_type__get_member_name(data_type,
+ member,
+ sizeof(member),
+ offset) &&
+ sizeof(bf) > (size_t)printed) {
+ printed += scnprintf(bf + printed,
+ sizeof(bf) - printed,
+ " (%s)", member);
+ }
- (void)apd;
+ obj__printf(obj, "%-*s", width, bf);
+ } else {
+ obj__printf(obj, "%-*s", width, " ");
+ }
+ } else {
+ obj__printf(obj, "%-*s", width, " ");
+ }
}
}
--
2.50.0On Tue, Jul 15, 2025 at 10:00:51PM -0700, Namhyung Kim wrote:
> Until now, the --code-with-type option is available only on stdio.
> But it was an artifical limitation because of an implemention issue.
>
> Implement the same logic in annotation_line__write() for stdio2/TUI.
> Make disasm_line__write() return the number of printed characters so
> that it can skip unnecessary operations when the screen is full.
>
> Remove the limitation and update the man page.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-annotate.txt | 1 -
> tools/perf/builtin-annotate.c | 5 --
> tools/perf/ui/browsers/annotate.c | 6 +++
> tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> 4 files changed, 61 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index 46090c5b42b4762f..547f1a2680185e3c 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -170,7 +170,6 @@ include::itrace.txt[]
>
> --code-with-type::
> Show data type info in code annotation (for memory instructions only).
> - Currently it only works with --stdio option.
>
>
> SEE ALSO
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 9833c2c82a2fee46..6debd725392db4a4 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> symbol_conf.annotate_data_sample = true;
> } else if (annotate_opts.code_with_type) {
> symbol_conf.annotate_data_member = true;
> -
> - if (!annotate.use_stdio) {
> - pr_err("--code-with-type only works with --stdio.\n");
> - goto out_delete;
> - }
> }
>
> setup_browser(true);
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 23bea5b165774ae7..cdee1969f3131a7c 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -4,6 +4,7 @@
> #include "../ui.h"
> #include "../../util/annotate.h"
> #include "../../util/debug.h"
> +#include "../../util/debuginfo.h"
> #include "../../util/dso.h"
> #include "../../util/hist.h"
> #include "../../util/sort.h"
> @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>
> ui_helpline__push("Press ESC to exit");
>
> + if (annotate_opts.code_with_type)
> + browser.dbg = debuginfo__new(dso__long_name(dso));
Some error checking here to tell the user if debuginfo isn't available
and hints on how to get it in place?
> +
> browser.b.width = notes->src->widths.max_line_len;
> browser.b.nr_entries = notes->src->nr_entries;
> browser.b.entries = ¬es->src->source;
> @@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> + if (annotate_opts.code_with_type)
> + debuginfo__delete(browser.dbg);
This is a local variable, so no need to zero browser.dbg after deleting
it, ok.
> if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index d69e406c1bc289cd..06ddc7a9f58722a4 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> };
> struct annotation_line *al;
>
> + if (annotate_opts.code_with_type) {
> + evsel__get_arch(apd->evsel, &apd->arch);
> + apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
> + }
> +
> list_for_each_entry(al, ¬es->src->source, node) {
> if (annotation_line__filter(al))
> continue;
> @@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> wops.first_line = false;
> }
>
> + if (annotate_opts.code_with_type)
> + debuginfo__delete(apd->dbg);
But here it would be good to nullify apd->dbg?
> +
> return 0;
> }
>
> @@ -1743,7 +1751,7 @@ static double annotation_line__max_percent(struct annotation_line *al,
> return percent_max;
> }
>
> -static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> +static int disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> void *obj, char *bf, size_t size,
> void (*obj__printf)(void *obj, const char *fmt, ...),
> void (*obj__write_graph)(void *obj, int graph))
> @@ -1771,8 +1779,8 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> obj__printf(obj, " ");
> }
>
> - disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
> - notes->src->widths.max_ins_name);
> + return disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
> + notes->src->widths.max_ins_name);
> }
>
> static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
> @@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>
> width -= printed + 3;
>
> - disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
> + printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
> + obj__printf, obj__write_graph);
> +
> + obj__printf(obj, "%s", bf);
> + width -= printed;
> +
> + if (annotate_opts.code_with_type && apd->dbg) {
> + struct annotated_data_type *data_type;
> + int offset = 0;
> +
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> + apd->dbg,
> + disasm_line(al),
> + &offset);
> + if (data_type && data_type != NO_TYPE) {
> + char member[256];
> +
> + printed = scnprintf(bf, sizeof(bf),
> + "\t\t# data-type: %s",
> + data_type->self.type_name);
>
> - obj__printf(obj, "%-*s", width, bf);
> + if (data_type != &stackop_type &&
> + data_type != &canary_type &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " +%#x", offset);
> + }
> +
> + if (annotated_data_type__get_member_name(data_type,
> + member,
> + sizeof(member),
> + offset) &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " (%s)", member);
> + }
>
> - (void)apd;
> + obj__printf(obj, "%-*s", width, bf);
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> }
>
> }
> --
> 2.50.0
>
On Wed, Jul 23, 2025 at 01:04:11PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Jul 15, 2025 at 10:00:51PM -0700, Namhyung Kim wrote:
> > Until now, the --code-with-type option is available only on stdio.
> > But it was an artifical limitation because of an implemention issue.
> >
> > Implement the same logic in annotation_line__write() for stdio2/TUI.
> > Make disasm_line__write() return the number of printed characters so
> > that it can skip unnecessary operations when the screen is full.
> >
> > Remove the limitation and update the man page.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/Documentation/perf-annotate.txt | 1 -
> > tools/perf/builtin-annotate.c | 5 --
> > tools/perf/ui/browsers/annotate.c | 6 +++
> > tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> > 4 files changed, 61 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> > index 46090c5b42b4762f..547f1a2680185e3c 100644
> > --- a/tools/perf/Documentation/perf-annotate.txt
> > +++ b/tools/perf/Documentation/perf-annotate.txt
> > @@ -170,7 +170,6 @@ include::itrace.txt[]
> >
> > --code-with-type::
> > Show data type info in code annotation (for memory instructions only).
> > - Currently it only works with --stdio option.
> >
> >
> > SEE ALSO
> > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > index 9833c2c82a2fee46..6debd725392db4a4 100644
> > --- a/tools/perf/builtin-annotate.c
> > +++ b/tools/perf/builtin-annotate.c
> > @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> > symbol_conf.annotate_data_sample = true;
> > } else if (annotate_opts.code_with_type) {
> > symbol_conf.annotate_data_member = true;
> > -
> > - if (!annotate.use_stdio) {
> > - pr_err("--code-with-type only works with --stdio.\n");
> > - goto out_delete;
> > - }
> > }
> >
> > setup_browser(true);
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 23bea5b165774ae7..cdee1969f3131a7c 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -4,6 +4,7 @@
> > #include "../ui.h"
> > #include "../../util/annotate.h"
> > #include "../../util/debug.h"
> > +#include "../../util/debuginfo.h"
> > #include "../../util/dso.h"
> > #include "../../util/hist.h"
> > #include "../../util/sort.h"
> > @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >
> > ui_helpline__push("Press ESC to exit");
> >
> > + if (annotate_opts.code_with_type)
> > + browser.dbg = debuginfo__new(dso__long_name(dso));
>
> Some error checking here to tell the user if debuginfo isn't available
> and hints on how to get it in place?
Ok, you did it later with:
commit 81d638a1b96ec04d7c41e163b5077419cf85e798 (HEAD)
Author: Namhyung Kim <namhyung@kernel.org>
Date: Tue Jul 15 22:00:53 2025 -0700
perf annotate: Show warning when debuginfo is not available
When user requests data-type annotation but no DWARF info is available,
show a warning message about it.
- Arnaldo
On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Until now, the --code-with-type option is available only on stdio.
> But it was an artifical limitation because of an implemention issue.
>
> Implement the same logic in annotation_line__write() for stdio2/TUI.
> Make disasm_line__write() return the number of printed characters so
> that it can skip unnecessary operations when the screen is full.
>
> Remove the limitation and update the man page.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-annotate.txt | 1 -
> tools/perf/builtin-annotate.c | 5 --
> tools/perf/ui/browsers/annotate.c | 6 +++
> tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> 4 files changed, 61 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index 46090c5b42b4762f..547f1a2680185e3c 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -170,7 +170,6 @@ include::itrace.txt[]
>
> --code-with-type::
> Show data type info in code annotation (for memory instructions only).
> - Currently it only works with --stdio option.
>
>
> SEE ALSO
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 9833c2c82a2fee46..6debd725392db4a4 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> symbol_conf.annotate_data_sample = true;
> } else if (annotate_opts.code_with_type) {
> symbol_conf.annotate_data_member = true;
> -
> - if (!annotate.use_stdio) {
> - pr_err("--code-with-type only works with --stdio.\n");
> - goto out_delete;
> - }
> }
>
> setup_browser(true);
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 23bea5b165774ae7..cdee1969f3131a7c 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -4,6 +4,7 @@
> #include "../ui.h"
> #include "../../util/annotate.h"
> #include "../../util/debug.h"
> +#include "../../util/debuginfo.h"
> #include "../../util/dso.h"
> #include "../../util/hist.h"
> #include "../../util/sort.h"
> @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>
> ui_helpline__push("Press ESC to exit");
>
> + if (annotate_opts.code_with_type)
> + browser.dbg = debuginfo__new(dso__long_name(dso));
> +
> browser.b.width = notes->src->widths.max_line_len;
> browser.b.nr_entries = notes->src->nr_entries;
> browser.b.entries = ¬es->src->source;
> @@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> + if (annotate_opts.code_with_type)
> + debuginfo__delete(browser.dbg);
> if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index d69e406c1bc289cd..06ddc7a9f58722a4 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> };
> struct annotation_line *al;
>
> + if (annotate_opts.code_with_type) {
> + evsel__get_arch(apd->evsel, &apd->arch);
> + apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
This API looks unfortunate. A dso may have a long name (it'd be easier
to understand if this were called path rather than long name) or a
build ID. The API isn't the fault of this change, but I thought I'd
mention as we move toward greater use of build IDs.
Thanks,
Ian
> + }
> +
> list_for_each_entry(al, ¬es->src->source, node) {
> if (annotation_line__filter(al))
> continue;
> @@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> wops.first_line = false;
> }
>
> + if (annotate_opts.code_with_type)
> + debuginfo__delete(apd->dbg);
> +
> return 0;
> }
>
> @@ -1743,7 +1751,7 @@ static double annotation_line__max_percent(struct annotation_line *al,
> return percent_max;
> }
>
> -static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> +static int disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> void *obj, char *bf, size_t size,
> void (*obj__printf)(void *obj, const char *fmt, ...),
> void (*obj__write_graph)(void *obj, int graph))
> @@ -1771,8 +1779,8 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> obj__printf(obj, " ");
> }
>
> - disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
> - notes->src->widths.max_ins_name);
> + return disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
> + notes->src->widths.max_ins_name);
> }
>
> static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
> @@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>
> width -= printed + 3;
>
> - disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
> + printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
> + obj__printf, obj__write_graph);
> +
> + obj__printf(obj, "%s", bf);
> + width -= printed;
> +
> + if (annotate_opts.code_with_type && apd->dbg) {
> + struct annotated_data_type *data_type;
> + int offset = 0;
> +
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> + apd->dbg,
> + disasm_line(al),
> + &offset);
> + if (data_type && data_type != NO_TYPE) {
> + char member[256];
> +
> + printed = scnprintf(bf, sizeof(bf),
> + "\t\t# data-type: %s",
> + data_type->self.type_name);
>
> - obj__printf(obj, "%-*s", width, bf);
> + if (data_type != &stackop_type &&
> + data_type != &canary_type &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " +%#x", offset);
> + }
> +
> + if (annotated_data_type__get_member_name(data_type,
> + member,
> + sizeof(member),
> + offset) &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " (%s)", member);
> + }
>
> - (void)apd;
> + obj__printf(obj, "%-*s", width, bf);
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> }
>
> }
> --
> 2.50.0
>
On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Until now, the --code-with-type option is available only on stdio.
> > But it was an artifical limitation because of an implemention issue.
> >
> > Implement the same logic in annotation_line__write() for stdio2/TUI.
> > Make disasm_line__write() return the number of printed characters so
> > that it can skip unnecessary operations when the screen is full.
> >
> > Remove the limitation and update the man page.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/Documentation/perf-annotate.txt | 1 -
> > tools/perf/builtin-annotate.c | 5 --
> > tools/perf/ui/browsers/annotate.c | 6 +++
> > tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> > 4 files changed, 61 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> > index 46090c5b42b4762f..547f1a2680185e3c 100644
> > --- a/tools/perf/Documentation/perf-annotate.txt
> > +++ b/tools/perf/Documentation/perf-annotate.txt
> > @@ -170,7 +170,6 @@ include::itrace.txt[]
> >
> > --code-with-type::
> > Show data type info in code annotation (for memory instructions only).
> > - Currently it only works with --stdio option.
> >
> >
> > SEE ALSO
> > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > index 9833c2c82a2fee46..6debd725392db4a4 100644
> > --- a/tools/perf/builtin-annotate.c
> > +++ b/tools/perf/builtin-annotate.c
> > @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> > symbol_conf.annotate_data_sample = true;
> > } else if (annotate_opts.code_with_type) {
> > symbol_conf.annotate_data_member = true;
> > -
> > - if (!annotate.use_stdio) {
> > - pr_err("--code-with-type only works with --stdio.\n");
> > - goto out_delete;
> > - }
> > }
> >
> > setup_browser(true);
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 23bea5b165774ae7..cdee1969f3131a7c 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -4,6 +4,7 @@
> > #include "../ui.h"
> > #include "../../util/annotate.h"
> > #include "../../util/debug.h"
> > +#include "../../util/debuginfo.h"
> > #include "../../util/dso.h"
> > #include "../../util/hist.h"
> > #include "../../util/sort.h"
> > @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >
> > ui_helpline__push("Press ESC to exit");
> >
> > + if (annotate_opts.code_with_type)
> > + browser.dbg = debuginfo__new(dso__long_name(dso));
> > +
> > browser.b.width = notes->src->widths.max_line_len;
> > browser.b.nr_entries = notes->src->nr_entries;
> > browser.b.entries = ¬es->src->source;
> > @@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >
> > ret = annotate_browser__run(&browser, evsel, hbt);
> >
> > + if (annotate_opts.code_with_type)
> > + debuginfo__delete(browser.dbg);
> > if (not_annotated && !notes->src->tried_source)
> > annotated_source__purge(notes->src);
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index d69e406c1bc289cd..06ddc7a9f58722a4 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> > };
> > struct annotation_line *al;
> >
> > + if (annotate_opts.code_with_type) {
> > + evsel__get_arch(apd->evsel, &apd->arch);
> > + apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
>
> This API looks unfortunate. A dso may have a long name (it'd be easier
> to understand if this were called path rather than long name) or a
> build ID. The API isn't the fault of this change, but I thought I'd
> mention as we move toward greater use of build IDs.
Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
major/minor + inode) and the long name should be available always.
Thanks,
Namhyung
On Wed, Jul 16, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> > On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Until now, the --code-with-type option is available only on stdio.
> > > But it was an artifical limitation because of an implemention issue.
> > >
> > > Implement the same logic in annotation_line__write() for stdio2/TUI.
> > > Make disasm_line__write() return the number of printed characters so
> > > that it can skip unnecessary operations when the screen is full.
> > >
> > > Remove the limitation and update the man page.
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > tools/perf/Documentation/perf-annotate.txt | 1 -
> > > tools/perf/builtin-annotate.c | 5 --
> > > tools/perf/ui/browsers/annotate.c | 6 +++
> > > tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> > > 4 files changed, 61 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> > > index 46090c5b42b4762f..547f1a2680185e3c 100644
> > > --- a/tools/perf/Documentation/perf-annotate.txt
> > > +++ b/tools/perf/Documentation/perf-annotate.txt
> > > @@ -170,7 +170,6 @@ include::itrace.txt[]
> > >
> > > --code-with-type::
> > > Show data type info in code annotation (for memory instructions only).
> > > - Currently it only works with --stdio option.
> > >
> > >
> > > SEE ALSO
> > > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > > index 9833c2c82a2fee46..6debd725392db4a4 100644
> > > --- a/tools/perf/builtin-annotate.c
> > > +++ b/tools/perf/builtin-annotate.c
> > > @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> > > symbol_conf.annotate_data_sample = true;
> > > } else if (annotate_opts.code_with_type) {
> > > symbol_conf.annotate_data_member = true;
> > > -
> > > - if (!annotate.use_stdio) {
> > > - pr_err("--code-with-type only works with --stdio.\n");
> > > - goto out_delete;
> > > - }
> > > }
> > >
> > > setup_browser(true);
> > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > index 23bea5b165774ae7..cdee1969f3131a7c 100644
> > > --- a/tools/perf/ui/browsers/annotate.c
> > > +++ b/tools/perf/ui/browsers/annotate.c
> > > @@ -4,6 +4,7 @@
> > > #include "../ui.h"
> > > #include "../../util/annotate.h"
> > > #include "../../util/debug.h"
> > > +#include "../../util/debuginfo.h"
> > > #include "../../util/dso.h"
> > > #include "../../util/hist.h"
> > > #include "../../util/sort.h"
> > > @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > >
> > > ui_helpline__push("Press ESC to exit");
> > >
> > > + if (annotate_opts.code_with_type)
> > > + browser.dbg = debuginfo__new(dso__long_name(dso));
> > > +
> > > browser.b.width = notes->src->widths.max_line_len;
> > > browser.b.nr_entries = notes->src->nr_entries;
> > > browser.b.entries = ¬es->src->source;
> > > @@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > >
> > > ret = annotate_browser__run(&browser, evsel, hbt);
> > >
> > > + if (annotate_opts.code_with_type)
> > > + debuginfo__delete(browser.dbg);
> > > if (not_annotated && !notes->src->tried_source)
> > > annotated_source__purge(notes->src);
> > >
> > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > index d69e406c1bc289cd..06ddc7a9f58722a4 100644
> > > --- a/tools/perf/util/annotate.c
> > > +++ b/tools/perf/util/annotate.c
> > > @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> > > };
> > > struct annotation_line *al;
> > >
> > > + if (annotate_opts.code_with_type) {
> > > + evsel__get_arch(apd->evsel, &apd->arch);
> > > + apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
> >
> > This API looks unfortunate. A dso may have a long name (it'd be easier
> > to understand if this were called path rather than long name) or a
> > build ID. The API isn't the fault of this change, but I thought I'd
> > mention as we move toward greater use of build IDs.
>
> Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
> major/minor + inode) and the long name should be available always.
I'm thinking of work I'm doing with build IDs like (unmerged):
https://lore.kernel.org/lkml/20250628045017.1361563-1-irogers@google.com/
Even with mmap2 events the filename shouldn't be necessary as the
build ID should be preferred - if you profile remotely there may be a
file name/path collision on a local machine, but it should be unlikely
for a build ID collision. In those patches the dso_id is changed to
instead of considering inode numbers using build IDs when possible. It
was already the case that the name of a dso could be changed, which
affects sorting. In general I think we should move away from file
paths, inodes and the like as the build IDs will avoid races, work
across systems, etc. I think in this case we could add:
```
apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map))))
```
or possibly just change `apd->dbg` to be the dso and grab the
debuginfo from the dso when needed. For now the dso__debuginfo would
be something like:
```
struct debuginfo *dso__debuginfo(struct dso *dso)
{
debuginfo__delete(dso->debuginfo);
dso->debuginfo = debuginfo__new(dso__long_name(dso));
return dso->debuginfo;
}
```
but in the future we can find the debuginfo off of the build ID and paths, etc.
Thanks,
Ian
> Thanks,
> Namhyung
>
On Wed, Jul 16, 2025 at 01:42:13PM -0700, Ian Rogers wrote:
> On Wed, Jul 16, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> > > On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > Until now, the --code-with-type option is available only on stdio.
> > > > But it was an artifical limitation because of an implemention issue.
> > > >
> > > > Implement the same logic in annotation_line__write() for stdio2/TUI.
> > > > Make disasm_line__write() return the number of printed characters so
> > > > that it can skip unnecessary operations when the screen is full.
> > > >
> > > > Remove the limitation and update the man page.
> > > >
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > tools/perf/Documentation/perf-annotate.txt | 1 -
> > > > tools/perf/builtin-annotate.c | 5 --
> > > > tools/perf/ui/browsers/annotate.c | 6 +++
> > > > tools/perf/util/annotate.c | 61 +++++++++++++++++++---
> > > > 4 files changed, 61 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> > > > index 46090c5b42b4762f..547f1a2680185e3c 100644
> > > > --- a/tools/perf/Documentation/perf-annotate.txt
> > > > +++ b/tools/perf/Documentation/perf-annotate.txt
> > > > @@ -170,7 +170,6 @@ include::itrace.txt[]
> > > >
> > > > --code-with-type::
> > > > Show data type info in code annotation (for memory instructions only).
> > > > - Currently it only works with --stdio option.
> > > >
> > > >
> > > > SEE ALSO
> > > > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > > > index 9833c2c82a2fee46..6debd725392db4a4 100644
> > > > --- a/tools/perf/builtin-annotate.c
> > > > +++ b/tools/perf/builtin-annotate.c
> > > > @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
> > > > symbol_conf.annotate_data_sample = true;
> > > > } else if (annotate_opts.code_with_type) {
> > > > symbol_conf.annotate_data_member = true;
> > > > -
> > > > - if (!annotate.use_stdio) {
> > > > - pr_err("--code-with-type only works with --stdio.\n");
> > > > - goto out_delete;
> > > > - }
> > > > }
> > > >
> > > > setup_browser(true);
> > > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > > index 23bea5b165774ae7..cdee1969f3131a7c 100644
> > > > --- a/tools/perf/ui/browsers/annotate.c
> > > > +++ b/tools/perf/ui/browsers/annotate.c
> > > > @@ -4,6 +4,7 @@
> > > > #include "../ui.h"
> > > > #include "../../util/annotate.h"
> > > > #include "../../util/debug.h"
> > > > +#include "../../util/debuginfo.h"
> > > > #include "../../util/dso.h"
> > > > #include "../../util/hist.h"
> > > > #include "../../util/sort.h"
> > > > @@ -1101,6 +1102,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > > >
> > > > ui_helpline__push("Press ESC to exit");
> > > >
> > > > + if (annotate_opts.code_with_type)
> > > > + browser.dbg = debuginfo__new(dso__long_name(dso));
> > > > +
> > > > browser.b.width = notes->src->widths.max_line_len;
> > > > browser.b.nr_entries = notes->src->nr_entries;
> > > > browser.b.entries = ¬es->src->source;
> > > > @@ -1111,6 +1115,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > > >
> > > > ret = annotate_browser__run(&browser, evsel, hbt);
> > > >
> > > > + if (annotate_opts.code_with_type)
> > > > + debuginfo__delete(browser.dbg);
> > > > if (not_annotated && !notes->src->tried_source)
> > > > annotated_source__purge(notes->src);
> > > >
> > > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > > index d69e406c1bc289cd..06ddc7a9f58722a4 100644
> > > > --- a/tools/perf/util/annotate.c
> > > > +++ b/tools/perf/util/annotate.c
> > > > @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> > > > };
> > > > struct annotation_line *al;
> > > >
> > > > + if (annotate_opts.code_with_type) {
> > > > + evsel__get_arch(apd->evsel, &apd->arch);
> > > > + apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
> > >
> > > This API looks unfortunate. A dso may have a long name (it'd be easier
> > > to understand if this were called path rather than long name) or a
> > > build ID. The API isn't the fault of this change, but I thought I'd
> > > mention as we move toward greater use of build IDs.
> >
> > Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
> > major/minor + inode) and the long name should be available always.
>
> I'm thinking of work I'm doing with build IDs like (unmerged):
> https://lore.kernel.org/lkml/20250628045017.1361563-1-irogers@google.com/
> Even with mmap2 events the filename shouldn't be necessary as the
> build ID should be preferred - if you profile remotely there may be a
> file name/path collision on a local machine, but it should be unlikely
> for a build ID collision. In those patches the dso_id is changed to
> instead of considering inode numbers using build IDs when possible. It
> was already the case that the name of a dso could be changed, which
> affects sorting. In general I think we should move away from file
> paths, inodes and the like as the build IDs will avoid races, work
> across systems, etc. I think in this case we could add:
> ```
> apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map))))
> ```
> or possibly just change `apd->dbg` to be the dso and grab the
> debuginfo from the dso when needed. For now the dso__debuginfo would
> be something like:
> ```
> struct debuginfo *dso__debuginfo(struct dso *dso)
> {
> debuginfo__delete(dso->debuginfo);
> dso->debuginfo = debuginfo__new(dso__long_name(dso));
> return dso->debuginfo;
> }
> ```
> but in the future we can find the debuginfo off of the build ID and paths, etc.
Looks good, will add this change on top.
Thanks,
Namhyung
On Wed, Jul 16, 2025 at 01:42:13PM -0700, Ian Rogers wrote: > affects sorting. In general I think we should move away from file > paths, inodes and the like as the build IDs will avoid races, work > across systems, etc. I think in this case we could add: <SNIP> > but in the future we can find the debuginfo off of the build ID and paths, etc. Yeah, agreed, we better move to make build-id the primary means of getting what is needed, when that isn't possible, fall back to filenames. Then we should check if the hostname in the perf.data header is the same as the machine running to warn the user about that, other options are to check the time the perf.data file was created to warn that its likely updates took place, so take the results with a grain of salt, etc. If the architecture or distro is different, things we can also see from the perf.data files, outright refuse to use filenames :-) - Arnaldo
© 2016 - 2025 Red Hat, Inc.