Re: [PATCH v7 3/4] perf script: Add --lazy-load-symbols to load symbol names lazily

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf symbols: skipping .dynsym copy in dso__load_sym_internal()
  alters fixup
--

commit bbcf49c4a773356002f2cf428fb0dbd0071729ba
Author: Alireza Haghdoost <haghdoost@uber.com>

perf script: Add --lazy-load-symbols to load symbol names lazily

This commit adds a --lazy-load-symbols option to perf script that defers
reading and demangling userspace ELF symbol names until a symbol lookup
returns them, storing the string table file offset in the meantime.

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 98b6d7c73d235..7b7f06a882f61 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c

[ ... ]

> @@ -1774,14 +2020,36 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>  			}
>  		}
>  
> -		demangled = dso__demangle_sym(dso, kmodule, elf_name);
> -		if (demangled != NULL)
> -			elf_name = demangled;
> +		/*
> +		 * Most .dynsym entries are copies of .symtab symbols, which
> +		 * duplicate selection would drop anyway. Skip them here, so
> +		 * that perf does not read both names just to choose one.
> +		 */
> +		if (lazy_names && dynsym && !symbol_conf.allow_aliases) {
> +			f = lazy_dynsym_copy(dso__symbols(curr_dso), elf, &sym, elf_name);
> +			if (f) {
> +				if (GELF_ST_TYPE(sym.st_info) == STT_GNU_IFUNC)
> +					symbol__set_ifunc_alias(f, true);
> +				continue;
> +			}
> +		}

[Severity: Low]
Can skipping the .dynsym copy here change how symbols__fixup_end() and
symbols__fixup_duplicate() resolve symbols when .symtab also has a
zero-size symbol at the same address?

__symbols__insert() places symbols with equal start addresses in insertion
order.  If .symtab has a sized symbol followed by a zero-size symbol at
the same address (such as an assembly label without a .size directive),
and .dynsym has a copy of the sized symbol, eager loading inserts the
.dynsym copy after the zero-size symbol.  When symbols__fixup_end() runs,
the node following the zero-size symbol still has the same start address,
so the zero-size symbol keeps a zero length and symbols__fixup_duplicate()
drops it in favor of the sized symbol.

When this check skips inserting the .dynsym copy, the zero-size symbol is
left as the last node at that start address.  symbols__fixup_end() then
stretches its end to the next higher symbol address (or rounds it up by
4096 if it is the last symbol), giving it a bogus non-zero size so
symbols__fixup_duplicate() no longer discards it for having zero length
and can select it over the real sized symbol.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-perf-symbol-memory-send-v7-0-d992eced9f9c@uber.com?part=3