Re: [PATCH v1 4/5] perf capstone: Support for dlopen-ing libcapstone.so

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Jan 23, 2025 at 01:37:16PM -0800, Ian Rogers wrote:
> > > On Wed, Jan 22, 2025 at 2:27 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > I think reverse engineering the types is fragile.  Can we simply change
> > > > to dlopen if libcapstone is available?
> 
> To understand the scope of the problem, here is the LLVM code:
> https://github.com/googleprodkernel/linux-perf/blob/google_tools_master/tools/perf/util/llvm.c#L22
> 3 typedefs and 5 #defines on 13 lines of code.

It doesn't look that bad than I thought. :)

> 
> The capstone code is larger primarily due to needing to define cs_x86
> (and its dependencies) which are used for extra detail in printing:
> https://github.com/googleprodkernel/linux-perf/blob/google_tools_master/tools/perf/util/capstone.c#L23-L132
> We could opt out of providing the extra detail if capstone.h isn't
> present. I believe this can remove lines 49 to 123. This would mean
> there would be 37 lines of reverse engineered types - 2 structs and 5
> enums.

Probably, we don't need to carry the full details of instructions when
the library support was not built.

> 
> My feeling is that this isn't significant and better than returning
> errors for capstone/llvm, having #ifdefs/__maybe_unused for the cases
> we lack the types throughout the code and relying on people building
> noticing a warning to realize they need to install a dependency - the
> alternative of failing the build if the capstone/LLVM header files
> aren't present, and unless some NO_... build option isn't passed,
> likely won't fly with perf developers.

I don't think failing the build due to the optional dependencies is an
option.  I'm not against the idea of dlopen but my concern is to carry
those external definitions in the perf code base.  Maybe it'd be ok if
we could keep it minimally.

Thanks,
Namhyung