* [PATCH 1/3] perf callchain: Fix loss of symbol in merge_chain_branch()
2026-10-31 6:35 [PATCH 0/3] perf symbols/callchain/annotate: Bug fixes from symbol reference counting review Ian Rogers
@ 2026-10-31 6:35 ` Ian Rogers
2026-10-31 6:39 ` sashiko-bot
2026-10-31 6:35 ` [PATCH 2/3] perf symbol-elf: Fix zero-size symbol end calculation across .symtab and .dynsym Ian Rogers
2026-10-31 6:35 ` [PATCH 3/3] perf annotate: Fix block_range symbol loss on split and leaks on exit Ian Rogers
2 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-10-31 6:35 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Jiri Olsa, Adrian Hunter, James Clark, Alireza Haghdoost,
linux-perf-users, linux-kernel, Ian Rogers
Commit bffb5b0c0976 ("perf map/maps/thread: Changes to reference
counting") replaced passing &list->ms to callchain_cursor_append() in
merge_chain_branch() with a temporary struct map_symbol initialized with
.thread and .map, but omitted .sym = list->ms.sym.
As a result, callchains merged during histogram collapse lose their
resolved struct symbol pointer, causing 'perf report --children' to
print raw hexadecimal addresses instead of function names and inlined
frames on merged callchain branches.
Initialize .sym = list->ms.sym in the temporary struct map_symbol in
merge_chain_branch().
Fixes: bffb5b0c0976 ("perf map/maps/thread: Changes to reference counting")
Suggested-by: Alireza Haghdoost <haghdoost@uber.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/callchain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 31c675cbab63..6d5212be466d 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1092,6 +1092,7 @@ merge_chain_branch(struct callchain_cursor *cursor,
struct map_symbol ms = {
.thread = thread__get(list->ms.thread),
.map = map__get(list->ms.map),
+ .sym = list->ms.sym,
};
callchain_cursor_append(cursor, list->ip, &ms, false, NULL, 0, 0, 0, list->srcline);
list_del_init(&list->list);
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] perf symbol-elf: Fix zero-size symbol end calculation across .symtab and .dynsym
2026-10-31 6:35 [PATCH 0/3] perf symbols/callchain/annotate: Bug fixes from symbol reference counting review Ian Rogers
2026-10-31 6:35 ` [PATCH 1/3] perf callchain: Fix loss of symbol in merge_chain_branch() Ian Rogers
@ 2026-10-31 6:35 ` Ian Rogers
2026-10-31 6:41 ` sashiko-bot
2026-10-31 6:35 ` [PATCH 3/3] perf annotate: Fix block_range symbol loss on split and leaks on exit Ian Rogers
2 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-10-31 6:35 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Jiri Olsa, Adrian Hunter, James Clark, Alireza Haghdoost,
linux-perf-users, linux-kernel, Ian Rogers
When loading symbols from an ELF binary in dso__load_sym(),
dso__load_sym_internal() can be invoked multiple times: first for
.symtab (or .gnu_debugdata's .symtab) and then for .dynsym.
Previously, dso__load_sym_internal() ran symbols__fixup_end() and
symbols__fixup_duplicate() at the end of each individual symbol table
load. Consequently, a zero-size symbol in .symtab had its end address
extended to the start of the next .symtab symbol before .dynsym was
loaded. If an exported function existed only in .dynsym (for example,
when stripped from .symtab or when using .gnu_debugdata minidebuginfo)
at an address between the two .symtab symbols, the zero-size .symtab
symbol overlapped the .dynsym function and samples inside the .dynsym
function were misattributed to the preceding zero-size symbol.
Move symbols__fixup_end(), symbols__fixup_duplicate(), and
maps__fixup_end() from dso__load_sym_internal() to dso__load_sym() so
they run once after all symbol tables have been loaded into the DSO, and
add a unit test in 'perf test symbols' verifying that a zero-size
.symtab symbol ends at a following .dynsym-only function rather than
overlapping it.
Fixes: 87704345cc60 ("perf symbol-elf: Decode dynsym even if symtab exists")
Fixes: b10f74308e13 ("perf symbol: Support .gnu_debugdata for symbols")
Suggested-by: Alireza Haghdoost <haghdoost@uber.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/symbols.c | 169 +++++++++++++++++++++++++++++++++++
tools/perf/util/symbol-elf.c | 36 ++++----
2 files changed, 190 insertions(+), 15 deletions(-)
diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c
index b644d95c6be9..d3180db45ee6 100644
--- a/tools/perf/tests/symbols.c
+++ b/tools/perf/tests/symbols.c
@@ -2,8 +2,11 @@
#include <linux/compiler.h>
#include <linux/string.h>
#include <sys/mman.h>
+#include <elf.h>
#include <inttypes.h>
#include <limits.h>
+#include <stdlib.h>
+#include <unistd.h>
#include "debug.h"
#include "dso.h"
#include "env.h"
@@ -308,9 +311,175 @@ static int test__kallsyms_fixup_end(struct test_suite *test __maybe_unused,
return ret;
}
+#ifdef HAVE_LIBELF_SUPPORT
+static int write_test_elf(int fd)
+{
+ static const char shstrtab[] = "\0.shstrtab\0.text\0.strtab\0.symtab\0.dynsym";
+ static const char strtab[] = "\0zfun\0efun\0after_fun";
+ struct {
+ Elf64_Ehdr ehdr;
+ Elf64_Shdr shdrs[6];
+ char shstr[sizeof(shstrtab)];
+ char str[sizeof(strtab)];
+ Elf64_Sym symtab[3];
+ Elf64_Sym dynsym[2];
+ u8 text[0x60];
+ } img;
+
+ memset(&img, 0, sizeof(img));
+ memcpy(img.ehdr.e_ident, ELFMAG, SELFMAG);
+ img.ehdr.e_ident[EI_CLASS] = ELFCLASS64;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+ img.ehdr.e_ident[EI_DATA] = ELFDATA2MSB;
+#else
+ img.ehdr.e_ident[EI_DATA] = ELFDATA2LSB;
+#endif
+ img.ehdr.e_ident[EI_VERSION] = EV_CURRENT;
+ img.ehdr.e_type = ET_DYN;
+ img.ehdr.e_machine = EM_X86_64;
+ img.ehdr.e_version = EV_CURRENT;
+ img.ehdr.e_shoff = offsetof(typeof(img), shdrs);
+ img.ehdr.e_ehsize = sizeof(Elf64_Ehdr);
+ img.ehdr.e_shentsize = sizeof(Elf64_Shdr);
+ img.ehdr.e_shnum = ARRAY_SIZE(img.shdrs);
+ img.ehdr.e_shstrndx = 1;
+
+ memcpy(img.shstr, shstrtab, sizeof(shstrtab));
+ memcpy(img.str, strtab, sizeof(strtab));
+
+ /* Section 1: .shstrtab */
+ img.shdrs[1].sh_name = 1;
+ img.shdrs[1].sh_type = SHT_STRTAB;
+ img.shdrs[1].sh_offset = offsetof(typeof(img), shstr);
+ img.shdrs[1].sh_size = sizeof(shstrtab);
+
+ /* Section 2: .text */
+ img.shdrs[2].sh_name = 11;
+ img.shdrs[2].sh_type = SHT_PROGBITS;
+ img.shdrs[2].sh_flags = SHF_ALLOC | SHF_EXECINSTR;
+ img.shdrs[2].sh_addr = 0x1000;
+ img.shdrs[2].sh_offset = offsetof(typeof(img), text);
+ img.shdrs[2].sh_size = sizeof(img.text);
+
+ /* Section 3: .strtab */
+ img.shdrs[3].sh_name = 17;
+ img.shdrs[3].sh_type = SHT_STRTAB;
+ img.shdrs[3].sh_flags = SHF_ALLOC;
+ img.shdrs[3].sh_offset = offsetof(typeof(img), str);
+ img.shdrs[3].sh_size = sizeof(strtab);
+
+ /*
+ * Section 4: .symtab with zero-size zfun at 0x1010 and after_fun at
+ * 0x1040, but without efun (simulating objcopy --strip-symbol=efun or
+ * .gnu_debugdata minidebuginfo where exported symbols live only in
+ * .dynsym).
+ */
+ img.shdrs[4].sh_name = 25;
+ img.shdrs[4].sh_type = SHT_SYMTAB;
+ img.shdrs[4].sh_offset = offsetof(typeof(img), symtab);
+ img.shdrs[4].sh_size = sizeof(img.symtab);
+ img.shdrs[4].sh_link = 3;
+ img.shdrs[4].sh_info = 2;
+ img.shdrs[4].sh_entsize = sizeof(Elf64_Sym);
+
+ img.symtab[1].st_name = 1; /* zfun */
+ img.symtab[1].st_info = ELF64_ST_INFO(STB_LOCAL, STT_FUNC);
+ img.symtab[1].st_shndx = 2;
+ img.symtab[1].st_value = 0x1010;
+ img.symtab[1].st_size = 0;
+
+ img.symtab[2].st_name = 11; /* after_fun */
+ img.symtab[2].st_info = ELF64_ST_INFO(STB_GLOBAL, STT_FUNC);
+ img.symtab[2].st_shndx = 2;
+ img.symtab[2].st_value = 0x1040;
+ img.symtab[2].st_size = 0x10;
+
+ /* Section 5: .dynsym with efun at 0x1020..0x1030 */
+ img.shdrs[5].sh_name = 33;
+ img.shdrs[5].sh_type = SHT_DYNSYM;
+ img.shdrs[5].sh_flags = SHF_ALLOC;
+ img.shdrs[5].sh_offset = offsetof(typeof(img), dynsym);
+ img.shdrs[5].sh_size = sizeof(img.dynsym);
+ img.shdrs[5].sh_link = 3;
+ img.shdrs[5].sh_info = 1;
+ img.shdrs[5].sh_entsize = sizeof(Elf64_Sym);
+
+ img.dynsym[1].st_name = 6; /* efun */
+ img.dynsym[1].st_info = ELF64_ST_INFO(STB_GLOBAL, STT_FUNC);
+ img.dynsym[1].st_shndx = 2;
+ img.dynsym[1].st_value = 0x1020;
+ img.dynsym[1].st_size = 0x10;
+
+ return write(fd, &img, sizeof(img)) == (ssize_t)sizeof(img) ? 0 : -1;
+}
+
+static int test__elf_symtab_dynsym_fixup_end(struct test_suite *test __maybe_unused,
+ int subtest __maybe_unused)
+{
+ char tmpfile[] = "/tmp/perf-test-symtab-dynsym-XXXXXX";
+ struct test_info ti;
+ struct map *map = NULL;
+ struct dso *dso;
+ u64 text_offs;
+ int fd, ret;
+
+ fd = mkstemp(tmpfile);
+ if (fd < 0)
+ return TEST_FAIL;
+
+ if (write_test_elf(fd)) {
+ close(fd);
+ unlink(tmpfile);
+ return TEST_FAIL;
+ }
+ close(fd);
+
+ ret = init_test_info(&ti);
+ if (ret != TEST_OK) {
+ unlink(tmpfile);
+ return ret;
+ }
+
+ ret = create_map(&ti, tmpfile, &map);
+ if (ret != TEST_OK)
+ goto out_exit;
+
+ dso = map__dso(map);
+ if (dso__load(dso, map) <= 0) {
+ ret = TEST_FAIL;
+ goto out_put;
+ }
+
+ /* Verify no overlapping or zero-length symbols exist. */
+ ret = test_dso(dso);
+ text_offs = 0x1000 - dso__text_offset(dso);
+ if (check_symbol(dso, text_offs + 0x14, "zfun", text_offs + 0x20))
+ ret = TEST_FAIL;
+ if (check_symbol(dso, text_offs + 0x24, "efun", text_offs + 0x30))
+ ret = TEST_FAIL;
+ if (check_symbol(dso, text_offs + 0x44, "after_fun", text_offs + 0x50))
+ ret = TEST_FAIL;
+
+out_put:
+ map__put(map);
+out_exit:
+ exit_test_info(&ti);
+ unlink(tmpfile);
+ return ret;
+}
+#else
+static int test__elf_symtab_dynsym_fixup_end(struct test_suite *test __maybe_unused,
+ int subtest __maybe_unused)
+{
+ return TEST_SKIP;
+}
+#endif
+
static struct test_case tests__symbols[] = {
TEST_CASE("Symbols", symbols),
TEST_CASE("Kallsyms symbol ends", kallsyms_fixup_end),
+ TEST_CASE("ELF zero-size symbol end across .symtab and .dynsym",
+ elf_symtab_dynsym_fixup_end),
{ .name = NULL, }
};
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e2716c6af45..77ef5b2dbf32 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1789,21 +1789,6 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
nr++;
}
dso__put(curr_dso);
-
- /*
- * For misannotated, zeroed, ASM function sizes.
- */
- if (nr > 0) {
- symbols__fixup_end(dso__symbols(dso), false);
- symbols__fixup_duplicate(dso__symbols(dso));
- if (kmap) {
- /*
- * We need to fixup this here too because we create new
- * maps here, for things like vsyscall sections.
- */
- maps__fixup_end(kmaps);
- }
- }
return nr;
out_elf_end:
dso__put(curr_dso);
@@ -1864,6 +1849,27 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
nr += err;
}
+ /*
+ * For misannotated, zeroed, ASM function sizes. Run after all symbol
+ * tables (.symtab, .dynsym, .gnu_debugdata) have been loaded so a
+ * zero-size symbol in .symtab ends at the next symbol across all
+ * tables rather than overlapping a .dynsym-only function.
+ */
+ if (nr > 0) {
+ struct kmap *kmap = dso__kernel(dso) ? map__kmap(map) : NULL;
+ struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
+
+ symbols__fixup_end(dso__symbols(dso), false);
+ symbols__fixup_duplicate(dso__symbols(dso));
+ if (kmap) {
+ /*
+ * We need to fixup this here too because we create new
+ * maps here, for things like vsyscall sections.
+ */
+ maps__fixup_end(kmaps);
+ }
+ }
+
return nr;
}
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] perf annotate: Fix block_range symbol loss on split and leaks on exit
2026-10-31 6:35 [PATCH 0/3] perf symbols/callchain/annotate: Bug fixes from symbol reference counting review Ian Rogers
2026-10-31 6:35 ` [PATCH 1/3] perf callchain: Fix loss of symbol in merge_chain_branch() Ian Rogers
2026-10-31 6:35 ` [PATCH 2/3] perf symbol-elf: Fix zero-size symbol end calculation across .symtab and .dynsym Ian Rogers
@ 2026-10-31 6:35 ` Ian Rogers
2026-10-31 6:47 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-10-31 6:35 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Jiri Olsa, Adrian Hunter, James Clark, Alireza Haghdoost,
linux-perf-users, linux-kernel, Ian Rogers
When block_range__create() splits an existing struct block_range entry
into a leading head range (entry->start < start) or a trailing tail
range (end < entry->end), it copies coverage and branch statistics from
the original entry into head/tail, but omits .sym = entry->sym. Because
the returned iterator only spans [start, end], process_basic_block()
never revisits the split-off head or tail range, leaving its .sym NULL
and causing block_range__coverage() to return -1 on instructions in that
split range.
Additionally, the global block_ranges rbtree is never freed on exit from
cmd_annotate(), and process_branch_stack() frees the resolved
branch_info array without calling addr_map_symbol__exit() on bi[i].to
and bi[i].from, leaking thread and map references acquired by
sample__resolve_bstack().
Preserve .sym = entry->sym when splitting head and tail in
block_range__create(), add block_ranges__exit() and call it from
cmd_annotate(), and release bi[i].to and bi[i].from in
process_branch_stack().
Fixes: 70fbe0574558 ("perf annotate: Add branch stack / basic block")
Fixes: bffb5b0c0976 ("perf map/maps/thread: Changes to reference counting")
Suggested-by: Alireza Haghdoost <haghdoost@uber.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-annotate.c | 5 +++++
tools/perf/util/block-range.c | 16 ++++++++++++++++
tools/perf/util/block-range.h | 1 +
3 files changed, 22 insertions(+)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 8863f3a570f6..331adbdbd635 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -174,6 +174,10 @@ static void process_branch_stack(struct branch_stack *bs, struct addr_location *
prev = &bi[i].to;
}
+ for (i = 0; i < (int)bs->nr; i++) {
+ addr_map_symbol__exit(&bi[i].to);
+ addr_map_symbol__exit(&bi[i].from);
+ }
free(bi);
}
@@ -967,6 +971,7 @@ int cmd_annotate(int argc, const char **argv)
#ifndef NDEBUG
perf_session__delete(annotate.session);
#endif
+ block_ranges__exit();
annotation_options__exit();
return ret;
diff --git a/tools/perf/util/block-range.c b/tools/perf/util/block-range.c
index 7c559fcfd7e0..280d695c6c58 100644
--- a/tools/perf/util/block-range.c
+++ b/tools/perf/util/block-range.c
@@ -165,6 +165,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
return iter;
*head = (struct block_range){
+ .sym = entry->sym,
.start = entry->start,
.end = start - 1,
.is_target = entry->is_target,
@@ -203,6 +204,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
return iter;
*tail = (struct block_range){
+ .sym = entry->sym,
.start = end + 1,
.end = entry->end,
.is_target = 0,
@@ -330,3 +332,17 @@ double block_range__coverage(struct block_range *br)
return (double)br->coverage / branch->max_coverage;
}
+
+void block_ranges__exit(void)
+{
+ struct rb_node *rb = rb_first(&block_ranges.root);
+
+ while (rb) {
+ struct block_range *entry = rb_entry(rb, struct block_range, node);
+
+ rb = rb_next(rb);
+ rb_erase(&entry->node, &block_ranges.root);
+ free(entry);
+ }
+ block_ranges.blocks = 0;
+}
diff --git a/tools/perf/util/block-range.h b/tools/perf/util/block-range.h
index ec0fb534bf56..0ea4751292c8 100644
--- a/tools/perf/util/block-range.h
+++ b/tools/perf/util/block-range.h
@@ -72,5 +72,6 @@ static inline bool block_range_iter__valid(struct block_range_iter *iter)
extern struct block_range *block_range__find(u64 addr);
extern struct block_range_iter block_range__create(u64 start, u64 end);
extern double block_range__coverage(struct block_range *br);
+extern void block_ranges__exit(void);
#endif /* __PERF_BLOCK_RANGE_H */
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread