On Thu, Aug 8, 2024 at 6:05 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> int get_jited_program_text(int fd, char *text, size_t text_sz)
>
> Loads and disassembles jited instructions for program pointed to by fd.
> Much like 'bpftool prog dump jited ...'.
>
> The code and makefile changes are inspired by jit_disasm.c from bpftool.
> Use llvm libraries to disassemble BPF program instead of libbfd to avoid
> issues with disassembly output stability pointed out in [1].
>
> Selftests makefile uses Makefile.feature to detect if LLVM libraries
> are available. If that is not the case selftests build proceeds but
> the function returns -ENOTSUP at runtime.
>
> [1] commit eb9d1acf634b ("bpftool: Add LLVM as default library for disassembling JIT-ed programs")
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/.gitignore | 1 +
> tools/testing/selftests/bpf/Makefile | 51 +++-
> .../selftests/bpf/jit_disasm_helpers.c | 228 ++++++++++++++++++
> .../selftests/bpf/jit_disasm_helpers.h | 10 +
> 4 files changed, 288 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/jit_disasm_helpers.c
> create mode 100644 tools/testing/selftests/bpf/jit_disasm_helpers.h
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 8f14d8faeb0b..7de4108771a0 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -8,6 +8,7 @@ test_lru_map
> test_lpm_map
> test_tag
> FEATURE-DUMP.libbpf
> +FEATURE-DUMP.selftests
> fixdep
> /test_progs
> /test_progs-no_alu32
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index f54185e96a95..b1a52739d9e7 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -33,6 +33,13 @@ OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> LIBELF_CFLAGS := $(shell $(PKG_CONFIG) libelf --cflags 2>/dev/null)
> LIBELF_LIBS := $(shell $(PKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>
> +ifeq ($(srctree),)
> +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +endif
> +
> CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> -Wall -Werror -fno-omit-frame-pointer \
> $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS) \
> @@ -55,6 +62,11 @@ progs/test_sk_lookup.c-CFLAGS := -fno-strict-aliasing
> progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
> progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
>
> +# Some utility functions use LLVM libraries
> +jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)
> +test_progs-LDLIBS = $(LLVM_LDLIBS)
> +test_progs-LDFLAGS = $(LLVM_LDFLAGS)
> +
> ifneq ($(LLVM),)
> # Silence some warnings when compiled with clang
> CFLAGS += -Wno-unused-command-line-argument
> @@ -164,6 +176,31 @@ endef
>
> include ../lib.mk
>
> +NON_CHECK_FEAT_TARGETS := clean docs-clean
> +CHECK_FEAT := $(filter-out $(NON_CHECK_FEAT_TARGETS),$(or $(MAKECMDGOALS), "none"))
> +ifneq ($(CHECK_FEAT),)
> +FEATURE_USER := .selftests
> +FEATURE_TESTS := llvm
> +FEATURE_DISPLAY := $(FEATURE_TESTS)
> +
> +# Makefile.feature expects OUTPUT to end with a slash
> +$(let OUTPUT,$(OUTPUT)/,\
> + $(eval include ../../../build/Makefile.feature))
> +endif
> +
> +ifeq ($(feature-llvm),1)
> + LLVM_CFLAGS += -DHAVE_LLVM_SUPPORT
> + LLVM_CONFIG_LIB_COMPONENTS := mcdisassembler all-targets
> + # both llvm-config and lib.mk add -D_GNU_SOURCE, which ends up as conflict
> + LLVM_CFLAGS += $(filter-out -D_GNU_SOURCE,$(shell $(LLVM_CONFIG) --cflags))
> + LLVM_LDLIBS += $(shell $(LLVM_CONFIG) --libs $(LLVM_CONFIG_LIB_COMPONENTS))
> + ifeq ($(shell $(LLVM_CONFIG) --shared-mode),static)
> + LLVM_LDLIBS += $(shell $(LLVM_CONFIG) --system-libs $(LLVM_CONFIG_LIB_COMPONENTS))
> + LLVM_LDLIBS += -lstdc++
> + endif
> + LLVM_LDFLAGS += $(shell $(LLVM_CONFIG) --ldflags)
> +endif
> +
> SCRATCH_DIR := $(OUTPUT)/tools
> BUILD_DIR := $(SCRATCH_DIR)/build
> INCLUDE_DIR := $(SCRATCH_DIR)/include
> @@ -488,6 +525,7 @@ define DEFINE_TEST_RUNNER
>
> TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2
> TRUNNER_BINARY := $1$(if $2,-)$2
> +TRUNNER_BASE_NAME := $1
> TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o, \
> $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c)))
> TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \
> @@ -611,6 +649,10 @@ ifeq ($(filter clean docs-clean,$(MAKECMDGOALS)),)
> include $(wildcard $(TRUNNER_TEST_OBJS:.o=.d))
> endif
>
> +# add per extra obj CFGLAGS definitions
> +$(foreach N,$(patsubst $(TRUNNER_OUTPUT)/%.o,%,$(TRUNNER_EXTRA_OBJS)), \
> + $(eval $(TRUNNER_OUTPUT)/$(N).o: CFLAGS += $($(N).c-CFLAGS)))
> +
> $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
> %.c \
> $(TRUNNER_EXTRA_HDRS) \
> @@ -627,6 +669,9 @@ ifneq ($2:$(OUTPUT),:$(shell pwd))
> $(Q)rsync -aq $$^ $(TRUNNER_OUTPUT)/
> endif
>
> +$(OUTPUT)/$(TRUNNER_BINARY): LDLIBS += $$($(TRUNNER_BASE_NAME)-LDLIBS)
> +$(OUTPUT)/$(TRUNNER_BINARY): LDFLAGS += $$($(TRUNNER_BASE_NAME)-LDFLAGS)
is there any reason why you need to have this blah-LDFLAGS convention
and then applying that with extra pass, instead of just writing
$(OUTPUT)/$(TRUNNER_BINARY): LDFLAGS += $(LLVM_LDFLAGS)
I'm not sure I understand the need for extra logical hops to do this
> +
> # some X.test.o files have runtime dependencies on Y.bpf.o files
> $(OUTPUT)/$(TRUNNER_BINARY): | $(TRUNNER_BPF_OBJS)
>
> @@ -636,7 +681,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
> $(TRUNNER_BPFTOOL) \
> | $(TRUNNER_BINARY)-extras
> $$(call msg,BINARY,,$$@)
> - $(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
> + $(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) $$(LDFLAGS) -o $$@
> $(Q)$(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@
> $(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/$(USE_BOOTSTRAP)bpftool \
> $(OUTPUT)/$(if $2,$2/)bpftool
> @@ -655,6 +700,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \
> cap_helpers.c \
> unpriv_helpers.c \
> netlink_helpers.c \
> + jit_disasm_helpers.c \
> test_loader.c \
> xsk.c \
> disasm.c \
> @@ -797,7 +843,8 @@ EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
> $(addprefix $(OUTPUT)/,*.o *.d *.skel.h *.lskel.h *.subskel.h \
> no_alu32 cpuv4 bpf_gcc bpf_testmod.ko \
> bpf_test_no_cfi.ko \
> - liburandom_read.so)
> + liburandom_read.so) \
> + $(OUTPUT)/FEATURE-DUMP.selftests
>
> .PHONY: docs docs-clean
[...]
