Re: [PATCH] bpftool: Make the LLVM disassembler an optional runtime dependency

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The 'dump jited' functionality is completely broken for statically linked builds of bpftool.
- [High] Linking static LLVM archives into a shared object plugin will cause relocation errors and build failures on many distributions.
- [Medium] Hardcoding the absolute installation path for the LLVM plugin prevents running bpftool locally from the build tree.
- [Low] Multi-line comment does not follow the required BPF subsystem style.
--

commit c9ad3aff3b32fd9fdba7deb04f2d26819aac8786
Author: Korenberg Mark <socketpair@gmail.com>

bpftool: Make the LLVM disassembler an optional runtime dependency

This commit splits the libLLVM JIT disassembler into a separate shared
plugin. It dynamically loads the plugin via dlopen() to remove the hard
runtime dependency on the large libLLVM library from the main bpftool binary.

> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 0febf60e1b646..9887ac6fbae3e 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
[ ... ]
> @@ -165,19 +168,36 @@ endif
>  endif
>  
>  ifeq ($(HAS_LLVM),1)
> +  # The libLLVM-based JIT disassembler is built as a separate plugin,
> +  # bpftool-llvm.so, which is the only object that links against libLLVM.
> +  # bpftool loads it lazily with dlopen() (see jit_disasm.c), so the bpftool
> +  # binary itself keeps no dependency on the large libLLVM shared object.
>    CFLAGS  += -DHAVE_LLVM_SUPPORT
> +  CFLAGS  += -DLLVM_PLUGIN_DIR='"$(libdir)/bpftool"'

[Severity: Medium]
By unconditionally baking the absolute installation path into the binary, does
this break running bpftool locally from the build tree?

Standard developer workflows or selftests that execute a locally built bpftool
from a scratch directory won't have the plugin installed in the system library
directory. Since dlopen() is called with this absolute path exclusively, it
will fail to load the plugin locally.

[ ... ]
> +  ifeq ($(shell $(LLVM_CONFIG) --shared-mode),static)
> -    LIBS += $(shell $(LLVM_CONFIG) --system-libs $(LLVM_CONFIG_LIB_COMPONENTS))
> -    LIBS += -lstdc++
> +    LLVM_LINK_STATIC := 1
> +  endif
> +  ifeq ($(LLVM_LINK_STATIC),1)
> +    LLVM_PLUGIN_LIBS := $(shell $(LLVM_CONFIG) --link-static --libs $(LLVM_CONFIG_LIB_COMPONENTS))
> +    LLVM_PLUGIN_LIBS += $(shell $(LLVM_CONFIG) --link-static --system-libs $(LLVM_CONFIG_LIB_COMPONENTS))
> +    LLVM_PLUGIN_LIBS += -lstdc++
> +  else
> +    LLVM_PLUGIN_LIBS := $(shell $(LLVM_CONFIG) --libs $(LLVM_CONFIG_LIB_COMPONENTS))
>    endif
[ ... ]
> @@ -276,6 +296,20 @@ $(BPFTOOL_BOOTSTRAP): $(BOOTSTRAP_OBJS) $(LIBBPF_BOOTSTRAP)
>  $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
>  	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(OBJS) $(LIBS) -o $@
>  
> +ifeq ($(HAS_LLVM),1)
> +all: $(OUTPUT)bpftool-llvm.so
> +
> +$(OUTPUT)llvm_disasm.o: llvm_disasm.c
> +	$(QUIET_CC)$(CC) $(CFLAGS) $(LLVM_PLUGIN_CFLAGS) -fPIC -c -MMD $< -o $@
> +
> +# The plugin is a shared object by definition, so drop a global -static (e.g.
> +# from EXTRA_LDFLAGS for a static bpftool) which would conflict with -shared.
> +# Embedding libLLVM statically is controlled separately (see LLVM_LINK_STATIC).
> +$(OUTPUT)bpftool-llvm.so: $(OUTPUT)llvm_disasm.o
> +	$(QUIET_LINK)$(CC) $(CFLAGS) $(filter-out -static,$(LDFLAGS)) \
> +		$(LLVM_PLUGIN_LDFLAGS) -shared -o $@ $< $(LLVM_PLUGIN_LIBS)

[Severity: High]
Will linking static LLVM archives into a shared object plugin cause relocation
errors and build failures on distributions like Debian and Ubuntu?

Since static libraries are typically not compiled with position independent
code, attempting to link them with the shared flag here often produces linker
relocation errors.

[Severity: High]
Does this completely break the disassemble functionality for statically linked
builds of bpftool?

Statically linking bpftool produces a static binary, but this rule forces the
LLVM disassembler to be built exclusively as a shared object. Since static
binaries generally lack full dynamic loader capabilities for loading shared
objects via dlopen(), standalone static builds of bpftool will no longer be
able to disassemble JITed code.

> diff --git a/tools/bpf/bpftool/llvm_disasm.c b/tools/bpf/bpftool/llvm_disasm.c
> new file mode 100644
> index 0000000000000..b832161911ebb
> --- /dev/null
> +++ b/tools/bpf/bpftool/llvm_disasm.c
[ ... ]
> +#include <llvm-c/TargetMachine.h>
> +
> +#include "llvm_disasm.h"
> +
> +/* This callback to set the ref_type is necessary to have the LLVM disassembler
> + * print PC-relative addresses instead of byte offsets for branch instruction
> + * targets.
> + */
> +static const char *
> +symbol_lookup_callback(void *disasm_info, uint64_t ref_value,

[Severity: Low]
This isn't a bug, but the multi-line comment does not follow the required BPF
subsystem style. The opening block comment indicator should be on its own
line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-bpftool-plugin-v1-1-68c0ac91f5a3@gmail.com?part=1