linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 1/3] perf report: Support LLVM for addr2line()
@ 2024-07-19 15:00 Steinar H. Gunderson
  2024-07-19 15:00 ` [PATCH v9 2/3] perf annotate: split out read_symbol() Steinar H. Gunderson
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-07-19 15:00 UTC (permalink / raw)
  To: acme
  Cc: linux-perf-users, linux-kernel, irogers, Steinar H. Gunderson,
	Arnaldo Carvalho de Melo

In addition to the existing support for libbfd and calling out to
an external addr2line command, add support for using libllvm directly.
This is both faster than libbfd, and can be enabled in distro builds
(the LLVM license has an explicit provision for GPLv2 compatibility).
Thus, it is set as the primary choice if available.

As an example, running perf report on a medium-size profile with
DWARF-based backtraces took 58 seconds with LLVM, 78 seconds with
libbfd, 153 seconds with external llvm-addr2line, and I got tired
and aborted the test after waiting for 55 minutes with external
bfd addr2line (which is the default for perf as compiled by distributions
today). Evidently, for this case, the bfd addr2line process needs
18 seconds (on a 5.2 GHz Zen 3) to load the .debug ELF in question,
hits the 1-second timeout and gets killed during initialization,
getting restarted anew every time. Having an in-process addr2line
makes this much more robust.

As future extensions, libllvm can be used in many other places where
we currently use libbfd or other libraries:

 - Symbol enumeration (in particular, for PE binaries).
 - Demangling (including non-Itanium demangling, e.g. Microsoft
   or Rust).
 - Disassembling (perf annotate).

However, these are much less pressing; most people don't profile
PE binaries, and perf has non-bfd paths for ELF. The same with
demangling; the default _cxa_demangle path works fine for most
users, and while bfd objdump can be slow on large binaries,
it is possible to use --objdump=llvm-objdump to get the speed benefits.
(It appears LLVM-based demangling is very simple, should we want
that.)

Tested with LLVM 14, 15, 16, 18 and 19. For some reason, LLVM 12 was not
correctly detected using feature_check, and thus was not tested.

Signed-off-by: Steinar H. Gunderson <sesse@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/build/Makefile.feature       |   1 +
 tools/perf/Makefile.config         |  17 ++++
 tools/perf/builtin-version.c       |   1 +
 tools/perf/tests/make              |   2 +
 tools/perf/util/Build              |   1 +
 tools/perf/util/llvm-c-helpers.cpp | 134 +++++++++++++++++++++++++++++
 tools/perf/util/llvm-c-helpers.h   |  49 +++++++++++
 tools/perf/util/srcline.c          |  58 ++++++++++++-
 8 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/util/llvm-c-helpers.cpp
 create mode 100644 tools/perf/util/llvm-c-helpers.h

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 1e2ab148d5db..278b26216254 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -136,6 +136,7 @@ FEATURE_DISPLAY ?=              \
          libunwind              \
          libdw-dwarf-unwind     \
          libcapstone            \
+         llvm                   \
          zlib                   \
          lzma                   \
          get_cpuid              \
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a4829b6532d8..7825832737f1 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -972,6 +972,23 @@ ifdef BUILD_NONDISTRO
   endif
 endif
 
+ifndef NO_LIBLLVM
+  $(call feature_check,llvm)
+  ifeq ($(feature-llvm), 1)
+    CFLAGS += -DHAVE_LIBLLVM_SUPPORT
+    CFLAGS += $(shell $(LLVM_CONFIG) --cflags)
+    CXXFLAGS += -DHAVE_LIBLLVM_SUPPORT
+    CXXFLAGS += $(shell $(LLVM_CONFIG) --cxxflags)
+    LIBLLVM = $(shell $(LLVM_CONFIG) --libs all) $(shell $(LLVM_CONFIG) --system-libs)
+    EXTLIBS += -L$(shell $(LLVM_CONFIG) --libdir) $(LIBLLVM)
+    EXTLIBS += -lstdc++
+    $(call detected,CONFIG_LIBLLVM)
+  else
+    $(warning No libllvm found, slower source file resolution, please install llvm-devel/llvm-dev)
+    NO_LIBLLVM := 1
+  endif
+endif
+
 ifndef NO_DEMANGLE
   $(call feature_check,cxa-demangle)
   ifeq ($(feature-cxa-demangle), 1)
diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index 398aa53e9e2e..4b252196de12 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -65,6 +65,7 @@ static void library_status(void)
 	STATUS(HAVE_LIBBFD_SUPPORT, libbfd);
 	STATUS(HAVE_DEBUGINFOD_SUPPORT, debuginfod);
 	STATUS(HAVE_LIBELF_SUPPORT, libelf);
+	STATUS(HAVE_LIBLLVM_SUPPORT, libllvm);
 	STATUS(HAVE_LIBNUMA_SUPPORT, libnuma);
 	STATUS(HAVE_LIBNUMA_SUPPORT, numa_num_possible_cpus);
 	STATUS(HAVE_LIBPERL_SUPPORT, libperl);
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index a1f8adf85367..3df8b030eaa3 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -92,6 +92,7 @@ make_no_libbpf	    := NO_LIBBPF=1
 make_libbpf_dynamic := LIBBPF_DYNAMIC=1
 make_no_libbpf_DEBUG := NO_LIBBPF=1 DEBUG=1
 make_no_libcrypto   := NO_LIBCRYPTO=1
+make_no_libllvm     := NO_LIBLLVM=1
 make_with_babeltrace:= LIBBABELTRACE=1
 make_with_coresight := CORESIGHT=1
 make_no_sdt	    := NO_SDT=1
@@ -161,6 +162,7 @@ run += make_no_auxtrace
 run += make_no_libbpf
 run += make_no_libbpf_DEBUG
 run += make_no_libcrypto
+run += make_no_libllvm
 run += make_no_sdt
 run += make_no_syscall_tbl
 run += make_with_babeltrace
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 0f18fe81ef0b..cfb64706ffe7 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -226,6 +226,7 @@ perf-util-$(CONFIG_CXX_DEMANGLE) += demangle-cxx.o
 perf-util-y += demangle-ocaml.o
 perf-util-y += demangle-java.o
 perf-util-y += demangle-rust.o
+perf-util-$(CONFIG_LIBLLVM) += llvm-c-helpers.o
 
 ifdef CONFIG_JITDUMP
 perf-util-$(CONFIG_LIBELF) += jitdump.o
diff --git a/tools/perf/util/llvm-c-helpers.cpp b/tools/perf/util/llvm-c-helpers.cpp
new file mode 100644
index 000000000000..3cc967ec6f28
--- /dev/null
+++ b/tools/perf/util/llvm-c-helpers.cpp
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Must come before the linux/compiler.h include, which defines several
+ * macros (e.g. noinline) that conflict with compiler builtins used
+ * by LLVM.
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"  /* Needed for LLVM <= 15 */
+#include <llvm/DebugInfo/Symbolize/Symbolize.h>
+#pragma GCC diagnostic pop
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <linux/compiler.h>
+extern "C" {
+#include <linux/zalloc.h>
+}
+#include "symbol_conf.h"
+#include "llvm-c-helpers.h"
+
+using namespace llvm;
+using llvm::symbolize::LLVMSymbolizer;
+
+/*
+ * Allocate a static LLVMSymbolizer, which will live to the end of the program.
+ * Unlike the bfd paths, LLVMSymbolizer has its own cache, so we do not need
+ * to store anything in the dso struct.
+ */
+static LLVMSymbolizer *get_symbolizer()
+{
+	static LLVMSymbolizer *instance = nullptr;
+	if (instance == nullptr) {
+		LLVMSymbolizer::Options opts;
+		/*
+		 * LLVM sometimes demangles slightly different from the rest
+		 * of the code, and this mismatch can cause new_inline_sym()
+		 * to get confused and mark non-inline symbol as inlined
+		 * (since the name does not properly match up with base_sym).
+		 * Thus, disable the demangling and let the rest of the code
+		 * handle it.
+		 */
+		opts.Demangle = false;
+		instance = new LLVMSymbolizer(opts);
+	}
+	return instance;
+}
+
+/* Returns 0 on error, 1 on success. */
+static int extract_file_and_line(const DILineInfo &line_info, char **file,
+				 unsigned int *line)
+{
+	if (file) {
+		if (line_info.FileName == "<invalid>") {
+			/* Match the convention of libbfd. */
+			*file = nullptr;
+		} else {
+			/* The caller expects to get something it can free(). */
+			*file = strdup(line_info.FileName.c_str());
+			if (*file == nullptr)
+				return 0;
+		}
+	}
+	if (line)
+		*line = line_info.Line;
+	return 1;
+}
+
+extern "C"
+int llvm_addr2line(const char *dso_name, u64 addr,
+		   char **file, unsigned int *line,
+		   bool unwind_inlines,
+		   llvm_a2l_frame **inline_frames)
+{
+	LLVMSymbolizer *symbolizer = get_symbolizer();
+	object::SectionedAddress sectioned_addr = {
+		addr,
+		object::SectionedAddress::UndefSection
+	};
+
+	if (unwind_inlines) {
+		Expected<DIInliningInfo> res_or_err =
+			symbolizer->symbolizeInlinedCode(dso_name,
+							 sectioned_addr);
+		if (!res_or_err)
+			return 0;
+		unsigned num_frames = res_or_err->getNumberOfFrames();
+		if (num_frames == 0)
+			return 0;
+
+		if (extract_file_and_line(res_or_err->getFrame(0),
+					  file, line) == 0)
+			return 0;
+
+		*inline_frames = (llvm_a2l_frame *)calloc(
+			num_frames, sizeof(**inline_frames));
+		if (*inline_frames == nullptr)
+			return 0;
+
+		for (unsigned i = 0; i < num_frames; ++i) {
+			const DILineInfo &src = res_or_err->getFrame(i);
+
+			llvm_a2l_frame &dst = (*inline_frames)[i];
+			if (src.FileName == "<invalid>")
+				/* Match the convention of libbfd. */
+				dst.filename = nullptr;
+			else
+				dst.filename = strdup(src.FileName.c_str());
+			dst.funcname = strdup(src.FunctionName.c_str());
+			dst.line = src.Line;
+
+			if (dst.filename == nullptr ||
+			    dst.funcname == nullptr) {
+				for (unsigned j = 0; j <= i; ++j) {
+					zfree(&(*inline_frames)[j].filename);
+					zfree(&(*inline_frames)[j].funcname);
+				}
+				zfree(inline_frames);
+				return 0;
+			}
+		}
+
+		return num_frames;
+	} else {
+		if (inline_frames)
+			*inline_frames = nullptr;
+
+		Expected<DILineInfo> res_or_err =
+			symbolizer->symbolizeCode(dso_name, sectioned_addr);
+		if (!res_or_err)
+			return 0;
+		return extract_file_and_line(*res_or_err, file, line);
+	}
+}
diff --git a/tools/perf/util/llvm-c-helpers.h b/tools/perf/util/llvm-c-helpers.h
new file mode 100644
index 000000000000..19332dd98e14
--- /dev/null
+++ b/tools/perf/util/llvm-c-helpers.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_LLVM_C_HELPERS
+#define __PERF_LLVM_C_HELPERS 1
+
+/*
+ * Helpers to call into LLVM C++ code from C, for the parts that do not have
+ * C APIs.
+ */
+
+#include <linux/compiler.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct llvm_a2l_frame {
+  char* filename;
+  char* funcname;
+  unsigned int line;
+};
+
+/*
+ * Implement addr2line() using libLLVM. LLVM is a C++ API, and
+ * many of the linux/ headers cannot be included in a C++ compile unit,
+ * so we need to make a little bridge code here. llvm_addr2line() will
+ * convert the inline frame information from LLVM's internal structures
+ * and put them into a flat array given in inline_frames. The caller
+ * is then responsible for taking that array and convert it into perf's
+ * regular inline frame structures (which depend on e.g. struct list_head).
+ *
+ * If the address could not be resolved, or an error occurred (e.g. OOM),
+ * returns 0. Otherwise, returns the number of inline frames (which means 1
+ * if the address was not part of an inlined function). If unwind_inlines
+ * is set and the return code is nonzero, inline_frames will be set to
+ * a newly allocated array with that length. The caller is then responsible
+ * for freeing both the strings and the array itself.
+ */
+int llvm_addr2line(const char* dso_name,
+                   u64 addr,
+                   char** file,
+                   unsigned int* line,
+                   bool unwind_inlines,
+                   struct llvm_a2l_frame** inline_frames);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __PERF_LLVM_C_HELPERS */
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 760742fd4a7d..2e3845ac07ee 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -16,6 +16,9 @@
 #include "util/debug.h"
 #include "util/callchain.h"
 #include "util/symbol_conf.h"
+#ifdef HAVE_LIBLLVM_SUPPORT
+#include "util/llvm-c-helpers.h"
+#endif
 #include "srcline.h"
 #include "string2.h"
 #include "symbol.h"
@@ -130,7 +133,60 @@ static struct symbol *new_inline_sym(struct dso *dso,
 
 #define MAX_INLINE_NEST 1024
 
-#ifdef HAVE_LIBBFD_SUPPORT
+#ifdef HAVE_LIBLLVM_SUPPORT
+
+static void free_llvm_inline_frames(struct llvm_a2l_frame *inline_frames,
+				    int num_frames)
+{
+	if (inline_frames != NULL) {
+		for (int i = 0; i < num_frames; ++i) {
+			zfree(&inline_frames[i].filename);
+			zfree(&inline_frames[i].funcname);
+		}
+		zfree(&inline_frames);
+	}
+}
+
+static int addr2line(const char *dso_name, u64 addr,
+		     char **file, unsigned int *line, struct dso *dso,
+		     bool unwind_inlines, struct inline_node *node,
+		     struct symbol *sym)
+{
+	struct llvm_a2l_frame *inline_frames = NULL;
+	int num_frames = llvm_addr2line(dso_name, addr, file, line,
+					node && unwind_inlines, &inline_frames);
+
+	if (num_frames == 0 || !inline_frames) {
+		/* Error, or we didn't want inlines. */
+		return num_frames;
+	}
+
+	for (int i = 0; i < num_frames; ++i) {
+		struct symbol *inline_sym =
+			new_inline_sym(dso, sym, inline_frames[i].funcname);
+		char *srcline = NULL;
+
+		if (inline_frames[i].filename) {
+			srcline =
+				srcline_from_fileline(inline_frames[i].filename,
+						      inline_frames[i].line);
+		}
+		if (inline_list__append(inline_sym, srcline, node) != 0) {
+			free_llvm_inline_frames(inline_frames, num_frames);
+			return 0;
+		}
+	}
+	free_llvm_inline_frames(inline_frames, num_frames);
+
+	return num_frames;
+}
+
+void dso__free_a2l(struct dso *)
+{
+	/* Nothing to free. */
+}
+
+#elif defined(HAVE_LIBBFD_SUPPORT)
 
 /*
  * Implement addr2line using libbfd.
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v9 2/3] perf annotate: split out read_symbol()
  2024-07-19 15:00 [PATCH v9 1/3] perf report: Support LLVM for addr2line() Steinar H. Gunderson
@ 2024-07-19 15:00 ` Steinar H. Gunderson
  2024-07-19 15:00 ` [PATCH v9 3/3] perf annotate: LLVM-based disassembler Steinar H. Gunderson
  2024-07-30 19:42 ` [PATCH v9 1/3] perf report: Support LLVM for addr2line() Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-07-19 15:00 UTC (permalink / raw)
  To: acme; +Cc: linux-perf-users, linux-kernel, irogers, Steinar H. Gunderson

