Re: [perf top] annotation doesn't work, libunwind doesn't seem to be working either

[Arnaldo Carvalho de Melo <acme@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6425 bytes --]

On Mon, Apr 07, 2025 at 06:58:28PM +0200, Ingo Molnar wrote:
> * Howard Chu <howardchu95@gmail.com> wrote:
 
> > > I tried to install the libunwind-19-dev package, I tried to
> > > uninstall/reinstall - no combination seems to work.
> > 
> >  $ apt list --installed | grep libunwind
> > 
> > WARNING: apt does not have a stable CLI interface. Use with caution in scripts.
> > 
> > libunwind-dev/oracular,now 1.6.2-3.1 amd64 [installed]
> > libunwind8/oracular,now 1.6.2-3.1 amd64 [installed,automatic]
> > 
> > Built perf with make clean install, my libunwind is on.
> > 
> > ...                               libunwind: [ on  ]
> 
> I seem to have the exact same build environment:
> 
>   starship:~> apt list --installed | grep libunwind
> 
>   WARNING: apt does not have a stable CLI interface. Use with caution in scripts.
> 
>   libunwind-dev/oracular,now 1.6.2-3.1 amd64 [installed]
>   libunwind8/oracular,now 1.6.2-3.1 amd64 [installed]
> 
> Yet:
> 
>   Makefile.config:1142: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel
>   Makefile.config:1155: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
>   Makefile.config:1187: libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
> 
>   Auto-detecting system features:
> ...                                   libdw: [ on  ]
> ...                                   glibc: [ on  ]
> ...                                  libbfd: [ on  ]
> ...                          libbfd-buildid: [ on  ]
> ...                                  libelf: [ on  ]
> ...                                 libnuma: [ on  ]
> ...                  numa_num_possible_cpus: [ on  ]
> ...                                 libperl: [ on  ]
> ...                               libpython: [ on  ]
> ...                               libcrypto: [ on  ]
> ...                               libunwind: [ OFF ]
> ...                             libcapstone: [ on  ]
> ...                               llvm-perf: [ on  ]
> ...                                    zlib: [ on  ]
> ...                                    lzma: [ on  ]
> ...                               get_cpuid: [ on  ]
> ...                                     bpf: [ on  ]
> ...                                  libaio: [ on  ]
> ...                                 libzstd: [ on  ]
 
> > But when I ran perf check,
 
> >  $ perf check feature libunwind
> >              libunwind: [ OFF ]  # HAVE_LIBUNWIND_SUPPORT

> > libunwind is OFF.
 
> Same here.

So, with a patch I have (attached) here this turns into:

⬢ [acme@toolbox perf-tools-next]$ perf check feature libunwind
             libunwind: [ OFF ]  # HAVE_LIBUNWIND_SUPPORT ( tip: Deprecated, use LIBUNWIND=1 and install libunwind-dev[el] to build with it )
⬢ [acme@toolbox perf-tools-next]$

Then I follow the suggestion and install libunwind-devel and build with
LIBUNWIND=1:

$ make -k LIBUNWIND=1 O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin
<SNIP>
Auto-detecting system features:
...                                   libdw: [ on  ]
...                                   glibc: [ on  ]
...                                  libbfd: [ on  ]
...                          libbfd-buildid: [ on  ]
...                                  libelf: [ on  ]
...                                 libnuma: [ on  ]
...                  numa_num_possible_cpus: [ on  ]
...                                 libperl: [ on  ]
...                               libpython: [ on  ]
...                               libcrypto: [ on  ]
...                               libunwind: [ on  ]
...                             libcapstone: [ on  ]
...                               llvm-perf: [ on  ]
...                                    zlib: [ on  ]
...                                    lzma: [ on  ]
...                               get_cpuid: [ on  ]
...                                     bpf: [ on  ]
...                                  libaio: [ on  ]
...                                 libzstd: [ on  ]

⬢ [acme@toolbox perf-tools-next]$ ldd ~/bin/perf | grep unwind
	libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f434eaea000)
	libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f434ead0000)
⬢ [acme@toolbox perf-tools-next]$

I need to have that hooked up to tools/build/Makefile.feature, where it
would find that tip somewhere, i.e. that "Deprecated, use LIBUNWIND=1
and install libunwind-dev[el] to build with it".

This all would not happen if when libunwind was turned into an opt-in
the default output for it had been removed, so that OFF thing wouldn't
appear on the screen.

The annotate case is unrelated from what I can see, the preference for
the disassembler (libcapstone, libllvm or the old method of parsing
objdump output) has to start with the most capable, and the source code
part shows that the current order isn't the best, I'll continue the
investigation/tests tomorrow and will come up with a series of patches
for the problems reported.

For reference, from 'perf-config' man page:

annotate.*::
        These are in control of addresses, jump function, source code
        in lines of assembly code from a specific program.

        annotate.disassemblers::
                Choose the disassembler to use: "objdump", "llvm",  "capstone",
                if not specified it will first try, if available, the "llvm" one,
                then, if it fails, "capstone", and finally the original "objdump"
                based one.

                Choosing a different one is useful when handling some feature that
                is known to be best support at some point by one of the options,
                to compare the output when in doubt about some bug, etc.

                This can be a list, in order of preference, the first one that works
                finishes the process.

