Re: [PATCH v6 2/5] perf symbols: Split out name-independent duplicate symbol selection

[Ian Rogers]

On Wed, Oct 28, 2026 at 4:42 PM Alireza Haghdoost via B4 Relay
<devnull+haghdoost.uber.com@xxxxxxxxxx> wrote:
>
> From: Alireza Haghdoost <haghdoost@xxxxxxxx>
>
> This patch is a refactoring and does not change behavior. It moves the
> size, type and binding checks from choose_best_symbol() to
> choose_best_symbol_by_attrs(). It also adds symbol__choice_needs_names()
> to report whether choosing between two symbols requires their names.
>
> symbols__fixup_duplicate() keeps one of the symbols that share a start
> address. It compares size, type and binding first. If those attributes
> are equal, it compares leading underscores, name lengths and the result
> of arch__choose_best_symbol().
>
> An unresolved lazy symbol stores a file offset instead of a name
> pointer. Thus, the lazy loader cannot call choose_best_symbol() until it
> loads both names. The new helper lets the loader keep the names
> unresolved when the attributes select a winner. It loads names only when
> the attributes are equal and duplicate selection needs them.
>
> Signed-off-by: Alireza Haghdoost <haghdoost@xxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Generally I think attr/attribute means perf_event_attr in the code,
but I lack naming abilitites, so it works for me.

Thanks,
Ian

> ---
>  tools/perf/util/symbol.c | 26 ++++++++++++++++++++++++--
>  tools/perf/util/symbol.h |  1 +
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index b61a574ce9a3..2d611c05d000 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -157,11 +157,15 @@ int __weak arch__choose_best_symbol(struct symbol *syma,
>         return SYMBOL_A;
>  }
>
> -static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
> +/*
> + * Choose between two symbols at the same address by size, type and binding.
> + * Returns SYMBOL_A or SYMBOL_B, or -1 if only their names can decide.
> + */
> +static int choose_best_symbol_by_attrs(const struct symbol *syma,
> +                                      const struct symbol *symb)
>  {
>         s64 a;
>         s64 b;
> -       size_t na, nb;
>
>         /* Prefer a symbol with non zero length */
>         a = syma->end - syma->start;
> @@ -194,6 +198,24 @@ static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
>         if (b && !a)
>                 return SYMBOL_B;
>
> +       return -1;
> +}
> +
> +bool symbol__choice_needs_names(const struct symbol *syma, const struct symbol *symb)
> +{
> +       return choose_best_symbol_by_attrs(syma, symb) < 0;
> +}
> +
> +static int choose_best_symbol(struct symbol *syma, struct symbol *symb)
> +{
> +       int ret = choose_best_symbol_by_attrs(syma, symb);
> +       s64 a;
> +       s64 b;
> +       size_t na, nb;
> +
> +       if (ret >= 0)
> +               return ret;
> +
>         /* Prefer a symbol with less underscores */
>         a = prefix_underscores_count(syma->name);
>         b = prefix_underscores_count(symb->name);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 46b1649c64fc..d8386c4a7fc5 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -303,6 +303,7 @@ int arch__compare_symbol_names(const char *namea, const char *nameb);
>  int arch__compare_symbol_names_n(const char *namea, const char *nameb,
>                                  unsigned int n);
>  int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> +bool symbol__choice_needs_names(const struct symbol *syma, const struct symbol *symb);
>
>  enum symbol_tag_include {
>         SYMBOL_TAG_INCLUDE__NONE = 0,
>
> --
> Git-157)
>
>