Re: [PATCH v3 13/18] perf build: Remove libbfd support

[Daniel Xu]

On Tue, Jan 28, 2025 at 05:40:44PM -0800, Ian Rogers wrote:
> On Tue, Jan 28, 2025 at 4:31 PM Daniel Xu <dxu@xxxxxxxxx> wrote:
> >
> > Hi Ian,
> >
> > On Wed, Jan 22, 2025 at 09:43:03AM -0800, Ian Rogers wrote:
> > > libbfd is license incompatible with perf and building requires the
> > > BUILD_NONDISTRO=1 build flag. Remove the code to simplify the code
> > > base.
> > >
> > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > > ---
> > >  tools/perf/Documentation/perf-check.txt |   1 -
> > >  tools/perf/Makefile.config              |  38 +---
> > >  tools/perf/builtin-check.c              |   1 -
> > >  tools/perf/tests/Build                  |   1 -
> > >  tools/perf/tests/builtin-test.c         |   1 -
> > >  tools/perf/tests/pe-file-parsing.c      | 101 ----------
> > >  tools/perf/tests/tests.h                |   1 -
> > >  tools/perf/util/demangle-cxx.cpp        |  13 +-
> > >  tools/perf/util/disasm_bpf.c            | 166 ----------------
> > >  tools/perf/util/srcline.c               | 243 +-----------------------
> > >  tools/perf/util/symbol-elf.c            |  86 +--------
> > >  tools/perf/util/symbol.c                | 135 -------------
> > >  tools/perf/util/symbol.h                |   4 -
> > >  13 files changed, 7 insertions(+), 784 deletions(-)
> > >  delete mode 100644 tools/perf/tests/pe-file-parsing.c
> >
> > [..]
> >
> > I was briefly investigating why the centos build of perf was not
> > demangling rust v0 symbols [0]. From looking at the rust issue [1], it
> > appears the rust team somehow delivered support for v0 demangling
> > through libbfd. The code itself looked a bit odd (relying on cxx
> > demangle to run first?), but that's a separate thing.
> 
> There is still C++ demangling support by way of cxxabi:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/demangle-cxx.cpp?h=perf-tools-next#n44
> that was in libstdc++ (GNU) and libcxx (LLVM) when I looked.
> 
> > The centos build does not build with libbfd for the license issues you
> > mentioned. So your change probably won't regress any distro use cases.
> > But it does remove support for motivated users who don't have
> > re-distribution requirements.
> >
> > But since this patchset came up first in my search, I thought it'd be
> > good to mention that someone probably needs to add v0 support to
> > tools/perf/util/demangle-rust.c.
> 
> So I don't see any libbfd dependencies in demangle-rust.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/demangle-rust.c?h=perf-tools-next#n8
> Unusually we don't have any tests on the Rust demangling, we do for
> Java and OCaml:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/demangle-java-test.c?h=perf-tools-next
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/demangle-ocaml-test.c?h=perf-tools-next
> 
> Reading a bit more it seems that previous libiberty was coming to the
> rescue by way of C++ demangling. I'll see if I can write a demangler
> by way of lex and yacc.

Cool :)

> If we have a v0 standard one is there any
> value in the existing demangler or legacy demangling? It seems this
> has been broken for the best part of 5 years.

I believe the "legacy" symbol format is still the rust default. So
probably can't remove that. Looks like there's some desire to change
that, probably probably not very soon [0].

That probably also explains why nobody reported the breakage - only very
cool kids are using v0 scheme currently.

Thanks,
Daniel


[0]: https://github.com/rust-lang/rust/pull/89917