On Thu, Jan 23, 2025 at 1:59 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote: > > On Tue, Jan 21, 2025 at 10:23:15PM -0800, Ian Rogers wrote: > > Linking against libcapstone and libLLVM can be a significant increase > > in dependencies and size of memory footprint. For something like `perf > > record` the disassembler and addr2line functionality won't be > > used. Support dynamically loading these libraries using dlopen and > > then calling the appropriate functions found using dlsym. > > It's not clear from the description how you would use dlopen/dlsym. > Based on an offline discussion, you want to leave the current linking > model as is, and to support dlopen/dlsym when it's NOT detected at > build-time, right? Yep. Current behavior is no header file than these options fail, new behavior is that we try to use dlopen/dlsym and fail if the dlopen fails. > For that, you need to carry some definitions of the functions and types > for the used APIs. But I'm not sure if it's right to carry them in the > perf code base. Right. I mention that here: https://lore.kernel.org/lkml/CAP-5=fUhNuybCU-2_5EgcCwgwXnxvyFMvyhzKe=ZP1bssQwXHw@xxxxxxxxxxxxxx/ For LLVM we need 3 typedefs and 5 #defines, for capstone we need 2 structs and 5 enums (if we #ifdef some x86 only formatting code). The problem in not carrying those definitions is: 1) if the header file isn't present a build won't support LLVM/capstone even by dlopen - everything falls through to objdump/addr2line that have known performance issues; 2) package maintainers either need to spot a warning message to realize they've done this by having a missing header file (hard to spot and brittle) or we require the build to fail and people without capstone.h opt out of the build error with NO_CAPSTONE=1 - something perf developers will probably not like; 3) the LLVM/capstone code needs #ifdefs and __maybe_unused to suppress compiler warnings, or perhaps we have a minimal version of those files, leading to extra code complexity. I believe the approach here is no worse than what we do with vmlinux.h for BPF code and is robust as depending on dlsym being able to look up the function names. It is not perfect but I think it is more perfect and less complex than the alternative. > > > > BUILD_NONDISTRO is used to build perf against the license incompatible > > libbfd and libiberty libraries. As this has been opt-in for nearly 2 > > years, commit dd317df07207 ("perf build: Make binutil libraries opt > > in"), remove the code to simplify the code base. > > This part can be a separate series. Right, I posted it as a series here: https://lore.kernel.org/lkml/20250111202851.1075338-1-irogers@xxxxxxxxxx/ as mentioned in the v2 notes below. The issue was that Arnaldo pointed out removing BUILD_NONDISTRO removed disassemble_bpf that had only been implemented for libbfd. This series adds the LLVM/capstone definitions built into the symbol__disassemble refactor. Merging that series would conflict with this series, so I posted everything together to avoid having series of patches depending upon one another. I also wanted to check that what is in disasm.c in the end is reasonable, which I believe it is with significantly reduced ifdef-ery. > > > > The patch series: > > 1) does some initial clean up; > > 2) moves the capstone and LLVM code to their own C files, > > 3) simplifies a little the capstone code; > > I like changes up to this in general. Let me take a look at the > patches. Thanks, Ian Ian > Thanks, > Namhyung