The Capstone disassembler code has a useful code snippet to read
the bytes for a given code symbol into memory. Split it out into
its own function, so that the LLVM disassembler can use it in
the next patch.

Signed-off-by: Steinar H. Gunderson <sesse@google.com>
---
 tools/perf/util/disasm.c | 90 +++++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 34 deletions(-)

diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index e10558b79504..9be2becfe6e8 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -1396,6 +1396,53 @@ static int find_file_offset(u64 start, u64 len, u64 pgoff, void *arg)
 	return 0;
 }
 
+static u8 *
+read_symbol(const char *filename, struct map *map, struct symbol *sym,
+	    u64 *len, bool *is_64bit)
+{
+	struct dso *dso = map__dso(map);
+	struct nscookie nsc;
+	u64 start = map__rip_2objdump(map, sym->start);
+	u64 end = map__rip_2objdump(map, sym->end);
+	int fd, count;
+	u8 *buf = NULL;
+	struct find_file_offset_data data = {
+		.ip = start,
+	};
+
+	*is_64bit = false;
+
+	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
+	fd = open(filename, O_RDONLY);
+	nsinfo__mountns_exit(&nsc);
+	if (fd < 0)
+		return NULL;
+
+	if (file__read_maps(fd, /*exe=*/true, find_file_offset, &data,
+			    is_64bit) == 0)
+		goto err;
+
+	*len = end - start;
+	buf = malloc(*len);
+	if (buf == NULL)
+		goto err;
+
+	count = pread(fd, buf, *len, data.offset);
+	close(fd);
+	fd = -1;
+
+	if ((u64)count != *len)
+		goto err;
+
+	return buf;
+
+err:
+	if (fd >= 0)
+		close(fd);
+	free(buf);
+	return NULL;
+}
+
 static void print_capstone_detail(cs_insn *insn, char *buf, size_t len,
 				  struct annotate_args *args, u64 addr)
 {
@@ -1458,19 +1505,13 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 {
 	struct annotation *notes = symbol__annotation(sym);
 	struct map *map = args->ms.map;
-	struct dso *dso = map__dso(map);
-	struct nscookie nsc;
 	u64 start = map__rip_2objdump(map, sym->start);
-	u64 end = map__rip_2objdump(map, sym->end);
-	u64 len = end - start;
+	u64 len;
 	u64 offset;
-	int i, fd, count;
+	int i, count;
 	bool is_64bit = false;
 	bool needs_cs_close = false;
 	u8 *buf = NULL;
-	struct find_file_offset_data data = {
-		.ip = start,
-	};
 	csh handle;
 	cs_insn *insn;
 	char disasm_buf[512];
@@ -1479,31 +1520,9 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 	if (args->options->objdump_path)
 		return -1;
 
-	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
-	fd = open(filename, O_RDONLY);
-	nsinfo__mountns_exit(&nsc);
-	if (fd < 0)
-		return -1;
-
-	if (file__read_maps(fd, /*exe=*/true, find_file_offset, &data,
-			    &is_64bit) == 0)
-		goto err;
-
-	if (open_capstone_handle(args, is_64bit, &handle) < 0)
-		goto err;
-
-	needs_cs_close = true;
-
-	buf = malloc(len);
+	buf = read_symbol(filename, map, sym, &len, &is_64bit);
 	if (buf == NULL)
-		goto err;
-
-	count = pread(fd, buf, len, data.offset);
-	close(fd);
-	fd = -1;
-
-	if ((u64)count != len)
-		goto err;
+		return -1;
 
 	/* add the function address and name */
 	scnprintf(disasm_buf, sizeof(disasm_buf), "%#"PRIx64" <%s>:",
@@ -1521,6 +1540,11 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 
 	annotation_line__add(&dl->al, &notes->src->source);
 
+	if (open_capstone_handle(args, is_64bit, &handle) < 0)
+		goto err;
+
+	needs_cs_close = true;
+
 	count = cs_disasm(handle, buf, len, start, len, &insn);
 	for (i = 0, offset = 0; i < count; i++) {
 		int printed;
@@ -1565,8 +1589,6 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 	return count < 0 ? count : 0;
 
 err:
-	if (fd >= 0)
-		close(fd);
 	if (needs_cs_close) {
 		struct disasm_line *tmp;
 
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v9 3/3] perf annotate: LLVM-based disassembler
  2024-07-19 15:00 [PATCH v9 1/3] perf report: Support LLVM for addr2line() Steinar H. Gunderson
  2024-07-19 15:00 ` [PATCH v9 2/3] perf annotate: split out read_symbol() Steinar H. Gunderson
@ 2024-07-19 15:00 ` Steinar H. Gunderson
  2024-09-02 14:53   ` Arnaldo Carvalho de Melo
  2024-07-30 19:42 ` [PATCH v9 1/3] perf report: Support LLVM for addr2line() Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-07-19 15:00 UTC (permalink / raw)
  To: acme; +Cc: linux-perf-users, linux-kernel, irogers, Steinar H. Gunderson

Support using LLVM as a disassembler method, allowing helperless
annotation in non-distro builds. (It is also much faster than
using libbfd or bfd objdump on binaries with a lot of debug
information.)

This is nearly identical to the output of llvm-objdump; there are
some very rare whitespace differences, some minor changes to demangling
(since we use perf's regular demangling and not LLVM's own) and
the occasional case where llvm-objdump makes a different choice
when multiple symbols share the same address. It should work across
all of LLVM's supported architectures, although I've only tested 64-bit
x86, and finding the right triple from perf's idea of machine
architecture can sometimes be a bit tricky. Ideally, we should have
some way of finding the triplet just from the file itself.

Signed-off-by: Steinar H. Gunderson <sesse@google.com>
---
 tools/perf/util/disasm.c           | 193 +++++++++++++++++++++++++++++
 tools/perf/util/llvm-c-helpers.cpp |  62 +++++++++
 tools/perf/util/llvm-c-helpers.h   |  11 ++
 3 files changed, 266 insertions(+)

diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 9be2becfe6e8..f0232ce3d751 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -43,6 +43,7 @@ static int call__scnprintf(struct ins *ins, char *bf, size_t size,
 
 static void ins__sort(struct arch *arch);
 static int disasm_line__parse(char *line, const char **namep, char **rawp);
+static char *expand_tabs(char *line, char **storage, size_t *storage_len);
 
 static __attribute__((constructor)) void symbol__init_regexpr(void)
 {
@@ -1378,7 +1379,9 @@ static int open_capstone_handle(struct annotate_args *args, bool is_64bit,
 
 	return 0;
 }
+#endif
 
+#if defined(HAVE_LIBCAPSTONE_SUPPORT) || defined(HAVE_LIBLLVM_SUPPORT)
 struct find_file_offset_data {
 	u64 ip;
 	u64 offset;
@@ -1442,7 +1445,9 @@ read_symbol(const char *filename, struct map *map, struct symbol *sym,
 	free(buf);
 	return NULL;
 }
+#endif
 
+#ifdef HAVE_LIBCAPSTONE_SUPPORT
 static void print_capstone_detail(cs_insn *insn, char *buf, size_t len,
 				  struct annotate_args *args, u64 addr)
 {
@@ -1606,6 +1611,189 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
 }
 #endif
 
+#ifdef HAVE_LIBLLVM_SUPPORT
+#include <llvm-c/Disassembler.h>
+#include <llvm-c/Target.h>
+#include "util/llvm-c-helpers.h"
+
+struct symbol_lookup_storage {
+	u64 branch_addr;
+	u64 pcrel_load_addr;
+};
+
+/*
+ * Whenever LLVM wants to resolve an address into a symbol, it calls this
+ * callback. We don't ever actually _return_ anything (in particular, because
+ * it puts quotation marks around what we return), but we use this as a hint
+ * that there is a branch or PC-relative address in the expression that we
+ * should add some textual annotation for after the instruction. The caller
+ * will use this information to add the actual annotation.
+ */
+static const char *
+symbol_lookup_callback(void *disinfo, uint64_t value,
+		       uint64_t *ref_type,
+		       uint64_t address __maybe_unused,
+		       const char **ref __maybe_unused)
+{
+	struct symbol_lookup_storage *storage = disinfo;
+
+	if (*ref_type == LLVMDisassembler_ReferenceType_In_Branch)
+		storage->branch_addr = value;
+	else if (*ref_type == LLVMDisassembler_ReferenceType_In_PCrel_Load)
+		storage->pcrel_load_addr = value;
+	*ref_type = LLVMDisassembler_ReferenceType_InOut_None;
+	return NULL;
+}
+
+static int symbol__disassemble_llvm(char *filename, struct symbol *sym,
+				    struct annotate_args *args)
+{
+	struct annotation *notes = symbol__annotation(sym);
+	struct map *map = args->ms.map;
+	struct dso *dso = map__dso(map);
+	u64 start = map__rip_2objdump(map, sym->start);
+	u8 *buf;
+	u64 len;
+	u64 pc;
+	bool is_64bit;
+	char triplet[64];
+	char disasm_buf[2048];
+	size_t disasm_len;
+	struct disasm_line *dl;
+	LLVMDisasmContextRef disasm = NULL;
+	struct symbol_lookup_storage storage;
+	char *line_storage = NULL;
+	size_t line_storage_len = 0;
+	int ret = -1;
+
+	if (args->options->objdump_path)
+		return -1;
+
+	LLVMInitializeAllTargetInfos();
+	LLVMInitializeAllTargetMCs();
+	LLVMInitializeAllDisassemblers();
+
+	buf = read_symbol(filename, map, sym, &len, &is_64bit);
+	if (buf == NULL)
+		return -1;
+
+	if (arch__is(args->arch, "x86")) {
+		if (is_64bit)
+			scnprintf(triplet, sizeof(triplet), "x86_64-pc-linux");
+		else
+			scnprintf(triplet, sizeof(triplet), "i686-pc-linux");
+	} else {
+		scnprintf(triplet, sizeof(triplet), "%s-linux-gnu",
+			  args->arch->name);
+	}
+
+	disasm = LLVMCreateDisasm(triplet, &storage, 0, NULL,
+				  symbol_lookup_callback);
+	if (disasm == NULL)
+		goto err;
+
+	if (args->options->disassembler_style &&
+	    !strcmp(args->options->disassembler_style, "intel"))
+		LLVMSetDisasmOptions(disasm,
+				     LLVMDisassembler_Option_AsmPrinterVariant);
+
+	/*
+	 * This needs to be set after AsmPrinterVariant, due to a bug in LLVM;
+	 * setting AsmPrinterVariant makes a new instruction printer, making it
+	 * forget about the PrintImmHex flag (which is applied before if both
+	 * are given to the same call).
+	 */
+	LLVMSetDisasmOptions(disasm, LLVMDisassembler_Option_PrintImmHex);
+
+	/* add the function address and name */
+	scnprintf(disasm_buf, sizeof(disasm_buf), "%#"PRIx64" <%s>:",
+		  start, sym->name);
+
+	args->offset = -1;
+	args->line = disasm_buf;
+	args->line_nr = 0;
+	args->fileloc = NULL;
+	args->ms.sym = sym;
+
+	dl = disasm_line__new(args);
+	if (dl == NULL)
+		goto err;
+
+	annotation_line__add(&dl->al, &notes->src->source);
+
+	pc = start;
+	for (u64 offset = 0; offset < len; ) {
+		unsigned int ins_len;
+
+		storage.branch_addr = 0;
+		storage.pcrel_load_addr = 0;
+
+		ins_len = LLVMDisasmInstruction(disasm, buf + offset,
+						len - offset, pc,
+						disasm_buf, sizeof(disasm_buf));
+		if (ins_len == 0)
+			goto err;
+		disasm_len = strlen(disasm_buf);
+
+		if (storage.branch_addr != 0) {
+			char *name = llvm_name_for_code(dso, filename,
+							storage.branch_addr);
+			if (name != NULL) {
+				disasm_len += scnprintf(disasm_buf + disasm_len,
+							sizeof(disasm_buf) -
+								disasm_len,
+							" <%s>", name);
+				free(name);
+			}
+		}
+		if (storage.pcrel_load_addr != 0) {
+			char *name = llvm_name_for_data(dso, filename,
+							storage.pcrel_load_addr);
+			disasm_len += scnprintf(disasm_buf + disasm_len,
+						sizeof(disasm_buf) - disasm_len,
+						"  # %#"PRIx64,
+						storage.pcrel_load_addr);
+			if (name) {
+				disasm_len += scnprintf(disasm_buf + disasm_len,
+							sizeof(disasm_buf) -
+							disasm_len,
+							" <%s>", name);
+				free(name);
+			}
+		}
+
+		args->offset = offset;
+		args->line = expand_tabs(disasm_buf, &line_storage,
+					 &line_storage_len);
+		args->line_nr = 0;
+		args->fileloc = NULL;
+		args->ms.sym = sym;
+
+		llvm_addr2line(filename, pc, &args->fileloc,
+			       (unsigned int *)&args->line_nr, false, NULL);
+
+		dl = disasm_line__new(args);
+		if (dl == NULL)
+			goto err;
+
+		annotation_line__add(&dl->al, &notes->src->source);
+
+		free(args->fileloc);
+		pc += ins_len;
+		offset += ins_len;
+	}
+
+	ret = 0;
+
+err:
+	LLVMDisasmDispose(disasm);
+	free(buf);
+	free(line_storage);
+	return ret;
+}
+#endif
+
+
 /*
  * Possibly create a new version of line with tabs expanded. Returns the
  * existing or new line, storage is updated if a new line is allocated. If
@@ -1730,6 +1918,11 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
 		strcpy(symfs_filename, tmp);
 	}
 
+#ifdef HAVE_LIBLLVM_SUPPORT
+	err = symbol__disassemble_llvm(symfs_filename, sym, args);
+	if (err == 0)
+		goto out_remove_tmp;
+#endif
 #ifdef HAVE_LIBCAPSTONE_SUPPORT
 	err = symbol__disassemble_capstone(symfs_filename, sym, args);
 	if (err == 0)
diff --git a/tools/perf/util/llvm-c-helpers.cpp b/tools/perf/util/llvm-c-helpers.cpp
index 3cc967ec6f28..4070e2d5682f 100644
--- a/tools/perf/util/llvm-c-helpers.cpp
+++ b/tools/perf/util/llvm-c-helpers.cpp
@@ -8,6 +8,7 @@
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-parameter"  /* Needed for LLVM <= 15 */
 #include <llvm/DebugInfo/Symbolize/Symbolize.h>
+#include <llvm/Support/TargetSelect.h>
 #pragma GCC diagnostic pop
 
 #include <stdio.h>
@@ -19,6 +20,9 @@ extern "C" {
 #include "symbol_conf.h"
 #include "llvm-c-helpers.h"
 
+extern "C"
+char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
+
 using namespace llvm;
 using llvm::symbolize::LLVMSymbolizer;
 
@@ -132,3 +136,61 @@ int llvm_addr2line(const char *dso_name, u64 addr,
 		return extract_file_and_line(*res_or_err, file, line);
 	}
 }
+
+static char *
+make_symbol_relative_string(struct dso *dso, const char *sym_name,
+			    u64 addr, u64 base_addr)
+{
+	if (!strcmp(sym_name, "<invalid>"))
+		return NULL;
+
+	char *demangled = dso__demangle_sym(dso, 0, sym_name);
+	if (base_addr && base_addr != addr) {
+		char buf[256];
+		snprintf(buf, sizeof(buf), "%s+0x%lx",
+			 demangled ? demangled : sym_name, addr - base_addr);
+		free(demangled);
+		return strdup(buf);
+	} else {
+		if (demangled)
+			return demangled;
+		else
+			return strdup(sym_name);
+	}
+}
+
+extern "C"
+char *llvm_name_for_code(struct dso *dso, const char *dso_name, u64 addr)
+{
+	LLVMSymbolizer *symbolizer = get_symbolizer();
+	object::SectionedAddress sectioned_addr = {
+		addr,
+		object::SectionedAddress::UndefSection
+	};
+	Expected<DILineInfo> res_or_err =
+		symbolizer->symbolizeCode(dso_name, sectioned_addr);
+	if (!res_or_err) {
+		return NULL;
+	}
+	return make_symbol_relative_string(
+		dso, res_or_err->FunctionName.c_str(),
+		addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
+}
+
+extern "C"
+char *llvm_name_for_data(struct dso *dso, const char *dso_name, u64 addr)
+{
+	LLVMSymbolizer *symbolizer = get_symbolizer();
+	object::SectionedAddress sectioned_addr = {
+		addr,
+		object::SectionedAddress::UndefSection
+	};
+	Expected<DIGlobal> res_or_err =
+		symbolizer->symbolizeData(dso_name, sectioned_addr);
+	if (!res_or_err) {
+		return NULL;
+	}
+	return make_symbol_relative_string(
+		dso, res_or_err->Name.c_str(),
+		addr, res_or_err->Start);
+}
diff --git a/tools/perf/util/llvm-c-helpers.h b/tools/perf/util/llvm-c-helpers.h
index 19332dd98e14..d2b99637a28a 100644
--- a/tools/perf/util/llvm-c-helpers.h
+++ b/tools/perf/util/llvm-c-helpers.h
@@ -13,6 +13,8 @@
 extern "C" {
 #endif
 
+struct dso;
+
 struct llvm_a2l_frame {
   char* filename;
   char* funcname;
@@ -42,6 +44,15 @@ int llvm_addr2line(const char* dso_name,
                    bool unwind_inlines,
                    struct llvm_a2l_frame** inline_frames);
 
+/*
+ * Simple symbolizers for addresses; will convert something like
+ * 0x12345 to "func+0x123". Will return NULL if no symbol was found.
+ *
+ * The returned value must be freed by the caller, with free().
+ */
+char *llvm_name_for_code(struct dso *dso, const char *dso_name, u64 addr);
+char *llvm_name_for_data(struct dso *dso, const char *dso_name, u64 addr);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-07-19 15:00 [PATCH v9 1/3] perf report: Support LLVM for addr2line() Steinar H. Gunderson
  2024-07-19 15:00 ` [PATCH v9 2/3] perf annotate: split out read_symbol() Steinar H. Gunderson
  2024-07-19 15:00 ` [PATCH v9 3/3] perf annotate: LLVM-based disassembler Steinar H. Gunderson
@ 2024-07-30 19:42 ` Arnaldo Carvalho de Melo
  2024-07-30 19:49   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-07-30 19:42 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Fri, Jul 19, 2024 at 05:00:49PM +0200, Steinar H. Gunderson wrote:
> In addition to the existing support for libbfd and calling out to
> an external addr2line command, add support for using libllvm directly.
> This is both faster than libbfd, and can be enabled in distro builds
> (the LLVM license has an explicit provision for GPLv2 compatibility).
> Thus, it is set as the primary choice if available.
> 
> As an example, running perf report on a medium-size profile with
> DWARF-based backtraces took 58 seconds with LLVM, 78 seconds with
> libbfd, 153 seconds with external llvm-addr2line, and I got tired
> and aborted the test after waiting for 55 minutes with external
> bfd addr2line (which is the default for perf as compiled by distributions
> today). Evidently, for this case, the bfd addr2line process needs
> 18 seconds (on a 5.2 GHz Zen 3) to load the .debug ELF in question,
> hits the 1-second timeout and gets killed during initialization,
> getting restarted anew every time. Having an in-process addr2line
> makes this much more robust.
> 
> As future extensions, libllvm can be used in many other places where
> we currently use libbfd or other libraries:
> 
>  - Symbol enumeration (in particular, for PE binaries).
>  - Demangling (including non-Itanium demangling, e.g. Microsoft
>    or Rust).
>  - Disassembling (perf annotate).
> 
> However, these are much less pressing; most people don't profile
> PE binaries, and perf has non-bfd paths for ELF. The same with
> demangling; the default _cxa_demangle path works fine for most
> users, and while bfd objdump can be slow on large binaries,
> it is possible to use --objdump=llvm-objdump to get the speed benefits.
> (It appears LLVM-based demangling is very simple, should we want
> that.)
> 
> Tested with LLVM 14, 15, 16, 18 and 19. For some reason, LLVM 12 was not
> correctly detected using feature_check, and thus was not tested.
> 
> Signed-off-by: Steinar H. Gunderson <sesse@google.com>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

I'm testing this again, I thought Ian or Namhyung had made comments on
previous versions of this patchset, no?

- Arnaldo

> ---
>  tools/build/Makefile.feature       |   1 +
>  tools/perf/Makefile.config         |  17 ++++
>  tools/perf/builtin-version.c       |   1 +
>  tools/perf/tests/make              |   2 +
>  tools/perf/util/Build              |   1 +
>  tools/perf/util/llvm-c-helpers.cpp | 134 +++++++++++++++++++++++++++++
>  tools/perf/util/llvm-c-helpers.h   |  49 +++++++++++
>  tools/perf/util/srcline.c          |  58 ++++++++++++-
>  8 files changed, 262 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/util/llvm-c-helpers.cpp
>  create mode 100644 tools/perf/util/llvm-c-helpers.h
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 1e2ab148d5db..278b26216254 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -136,6 +136,7 @@ FEATURE_DISPLAY ?=              \
>           libunwind              \
>           libdw-dwarf-unwind     \
>           libcapstone            \
> +         llvm                   \
>           zlib                   \
>           lzma                   \
>           get_cpuid              \
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a4829b6532d8..7825832737f1 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -972,6 +972,23 @@ ifdef BUILD_NONDISTRO
>    endif
>  endif
>  
> +ifndef NO_LIBLLVM
> +  $(call feature_check,llvm)
> +  ifeq ($(feature-llvm), 1)
> +    CFLAGS += -DHAVE_LIBLLVM_SUPPORT
> +    CFLAGS += $(shell $(LLVM_CONFIG) --cflags)
> +    CXXFLAGS += -DHAVE_LIBLLVM_SUPPORT
> +    CXXFLAGS += $(shell $(LLVM_CONFIG) --cxxflags)
> +    LIBLLVM = $(shell $(LLVM_CONFIG) --libs all) $(shell $(LLVM_CONFIG) --system-libs)
> +    EXTLIBS += -L$(shell $(LLVM_CONFIG) --libdir) $(LIBLLVM)
> +    EXTLIBS += -lstdc++
> +    $(call detected,CONFIG_LIBLLVM)
> +  else
> +    $(warning No libllvm found, slower source file resolution, please install llvm-devel/llvm-dev)
> +    NO_LIBLLVM := 1
> +  endif
> +endif
> +
>  ifndef NO_DEMANGLE
>    $(call feature_check,cxa-demangle)
>    ifeq ($(feature-cxa-demangle), 1)
> diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
> index 398aa53e9e2e..4b252196de12 100644
> --- a/tools/perf/builtin-version.c
> +++ b/tools/perf/builtin-version.c
> @@ -65,6 +65,7 @@ static void library_status(void)
>  	STATUS(HAVE_LIBBFD_SUPPORT, libbfd);
>  	STATUS(HAVE_DEBUGINFOD_SUPPORT, debuginfod);
>  	STATUS(HAVE_LIBELF_SUPPORT, libelf);
> +	STATUS(HAVE_LIBLLVM_SUPPORT, libllvm);
>  	STATUS(HAVE_LIBNUMA_SUPPORT, libnuma);
>  	STATUS(HAVE_LIBNUMA_SUPPORT, numa_num_possible_cpus);
>  	STATUS(HAVE_LIBPERL_SUPPORT, libperl);
> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index a1f8adf85367..3df8b030eaa3 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
> @@ -92,6 +92,7 @@ make_no_libbpf	    := NO_LIBBPF=1
>  make_libbpf_dynamic := LIBBPF_DYNAMIC=1
>  make_no_libbpf_DEBUG := NO_LIBBPF=1 DEBUG=1
>  make_no_libcrypto   := NO_LIBCRYPTO=1
> +make_no_libllvm     := NO_LIBLLVM=1
>  make_with_babeltrace:= LIBBABELTRACE=1
>  make_with_coresight := CORESIGHT=1
>  make_no_sdt	    := NO_SDT=1
> @@ -161,6 +162,7 @@ run += make_no_auxtrace
>  run += make_no_libbpf
>  run += make_no_libbpf_DEBUG
>  run += make_no_libcrypto
> +run += make_no_libllvm
>  run += make_no_sdt
>  run += make_no_syscall_tbl
>  run += make_with_babeltrace
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 0f18fe81ef0b..cfb64706ffe7 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -226,6 +226,7 @@ perf-util-$(CONFIG_CXX_DEMANGLE) += demangle-cxx.o
>  perf-util-y += demangle-ocaml.o
>  perf-util-y += demangle-java.o
>  perf-util-y += demangle-rust.o
> +perf-util-$(CONFIG_LIBLLVM) += llvm-c-helpers.o
>  
>  ifdef CONFIG_JITDUMP
>  perf-util-$(CONFIG_LIBELF) += jitdump.o
> diff --git a/tools/perf/util/llvm-c-helpers.cpp b/tools/perf/util/llvm-c-helpers.cpp
> new file mode 100644
> index 000000000000..3cc967ec6f28
> --- /dev/null
> +++ b/tools/perf/util/llvm-c-helpers.cpp
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Must come before the linux/compiler.h include, which defines several
> + * macros (e.g. noinline) that conflict with compiler builtins used
> + * by LLVM.
> + */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-parameter"  /* Needed for LLVM <= 15 */
> +#include <llvm/DebugInfo/Symbolize/Symbolize.h>
> +#pragma GCC diagnostic pop
> +
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <linux/compiler.h>
> +extern "C" {
> +#include <linux/zalloc.h>
> +}
> +#include "symbol_conf.h"
> +#include "llvm-c-helpers.h"
> +
> +using namespace llvm;
> +using llvm::symbolize::LLVMSymbolizer;
> +
> +/*
> + * Allocate a static LLVMSymbolizer, which will live to the end of the program.
> + * Unlike the bfd paths, LLVMSymbolizer has its own cache, so we do not need
> + * to store anything in the dso struct.
> + */
> +static LLVMSymbolizer *get_symbolizer()
> +{
> +	static LLVMSymbolizer *instance = nullptr;
> +	if (instance == nullptr) {
> +		LLVMSymbolizer::Options opts;
> +		/*
> +		 * LLVM sometimes demangles slightly different from the rest
> +		 * of the code, and this mismatch can cause new_inline_sym()
> +		 * to get confused and mark non-inline symbol as inlined
> +		 * (since the name does not properly match up with base_sym).
> +		 * Thus, disable the demangling and let the rest of the code
> +		 * handle it.
> +		 */
> +		opts.Demangle = false;
> +		instance = new LLVMSymbolizer(opts);
> +	}
> +	return instance;
> +}
> +
> +/* Returns 0 on error, 1 on success. */
> +static int extract_file_and_line(const DILineInfo &line_info, char **file,
> +				 unsigned int *line)
> +{
> +	if (file) {
> +		if (line_info.FileName == "<invalid>") {
> +			/* Match the convention of libbfd. */
> +			*file = nullptr;
> +		} else {
> +			/* The caller expects to get something it can free(). */
> +			*file = strdup(line_info.FileName.c_str());
> +			if (*file == nullptr)
> +				return 0;
> +		}
> +	}
> +	if (line)
> +		*line = line_info.Line;
> +	return 1;
> +}
> +
> +extern "C"
> +int llvm_addr2line(const char *dso_name, u64 addr,
> +		   char **file, unsigned int *line,
> +		   bool unwind_inlines,
> +		   llvm_a2l_frame **inline_frames)
> +{
> +	LLVMSymbolizer *symbolizer = get_symbolizer();
> +	object::SectionedAddress sectioned_addr = {
> +		addr,
> +		object::SectionedAddress::UndefSection
> +	};
> +
> +	if (unwind_inlines) {
> +		Expected<DIInliningInfo> res_or_err =
> +			symbolizer->symbolizeInlinedCode(dso_name,
> +							 sectioned_addr);
> +		if (!res_or_err)
> +			return 0;
> +		unsigned num_frames = res_or_err->getNumberOfFrames();
> +		if (num_frames == 0)
> +			return 0;
> +
> +		if (extract_file_and_line(res_or_err->getFrame(0),
> +					  file, line) == 0)
> +			return 0;
> +
> +		*inline_frames = (llvm_a2l_frame *)calloc(
> +			num_frames, sizeof(**inline_frames));
> +		if (*inline_frames == nullptr)
> +			return 0;
> +
> +		for (unsigned i = 0; i < num_frames; ++i) {
> +			const DILineInfo &src = res_or_err->getFrame(i);
> +
> +			llvm_a2l_frame &dst = (*inline_frames)[i];
> +			if (src.FileName == "<invalid>")
> +				/* Match the convention of libbfd. */
> +				dst.filename = nullptr;
> +			else
> +				dst.filename = strdup(src.FileName.c_str());
> +			dst.funcname = strdup(src.FunctionName.c_str());
> +			dst.line = src.Line;
> +
> +			if (dst.filename == nullptr ||
> +			    dst.funcname == nullptr) {
> +				for (unsigned j = 0; j <= i; ++j) {
> +					zfree(&(*inline_frames)[j].filename);
> +					zfree(&(*inline_frames)[j].funcname);
> +				}
> +				zfree(inline_frames);
> +				return 0;
> +			}
> +		}
> +
> +		return num_frames;
> +	} else {
> +		if (inline_frames)
> +			*inline_frames = nullptr;
> +
> +		Expected<DILineInfo> res_or_err =
> +			symbolizer->symbolizeCode(dso_name, sectioned_addr);
> +		if (!res_or_err)
> +			return 0;
> +		return extract_file_and_line(*res_or_err, file, line);
> +	}
> +}
> diff --git a/tools/perf/util/llvm-c-helpers.h b/tools/perf/util/llvm-c-helpers.h
> new file mode 100644
> index 000000000000..19332dd98e14
> --- /dev/null
> +++ b/tools/perf/util/llvm-c-helpers.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PERF_LLVM_C_HELPERS
> +#define __PERF_LLVM_C_HELPERS 1
> +
> +/*
> + * Helpers to call into LLVM C++ code from C, for the parts that do not have
> + * C APIs.
> + */
> +
> +#include <linux/compiler.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct llvm_a2l_frame {
> +  char* filename;
> +  char* funcname;
> +  unsigned int line;
> +};
> +
> +/*
> + * Implement addr2line() using libLLVM. LLVM is a C++ API, and
> + * many of the linux/ headers cannot be included in a C++ compile unit,
> + * so we need to make a little bridge code here. llvm_addr2line() will
> + * convert the inline frame information from LLVM's internal structures
> + * and put them into a flat array given in inline_frames. The caller
> + * is then responsible for taking that array and convert it into perf's
> + * regular inline frame structures (which depend on e.g. struct list_head).
> + *
> + * If the address could not be resolved, or an error occurred (e.g. OOM),
> + * returns 0. Otherwise, returns the number of inline frames (which means 1
> + * if the address was not part of an inlined function). If unwind_inlines
> + * is set and the return code is nonzero, inline_frames will be set to
> + * a newly allocated array with that length. The caller is then responsible
> + * for freeing both the strings and the array itself.
> + */
> +int llvm_addr2line(const char* dso_name,
> +                   u64 addr,
> +                   char** file,
> +                   unsigned int* line,
> +                   bool unwind_inlines,
> +                   struct llvm_a2l_frame** inline_frames);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __PERF_LLVM_C_HELPERS */
> diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
> index 760742fd4a7d..2e3845ac07ee 100644
> --- a/tools/perf/util/srcline.c
> +++ b/tools/perf/util/srcline.c
> @@ -16,6 +16,9 @@
>  #include "util/debug.h"
>  #include "util/callchain.h"
>  #include "util/symbol_conf.h"
> +#ifdef HAVE_LIBLLVM_SUPPORT
> +#include "util/llvm-c-helpers.h"
> +#endif
>  #include "srcline.h"
>  #include "string2.h"
>  #include "symbol.h"
> @@ -130,7 +133,60 @@ static struct symbol *new_inline_sym(struct dso *dso,
>  
>  #define MAX_INLINE_NEST 1024
>  
> -#ifdef HAVE_LIBBFD_SUPPORT
> +#ifdef HAVE_LIBLLVM_SUPPORT
> +
> +static void free_llvm_inline_frames(struct llvm_a2l_frame *inline_frames,
> +				    int num_frames)
> +{
> +	if (inline_frames != NULL) {
> +		for (int i = 0; i < num_frames; ++i) {
> +			zfree(&inline_frames[i].filename);
> +			zfree(&inline_frames[i].funcname);
> +		}
> +		zfree(&inline_frames);
> +	}
> +}
> +
> +static int addr2line(const char *dso_name, u64 addr,
> +		     char **file, unsigned int *line, struct dso *dso,
> +		     bool unwind_inlines, struct inline_node *node,
> +		     struct symbol *sym)
> +{
> +	struct llvm_a2l_frame *inline_frames = NULL;
> +	int num_frames = llvm_addr2line(dso_name, addr, file, line,
> +					node && unwind_inlines, &inline_frames);
> +
> +	if (num_frames == 0 || !inline_frames) {
> +		/* Error, or we didn't want inlines. */
> +		return num_frames;
> +	}
> +
> +	for (int i = 0; i < num_frames; ++i) {
> +		struct symbol *inline_sym =
> +			new_inline_sym(dso, sym, inline_frames[i].funcname);
> +		char *srcline = NULL;
> +
> +		if (inline_frames[i].filename) {
> +			srcline =
> +				srcline_from_fileline(inline_frames[i].filename,
> +						      inline_frames[i].line);
> +		}
> +		if (inline_list__append(inline_sym, srcline, node) != 0) {
> +			free_llvm_inline_frames(inline_frames, num_frames);
> +			return 0;
> +		}
> +	}
> +	free_llvm_inline_frames(inline_frames, num_frames);
> +
> +	return num_frames;
> +}
> +
> +void dso__free_a2l(struct dso *)
> +{
> +	/* Nothing to free. */
> +}
> +
> +#elif defined(HAVE_LIBBFD_SUPPORT)
>  
>  /*
>   * Implement addr2line using libbfd.
> -- 
> 2.45.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-07-30 19:42 ` [PATCH v9 1/3] perf report: Support LLVM for addr2line() Arnaldo Carvalho de Melo
@ 2024-07-30 19:49   ` Arnaldo Carvalho de Melo
  2024-08-03 15:11     ` Steinar H. Gunderson
  2024-09-01 10:57     ` Steinar H. Gunderson
  0 siblings, 2 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-07-30 19:49 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Jul 30, 2024 at 04:42:28PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jul 19, 2024 at 05:00:49PM +0200, Steinar H. Gunderson wrote:
> > In addition to the existing support for libbfd and calling out to
> > an external addr2line command, add support for using libllvm directly.
> > This is both faster than libbfd, and can be enabled in distro builds
> > (the LLVM license has an explicit provision for GPLv2 compatibility).
> > Thus, it is set as the primary choice if available.
> > 
> > As an example, running perf report on a medium-size profile with
> > DWARF-based backtraces took 58 seconds with LLVM, 78 seconds with
> > libbfd, 153 seconds with external llvm-addr2line, and I got tired
> > and aborted the test after waiting for 55 minutes with external
> > bfd addr2line (which is the default for perf as compiled by distributions
> > today). Evidently, for this case, the bfd addr2line process needs
> > 18 seconds (on a 5.2 GHz Zen 3) to load the .debug ELF in question,
> > hits the 1-second timeout and gets killed during initialization,
> > getting restarted anew every time. Having an in-process addr2line
> > makes this much more robust.
> > 
> > As future extensions, libllvm can be used in many other places where
> > we currently use libbfd or other libraries:
> > 
> >  - Symbol enumeration (in particular, for PE binaries).
> >  - Demangling (including non-Itanium demangling, e.g. Microsoft
> >    or Rust).
> >  - Disassembling (perf annotate).
> > 
> > However, these are much less pressing; most people don't profile
> > PE binaries, and perf has non-bfd paths for ELF. The same with
> > demangling; the default _cxa_demangle path works fine for most
> > users, and while bfd objdump can be slow on large binaries,
> > it is possible to use --objdump=llvm-objdump to get the speed benefits.
> > (It appears LLVM-based demangling is very simple, should we want
> > that.)
> > 
> > Tested with LLVM 14, 15, 16, 18 and 19. For some reason, LLVM 12 was not
> > correctly detected using feature_check, and thus was not tested.
> > 
> > Signed-off-by: Steinar H. Gunderson <sesse@google.com>
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> I'm testing this again, I thought Ian or Namhyung had made comments on
> previous versions of this patchset, no?

Unfortunately it clashed with recent patches in the capstone related
codebase, IIRC Athira's for powerpc data-type profiling.

Please take a look at what is in tmp.perf-tools-next at:

https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git

I'll continue processing other patched and eventually try to fix this if
you're busy,

Thanks again for your continued work on this!

- Arnaldo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-07-30 19:49   ` Arnaldo Carvalho de Melo
@ 2024-08-03 15:11     ` Steinar H. Gunderson
  2024-09-01 10:57     ` Steinar H. Gunderson
  1 sibling, 0 replies; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-08-03 15:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Jul 30, 2024 at 04:49:12PM -0300, Arnaldo Carvalho de Melo wrote:
>> I'm testing this again, I thought Ian or Namhyung had made comments on
>> previous versions of this patchset, no?

Yes, and I believe they have been addressed.

> Unfortunately it clashed with recent patches in the capstone related
> codebase, IIRC Athira's for powerpc data-type profiling.
> 
> Please take a look at what is in tmp.perf-tools-next at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git

I'll have a look and see if I can do a rebase.

/* Steinar */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-07-30 19:49   ` Arnaldo Carvalho de Melo
  2024-08-03 15:11     ` Steinar H. Gunderson
@ 2024-09-01 10:57     ` Steinar H. Gunderson
  2024-09-03 10:09       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-09-01 10:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Jul 30, 2024 at 04:49:12PM -0300, Arnaldo Carvalho de Melo wrote:
> Unfortunately it clashed with recent patches in the capstone related
> codebase, IIRC Athira's for powerpc data-type profiling.
> 
> […]
> 
> I'll continue processing other patched and eventually try to fix this if
> you're busy,

Hi,

Is there any movement in getting v10 merged? :-) I haven't heard back
since I sent out the rebase.

/* Steinar */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 3/3] perf annotate: LLVM-based disassembler
  2024-07-19 15:00 ` [PATCH v9 3/3] perf annotate: LLVM-based disassembler Steinar H. Gunderson
@ 2024-09-02 14:53   ` Arnaldo Carvalho de Melo
  2024-09-02 14:55     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-02 14:53 UTC (permalink / raw)
  To: Steinar H. Gunderson; +Cc: linux-perf-users, linux-kernel, irogers

On Fri, Jul 19, 2024 at 05:00:51PM +0200, Steinar H. Gunderson wrote:
> Support using LLVM as a disassembler method, allowing helperless
> annotation in non-distro builds. (It is also much faster than
> using libbfd or bfd objdump on binaries with a lot of debug
> information.)
> 
> This is nearly identical to the output of llvm-objdump; there are
> some very rare whitespace differences, some minor changes to demangling
> (since we use perf's regular demangling and not LLVM's own) and
> the occasional case where llvm-objdump makes a different choice
> when multiple symbols share the same address. It should work across
> all of LLVM's supported architectures, although I've only tested 64-bit
> x86, and finding the right triple from perf's idea of machine
> architecture can sometimes be a bit tricky. Ideally, we should have
> some way of finding the triplet just from the file itself.

<SNIP>
 
> @@ -1730,6 +1918,11 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
>  		strcpy(symfs_filename, tmp);
>  	}
>  
> +#ifdef HAVE_LIBLLVM_SUPPORT
> +	err = symbol__disassemble_llvm(symfs_filename, sym, args);
> +	if (err == 0)
> +		goto out_remove_tmp;
> +#endif
>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>  	err = symbol__disassemble_capstone(symfs_filename, sym, args);
>  	if (err == 0)

So the above makes it unconditionally be used, and if the user installed
llvm-devel, that now gets checked and suggested at build time, then that
is an indication that it should be used, but I wonder if, for debugging
purposes we shouldn't have this done in some configurable way, i.e. some
~/.perfconfig variable that allows us to try a specific disassembler,
something like:

	perf annotate --disassemble=capstone

I can even envision having some perf test that compares the output for
some well known function to see if they really produce the same output
from different disassemblers, etc.

I'm applying the patches, thanks!

- Arnaldo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 3/3] perf annotate: LLVM-based disassembler
  2024-09-02 14:53   ` Arnaldo Carvalho de Melo
@ 2024-09-02 14:55     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-02 14:55 UTC (permalink / raw)
  To: Namhyung Kim, Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers

Cc'ing Namhyung, see below.

On Mon, Sep 02, 2024 at 11:53:55AM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jul 19, 2024 at 05:00:51PM +0200, Steinar H. Gunderson wrote:
> > Support using LLVM as a disassembler method, allowing helperless
> > annotation in non-distro builds. (It is also much faster than
> > using libbfd or bfd objdump on binaries with a lot of debug
> > information.)
> > 
> > This is nearly identical to the output of llvm-objdump; there are
> > some very rare whitespace differences, some minor changes to demangling
> > (since we use perf's regular demangling and not LLVM's own) and
> > the occasional case where llvm-objdump makes a different choice
> > when multiple symbols share the same address. It should work across
> > all of LLVM's supported architectures, although I've only tested 64-bit
> > x86, and finding the right triple from perf's idea of machine
> > architecture can sometimes be a bit tricky. Ideally, we should have
> > some way of finding the triplet just from the file itself.
> 
> <SNIP>
  
> > @@ -1730,6 +1918,11 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> >  		strcpy(symfs_filename, tmp);
> >  	}
> >  
> > +#ifdef HAVE_LIBLLVM_SUPPORT
> > +	err = symbol__disassemble_llvm(symfs_filename, sym, args);
> > +	if (err == 0)
> > +		goto out_remove_tmp;
> > +#endif
> >  #ifdef HAVE_LIBCAPSTONE_SUPPORT
> >  	err = symbol__disassemble_capstone(symfs_filename, sym, args);
> >  	if (err == 0)
> 
> So the above makes it unconditionally be used, and if the user installed
> llvm-devel, that now gets checked and suggested at build time, then that
> is an indication that it should be used, but I wonder if, for debugging
> purposes we shouldn't have this done in some configurable way, i.e. some
> ~/.perfconfig variable that allows us to try a specific disassembler,
> something like:
> 
> 	perf annotate --disassemble=capstone
> 
> I can even envision having some perf test that compares the output for
> some well known function to see if they really produce the same output
> from different disassemblers, etc.

Namhyung, do you see a problem with this?

- Arnaldo
 
> I'm applying the patches, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-01 10:57     ` Steinar H. Gunderson
@ 2024-09-03 10:09       ` Arnaldo Carvalho de Melo
  2024-09-03 13:14         ` Arnaldo Carvalho de Melo
  2024-09-03 14:01         ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-03 10:09 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Sun, Sep 01, 2024 at 12:57:25PM +0200, Steinar H. Gunderson wrote:
> On Tue, Jul 30, 2024 at 04:49:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > Unfortunately it clashed with recent patches in the capstone related
> > codebase, IIRC Athira's for powerpc data-type profiling.
> > 
> > […]
> > 
> > I'll continue processing other patched and eventually try to fix this if
> > you're busy,
> 
> Hi,
> 
> Is there any movement in getting v10 merged? :-) I haven't heard back
> since I sent out the rebase

Test building is detecting some problems, I'll try to address them:

perfbuilder@number:~$ time dm
   1    13.50 almalinux:8                   : FAIL gcc version 8.5.0 20210514 (Red Hat 8.5.0-22) (GCC) 
    util/srcline.c: In function 'dso__free_a2l':
    util/srcline.c:184:20: error: parameter name omitted
     void dso__free_a2l(struct dso *)
                        ^~~~~~~~~~~~
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
   2    68.46 almalinux:9                   : FAIL clang version 17.0.6 (AlmaLinux OS Foundation 17.0.6-5.el9)
    util/srcline.c:184:32: error: omitting the parameter name in a function definition is a C2x extension [-Werror,-Wc2x-extensions]
      184 | void dso__free_a2l(struct dso *)
          |                                ^
    1 error generated.
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
   3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
    17.58 almalinux:9-i386              : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC) 
    util/llvm-c-helpers.cpp: In function ‘char* make_symbol_relative_string(dso*, const char*, u64, u64)’:
    util/llvm-c-helpers.cpp:150:52: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=]
      150 |                 snprintf(buf, sizeof(buf), "%s+0x%lx",
          |                                                  ~~^
          |                                                    |
          |                                                    long unsigned int
          |                                                  %llx
      151 |                          demangled ? demangled : sym_name, addr - base_addr);
          |                                                            ~~~~~~~~~~~~~~~~
          |                                                                 |
          |                                                                 u64 {aka long long unsigned int}
    cc1plus: all warnings being treated as errors
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
   4    18.08 alpine:3.15                   : FAIL gcc version 10.3.1 20211027 (Alpine 10.3.1_git20211027) 
    util/srcline.c: In function 'dso__free_a2l':
    util/srcline.c:184:20: error: parameter name omitted
      184 | void dso__free_a2l(struct dso *)
          |                    ^~~~~~~~~~~~
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-03 10:09       ` Arnaldo Carvalho de Melo
@ 2024-09-03 13:14         ` Arnaldo Carvalho de Melo
  2024-09-03 14:01         ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-03 13:14 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Sep 03, 2024 at 07:09:47AM -0300, Arnaldo Carvalho de Melo wrote:
> On Sun, Sep 01, 2024 at 12:57:25PM +0200, Steinar H. Gunderson wrote:
> > On Tue, Jul 30, 2024 at 04:49:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Unfortunately it clashed with recent patches in the capstone related
> > > codebase, IIRC Athira's for powerpc data-type profiling.
> > > 
> > > […]
> > > 
> > > I'll continue processing other patched and eventually try to fix this if
> > > you're busy,
> > 
> > Hi,
> > 
> > Is there any movement in getting v10 merged? :-) I haven't heard back
> > since I sent out the rebase
> 
> Test building is detecting some problems, I'll try to address them:
> 
> perfbuilder@number:~$ time dm
>    1    13.50 almalinux:8                   : FAIL gcc version 8.5.0 20210514 (Red Hat 8.5.0-22) (GCC) 
>     util/srcline.c: In function 'dso__free_a2l':
>     util/srcline.c:184:20: error: parameter name omitted
>      void dso__free_a2l(struct dso *)
>                         ^~~~~~~~~~~~
>     make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index 2e3845ac07ee7be6..f32d0d4f4bc9e659 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -6,6 +6,7 @@
 #include <string.h>
 #include <sys/types.h>
 
+#include <linux/compiler.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/zalloc.h>
@@ -181,7 +182,7 @@ static int addr2line(const char *dso_name, u64 addr,
 	return num_frames;
 }
 
-void dso__free_a2l(struct dso *)
+void dso__free_a2l(struct dso *dso __maybe_unused)
 {
 	/* Nothing to free. */
 }

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-03 10:09       ` Arnaldo Carvalho de Melo
  2024-09-03 13:14         ` Arnaldo Carvalho de Melo
@ 2024-09-03 14:01         ` Arnaldo Carvalho de Melo
  2024-09-03 14:15           ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-03 14:01 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Sep 03, 2024 at 07:09:47AM -0300, Arnaldo Carvalho de Melo wrote:
>    3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
>     17.58 almalinux:9-i386              : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC) 
>     util/llvm-c-helpers.cpp: In function ‘char* make_symbol_relative_string(dso*, const char*, u64, u64)’:
>     util/llvm-c-helpers.cpp:150:52: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=]
>       150 |                 snprintf(buf, sizeof(buf), "%s+0x%lx",
>           |                                                  ~~^
>           |                                                    |
>           |                                                    long unsigned int
>           |                                                  %llx
>       151 |                          demangled ? demangled : sym_name, addr - base_addr);
>           |                                                            ~~~~~~~~~~~~~~~~
>           |                                                                 |
>           |                                                                 u64 {aka long long unsigned int}
>     cc1plus: all warnings being treated as errors
>     make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2

The one above is fixed by the patch at the end, that I already added to
the cset where the problem was being introduced.

Now there is something a bit more tricky, we'll have to add a feature
check to see if the libllvm has what is needed if this appears in some
distro we still want to support, since alpine 3.16 has what is needed
I'll take the opportunity to drop test building on alpine 3.15.

FYI, so far:

perfbuilder@number:~$ time dm
   1   101.42 almalinux:8       : Ok   gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-22) , clang version 17.0.6 (Red Hat 17.0.6-1.module_el8.10.0+3757+fc27b834) flex 2.6.1
   2    89.34 almalinux:9       : Ok   gcc (GCC) 11.4.1 20231218 (Red Hat 11.4.1-3) , clang version 17.0.6 (AlmaLinux OS Foundation 17.0.6-5.el9) flex 2.6.4
   3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
   104.86 almalinux:9-i386      : Ok   gcc (GCC) 11.4.1 20231218 (Red Hat 11.4.1-3) , clang version 17.0.6 (AlmaLinux OS Foundation 17.0.6-5.el9) flex 2.6.4
   4    16.20 alpine:3.15                   : FAIL gcc version 10.3.1 20211027 (Alpine 10.3.1_git20211027) 
    util/llvm-c-helpers.cpp: In function 'char* llvm_name_for_code(dso*, const char*, u64)':
    util/llvm-c-helpers.cpp:178:21: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
      178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
          |                     ^~~~~~~~~~~~
    util/llvm-c-helpers.cpp:178:49: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
      178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
          |                                                 ^~~~~~~~~~~~
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
   5   119.65 alpine:3.16       : Ok   gcc (Alpine 11.2.1_git20220219) 11.2.1 20220219 , Alpine clang version 13.0.1 flex 2.6.4
   6    98.62 alpine:3.17       : Ok   gcc (Alpine 12.2.1_git20220924-r4) 12.2.1 20220924 , Alpine clang version 15.0.7 flex 2.6.4
   7   101.00 alpine:3.18       : Ok   gcc (Alpine 12.2.1_git20220924-r10) 12.2.1 20220924 , Alpine clang version 16.0.6 flex 2.6.4
   8   106.74 alpine:3.19       : Ok   gcc (Alpine 13.2.1_git20231014) 13.2.1 20231014 , Alpine clang version 17.0.5 flex 2.6.4
   9   103.22 alpine:3.20       : Ok   gcc (Alpine 13.2.1_git20240309) 13.2.1 20240309 , Alpine clang version 17.0.6 flex 2.6.4
  10   109.79 alpine:edge       : Ok   gcc (Alpine 14.2.0) 14.2.0 , Alpine clang version 18.1.8 flex 2.6.4
  11    87.75 amazonlinux:2023  : Ok   gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2) , clang version 15.0.7 (Amazon Linux 15.0.7-3.amzn2023.0.1) flex 2.6.4

- Arnaldo

diff --git a/tools/perf/util/llvm-c-helpers.cpp b/tools/perf/util/llvm-c-helpers.cpp
index 4070e2d5682fd674..663bcaba2041fc25 100644
--- a/tools/perf/util/llvm-c-helpers.cpp
+++ b/tools/perf/util/llvm-c-helpers.cpp
@@ -11,6 +11,7 @@
 #include <llvm/Support/TargetSelect.h>
 #pragma GCC diagnostic pop
 
+#include <inttypes.h>
 #include <stdio.h>
 #include <sys/types.h>
 #include <linux/compiler.h>
@@ -147,7 +148,7 @@ make_symbol_relative_string(struct dso *dso, const char *sym_name,
 	char *demangled = dso__demangle_sym(dso, 0, sym_name);
 	if (base_addr && base_addr != addr) {
 		char buf[256];
-		snprintf(buf, sizeof(buf), "%s+0x%lx",
+		snprintf(buf, sizeof(buf), "%s+0x%" PRIx64,
 			 demangled ? demangled : sym_name, addr - base_addr);
 		free(demangled);
 		return strdup(buf);

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-03 14:01         ` Arnaldo Carvalho de Melo
@ 2024-09-03 14:15           ` Arnaldo Carvalho de Melo
  2024-09-08 18:52             ` Steinar H. Gunderson
  2024-09-10 14:06             ` James Clark
  0 siblings, 2 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-03 14:15 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Sep 03, 2024 at 11:01:36AM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Sep 03, 2024 at 07:09:47AM -0300, Arnaldo Carvalho de Melo wrote:
> >    3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> > WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
> >     17.58 almalinux:9-i386              : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC) 
> >     util/llvm-c-helpers.cpp: In function ‘char* make_symbol_relative_string(dso*, const char*, u64, u64)’:
> >     util/llvm-c-helpers.cpp:150:52: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=]
> >       150 |                 snprintf(buf, sizeof(buf), "%s+0x%lx",
> >           |                                                  ~~^
> >           |                                                    |
> >           |                                                    long unsigned int
> >           |                                                  %llx
> >       151 |                          demangled ? demangled : sym_name, addr - base_addr);
> >           |                                                            ~~~~~~~~~~~~~~~~
> >           |                                                                 |
> >           |                                                                 u64 {aka long long unsigned int}
> >     cc1plus: all warnings being treated as errors
> >     make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
> 
> The one above is fixed by the patch at the end, that I already added to
> the cset where the problem was being introduced.
> 
> Now there is something a bit more tricky, we'll have to add a feature
> check to see if the libllvm has what is needed if this appears in some
> distro we still want to support, since alpine 3.16 has what is needed
> I'll take the opportunity to drop test building on alpine 3.15.

Or, as I'll do with debian:11, just remove llvm-dev and not build the
features it enables:

  17    13.79 debian:11                     : FAIL gcc version 10.2.1 20210110 (Debian 10.2.1-6) 
    util/llvm-c-helpers.cpp: In function 'char* llvm_name_for_code(dso*, const char*, u64)':
    util/llvm-c-helpers.cpp:178:21: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
      178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
          |                     ^~~~~~~~~~~~
    util/llvm-c-helpers.cpp:178:49: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
      178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
          |                                                 ^~~~~~~~~~~~
    make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-03 14:15           ` Arnaldo Carvalho de Melo
@ 2024-09-08 18:52             ` Steinar H. Gunderson
  2024-09-09 16:42               ` Arnaldo Carvalho de Melo
  2024-09-10 14:06             ` James Clark
  1 sibling, 1 reply; 16+ messages in thread
From: Steinar H. Gunderson @ 2024-09-08 18:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Tue, Sep 03, 2024 at 11:15:39AM -0300, Arnaldo Carvalho de Melo wrote:
> Or, as I'll do with debian:11, just remove llvm-dev and not build the
> features it enables:

Thanks for looking into this! Is there anything I should be doing,
or are the patches you posted earlier sufficient?

/* Steinar */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-08 18:52             ` Steinar H. Gunderson
@ 2024-09-09 16:42               ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-09-09 16:42 UTC (permalink / raw)
  To: Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim

On Sun, Sep 08, 2024 at 08:52:42PM +0200, Steinar H. Gunderson wrote:
> On Tue, Sep 03, 2024 at 11:15:39AM -0300, Arnaldo Carvalho de Melo wrote:
> > Or, as I'll do with debian:11, just remove llvm-dev and not build the
> > features it enables:
> 
> Thanks for looking into this! Is there anything I should be doing,
> or are the patches you posted earlier sufficient?

Everything was merged already, I was just mentioning that we don't
support building with llvm-dev in debian:11 and a few other distros
where the version is old and doesn't match what is being used in perf.

- Arnaldo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v9 1/3] perf report: Support LLVM for addr2line()
  2024-09-03 14:15           ` Arnaldo Carvalho de Melo
  2024-09-08 18:52             ` Steinar H. Gunderson
@ 2024-09-10 14:06             ` James Clark
  1 sibling, 0 replies; 16+ messages in thread
From: James Clark @ 2024-09-10 14:06 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Steinar H. Gunderson
  Cc: linux-perf-users, linux-kernel, irogers, Arnaldo Carvalho de Melo,
	Namhyung Kim



On 9/3/24 15:15, Arnaldo Carvalho de Melo wrote:
> On Tue, Sep 03, 2024 at 11:01:36AM -0300, Arnaldo Carvalho de Melo wrote:
>> On Tue, Sep 03, 2024 at 07:09:47AM -0300, Arnaldo Carvalho de Melo wrote:
>>>     3: almalinux:9-i386WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
>>> WARNING: image platform (linux/386) does not match the expected platform (linux/amd64)
>>>      17.58 almalinux:9-i386              : FAIL gcc version 11.4.1 20231218 (Red Hat 11.4.1-3) (GCC)
>>>      util/llvm-c-helpers.cpp: In function ‘char* make_symbol_relative_string(dso*, const char*, u64, u64)’:
>>>      util/llvm-c-helpers.cpp:150:52: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=]
>>>        150 |                 snprintf(buf, sizeof(buf), "%s+0x%lx",
>>>            |                                                  ~~^
>>>            |                                                    |
>>>            |                                                    long unsigned int
>>>            |                                                  %llx
>>>        151 |                          demangled ? demangled : sym_name, addr - base_addr);
>>>            |                                                            ~~~~~~~~~~~~~~~~
>>>            |                                                                 |
>>>            |                                                                 u64 {aka long long unsigned int}
>>>      cc1plus: all warnings being treated as errors
>>>      make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
>>
>> The one above is fixed by the patch at the end, that I already added to
>> the cset where the problem was being introduced.
>>
>> Now there is something a bit more tricky, we'll have to add a feature
>> check to see if the libllvm has what is needed if this appears in some
>> distro we still want to support, since alpine 3.16 has what is needed
>> I'll take the opportunity to drop test building on alpine 3.15.
> 
> Or, as I'll do with debian:11, just remove llvm-dev and not build the
> features it enables:
> 
>    17    13.79 debian:11                     : FAIL gcc version 10.2.1 20210110 (Debian 10.2.1-6)
>      util/llvm-c-helpers.cpp: In function 'char* llvm_name_for_code(dso*, const char*, u64)':
>      util/llvm-c-helpers.cpp:178:21: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
>        178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
>            |                     ^~~~~~~~~~~~
>      util/llvm-c-helpers.cpp:178:49: error: 'std::remove_reference_t<llvm::DILineInfo>' {aka 'struct llvm::DILineInfo'} has no member named 'StartAddress'
>        178 |   addr, res_or_err->StartAddress ? *res_or_err->StartAddress : 0);
>            |                                                 ^~~~~~~~~~~~
>      make[3]: *** [/git/perf-6.11.0-rc3/tools/build/Makefile.build:158: util] Error 2
> 

I also hit this issue. I think it makes sense to auto detect the version 
so I sent a patch. It was quite easy to autodetect and it might not be 
that obvious to others how to get the build working again given this error.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-09-10 14:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-19 15:00 [PATCH v9 1/3] perf report: Support LLVM for addr2line() Steinar H. Gunderson
2024-07-19 15:00 ` [PATCH v9 2/3] perf annotate: split out read_symbol() Steinar H. Gunderson
2024-07-19 15:00 ` [PATCH v9 3/3] perf annotate: LLVM-based disassembler Steinar H. Gunderson
2024-09-02 14:53   ` Arnaldo Carvalho de Melo
2024-09-02 14:55     ` Arnaldo Carvalho de Melo
2024-07-30 19:42 ` [PATCH v9 1/3] perf report: Support LLVM for addr2line() Arnaldo Carvalho de Melo
2024-07-30 19:49   ` Arnaldo Carvalho de Melo
2024-08-03 15:11     ` Steinar H. Gunderson
2024-09-01 10:57     ` Steinar H. Gunderson
2024-09-03 10:09       ` Arnaldo Carvalho de Melo
2024-09-03 13:14         ` Arnaldo Carvalho de Melo
2024-09-03 14:01         ` Arnaldo Carvalho de Melo
2024-09-03 14:15           ` Arnaldo Carvalho de Melo
2024-09-08 18:52             ` Steinar H. Gunderson
2024-09-09 16:42               ` Arnaldo Carvalho de Melo
2024-09-10 14:06             ` James Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).