A quick test here with perf top without setting the above ends up with
the behaviour that Ingo reported, no source code is shown and 's'
doesn't work. Then if I set it to use objdump:

root@number:~# perf config annotate.disassemblers=objdump
root@number:~# perf config annotate.disassemblers
annotate.disassemblers=objdump
root@number:~# cat ~/.perfconfig 
# this file is auto-generated.
[annotate]
	disassemblers = objdump
root@number:~#

It takes longer to show the annotated output but there is source code
and 's' works.

- Arnaldo

[-- Attachment #2: tip-perf-version-build-options.patch --]
[-- Type: text/plain, Size: 4235 bytes --]

diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
index 61a11a9b4e7594bf..e2cfff18de299367 100644
--- a/tools/perf/builtin-check.c
+++ b/tools/perf/builtin-check.c
@@ -43,7 +43,7 @@ struct feature_status supported_features[] = {
 	FEATURE_STATUS("libpython", HAVE_LIBPYTHON_SUPPORT),
 	FEATURE_STATUS("libslang", HAVE_SLANG_SUPPORT),
 	FEATURE_STATUS("libtraceevent", HAVE_LIBTRACEEVENT),
-	FEATURE_STATUS("libunwind", HAVE_LIBUNWIND_SUPPORT),
+	FEATURE_STATUS_TIP("libunwind", HAVE_LIBUNWIND_SUPPORT, "Deprecated, use LIBUNWIND=1 and install libunwind-dev[el] to build with it"),
 	FEATURE_STATUS("lzma", HAVE_LZMA_SUPPORT),
 	FEATURE_STATUS("numa_num_possible_cpus", HAVE_LIBNUMA_SUPPORT),
 	FEATURE_STATUS("zlib", HAVE_ZLIB_SUPPORT),
@@ -65,22 +65,17 @@ static void on_off_print(const char *status)
 	printf(" ]");
 }
 
-/* Helper function to print status of a feature along with name/macro */
-static void status_print(const char *name, const char *macro,
-			 const char *status)
+void feature_status__printf(const struct feature_status *feature)
 {
-	printf("%22s: ", name);
-	on_off_print(status);
-	printf("  # %s\n", macro);
-}
+	printf("%22s: ", feature->name);
+	on_off_print(feature->is_builtin ? "on" : "OFF");
+	printf("  # %s", feature->macro);
+
+	if (!feature->is_builtin && feature->tip)
+		printf(" ( tip: %s )", feature->tip);
 
-#define STATUS(feature)                                           \
-do {                                                              \
-	if (feature.is_builtin)                                   \
-		status_print(feature.name, feature.macro, "on");  \
-	else                                                      \
-		status_print(feature.name, feature.macro, "OFF"); \
-} while (0)
+	putchar('\n');
+}
 
 /**
  * check whether "feature" is built-in with perf
@@ -95,7 +90,7 @@ static int has_support(const char *feature)
 		if ((strcasecmp(feature, supported_features[i].name) == 0) ||
 		    (strcasecmp(feature, supported_features[i].macro) == 0)) {
 			if (!quiet)
-				STATUS(supported_features[i]);
+				feature_status__printf(&supported_features[i]);
 			return supported_features[i].is_builtin;
 		}
 	}
diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index e149d96c6dc57c22..10f25c6705b117a2 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -26,38 +26,10 @@ static const char * const version_usage[] = {
 	NULL
 };
 
-static void on_off_print(const char *status)
-{
-	printf("[ ");
-
-	if (!strcmp(status, "OFF"))
-		color_fprintf(stdout, PERF_COLOR_RED, "%-3s", status);
-	else
-		color_fprintf(stdout, PERF_COLOR_GREEN, "%-3s", status);
-
-	printf(" ]");
-}
-
-static void status_print(const char *name, const char *macro,
-			 const char *status)
-{
-	printf("%22s: ", name);
-	on_off_print(status);
-	printf("  # %s\n", macro);
-}
-
-#define STATUS(feature)                                   \
-do {                                                      \
-	if (feature.is_builtin)                               \
-		status_print(feature.name, feature.macro, "on");  \
-	else                                                  \
-		status_print(feature.name, feature.macro, "OFF"); \
-} while (0)
-
 static void library_status(void)
 {
 	for (int i = 0; supported_features[i].name; ++i)
-		STATUS(supported_features[i]);
+		feature_status__printf(&supported_features[i]);
 }
 
 int cmd_version(int argc, const char **argv)
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index a07e93c5384878d3..51f8f1f26813fb32 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -5,6 +5,7 @@
 struct feature_status {
 	const char *name;
 	const char *macro;
+	const char *tip;
 	int is_builtin;
 };
 
@@ -13,7 +14,16 @@ struct feature_status {
 	.macro = #macro_,                  \
 	.is_builtin = IS_BUILTIN(macro_) }
 
+#define FEATURE_STATUS_TIP(name_, macro_, tip_) { \
+	.name = name_,                     \
+	.macro = #macro_,                  \
+	.tip = tip_,			   \
+	.is_builtin = IS_BUILTIN(macro_) }
+
 extern struct feature_status supported_features[];
+
+void feature_status__printf(const struct feature_status *feature);
+
 struct cmdnames;
 
 void list_common_cmds_help(void);