Re: [PATCH v1] perf symbol: Add rust v0 demangling support

[Daniel Xu]

On Mon, Feb 03, 2025 at 09:20:30AM -0800, Ian Rogers wrote:
> On Thu, Jan 30, 2025 at 11:35 AM Daniel Xu <dxu@xxxxxxxxx> wrote:
> >
> > Hi Ian,
> >
> > Thanks so much for working on this!
> >
> > Only did a quick skim. Seems like it'd be nice to reuse the official
> > demangler now that we know about it. Will be interesting to
> > see how that gets packaged for distros.
> 
> I think the best solution is to use the official demangler but in
> general we'd like fewer rather than more library dependencies. For
> example your initial comments were to work to make the coupling with
> capstone and LLVM disassemblers more loose via dlopen:
> https://lore.kernel.org/lkml/20250122174308.350350-1-irogers@xxxxxxxxxx/
> 
> Thinking through the options (note I'm not doing Rust things and so my
> level of skin in the game is minimal):
> 1) stay as we are - well this is a mess as there is no v0 support and
> the legacy support comes from libbfd/libiberty.
> 2) option (1) plus this change cleaned up - there's still the mess
> with the legacy support, which is probably more important given v0
> isn't widely adopted.
> 3) wait for the official demangler to be packaged as a library and
> depend upon it, add testing from here - solves legacy and v0 issues,
> but brings in an extra library dependency to the build. Timeline not
> clear.
> 4) "contribute"/copy-paste the official demangler into perf tree and
> do like (3). Perhaps remove the forked file when a dependable library
> exists (there's some precedent for this with how libtraceevent was in
> and then removed from the linux tree) - solves the timeline, not great
> to fork the official demangler, so far not heard from the author Ariel
> Ben-Yehuda and I would strongly prefer they contribute rather than I
> copy-paste it.
> 
> For (3) and (4) it is probably a good idea to engage the Rust folks in:
> https://github.com/rust-lang/rust/issues/60705
> 
> Having broken Rust demangling seems like a high priority issue for the
> perf tool.
> 

FWIW, while I quite enjoy rust, I don't write much of it day to day
anymore. I was only investigating on behalf of someone else. So whatever
other folks prefer is fine with me.

[...]

> >
> >      * INPUT:
> >      *     sym: symbol that has been through BFD-demangling
> >
> > Perhaps double check that cxxabi is sufficient and update the comment?
> 
> I've not tested this change beyond making it pass the tests the change
> contains, that are based on the v0 specification's examples. I believe
> this/legacy may be working as most C++ implies libstdc++ which I
> believe uses libiberty in cxxabi, so BFD demangling. Given the
> popularity of libstdc++ I imagine things like LLVM's libcxx will try
> to be compatible. Perhaps if there are examples of legacy Rust symbols
> we can add a test?

Like you said above, let's see which direction the community prefers
first.

Thanks again,
Daniel