__jited_x86 test tag to check x86 assembly after jit

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 <[email protected]>

---
  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

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +30 -0

+static int disasm_one_func(FILE *text_out, uint8_t *image, __u32 len)
+{
+	char *label, *colon, *triple = NULL;
+	LLVMDisasmContextRef ctx = NULL;
+	struct local_labels labels = {};
+	__u32 *label_pc, pc;
+	int i, cnt, err = 0;
+	char buf[256];
+
+	triple = LLVMGetDefaultTargetTriple();
+	ctx = LLVMCreateDisasm(triple, &labels, 0, NULL, lookup_symbol);
+	if (!ASSERT_OK_PTR(ctx, "LLVMCreateDisasm")) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	cnt = LLVMSetDisasmOptions(ctx, LLVMDisassembler_Option_PrintImmHex);
+	if (!ASSERT_EQ(cnt, 1, "LLVMSetDisasmOptions")) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	/* discover labels */
+	labels.prog_len = len;
+	pc = 0;
+	while (pc < len) {
+		cnt = disasm_insn(ctx, image, len, pc, buf, 1);
+		if (cnt < 0) {
+			err = cnt;
+			goto out;

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +34 -0

+
+	/* now print with labels */
+	labels.print_phase = true;
+	pc = 0;
+	while (pc < len) {
+		cnt = disasm_insn(ctx, image, len, pc, buf, sizeof(buf));
+		if (cnt < 0) {
+			err = cnt;
+			goto out;
+		}
+		label_pc = bsearch(&pc, labels.pcs, labels.cnt, sizeof(*labels.pcs), cmp_u32);
+		label = "";
+		colon = "";
+		if (label_pc) {
+			label = labels.names[label_pc - labels.pcs];
+			colon = ":";
+		}
+		fprintf(text_out, "%x:\t", pc);
+		for (i = 0; i < cnt; ++i)
+			fprintf(text_out, "%02x ", image[pc + i]);
+		for (i = cnt * 3; i < 12 * 3; ++i)
+			fputc(' ', text_out);
+		fprintf(text_out, "%s%s%s\n", label, colon, buf);
+		pc += cnt;
+	}
+
+out:
+	if (triple)
+		LLVMDisposeMessage(triple);
+	if (ctx)
+		LLVMDisasmDispose(ctx);
+	return err;
+}
+

Review

Quoted 6 lines

(quoted diff) +6 -0

+	/* GCC can't figure max bound for i and thus reports possible truncation */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-truncation"
+	for (i = 0; i < labels.cnt; ++i)
+		snprintf(labels.names[i], sizeof(labels.names[i]), "L%d", i);
+#pragma GCC diagnostic pop

"-Wformat-truncation" is only available for llvm >= 18. One of my build with llvm15 has the following warning/error:

jit_disasm_helpers.c:113:32: error: unknown warning group '-Wformat-truncation', ignored [-Werror,-Wunknown-warning-option] #pragma GCC diagnostic ignored "-Wformat-truncation"

Maybe you want to guard with proper clang version? Not sure on gcc side when "-Wformat-truncation" is supported.

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 <[email protected]>

---
  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

tools/testing/selftests/bpf/.gitignore +1 -0

index 8f14d8faeb0b..7de4108771a0 100644
@@ -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

tools/testing/selftests/bpf/Makefile +49 -2

index f54185e96a95..b1a52739d9e7 100644
@@ -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)
+
  # 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
 

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +3 -0

new file mode 100644
index 000000000000..ae704f7c5ee7
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +22 -0

+
+static bool llvm_initialized;
+
+struct local_labels {
+	bool print_phase;
+	__u32 prog_len;
+	__u32 cnt;
+	__u32 pcs[MAX_LOCAL_LABELS];
+	char names[MAX_LOCAL_LABELS][4];
+};
+
+/* Depending on labels->print_phase:
+ * - if false: record save jump labels within the program in labels->pcs;
+ * - if true: if ref_value is in labels->pcs, return corresponding labels->name
+ */
+static const char *lookup_symbol(void *data, uint64_t ref_value, uint64_t *ref_type,
+				 uint64_t ref_pc, const char **ref_name)
+{
+	struct local_labels *labels = data;
+	uint64_t type = *ref_type;
+	int i;
+

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +29 -0

+	return NULL;
+}
+
+static int disasm_insn(LLVMDisasmContextRef ctx, uint8_t *image, __u32 len, __u32 pc,
+		       char *buf, __u32 buf_sz)
+{
+	int i, cnt;
+
+	cnt = LLVMDisasmInstruction(ctx, image + pc, len - pc, pc,
+				    buf, buf_sz);
+	if (cnt > 0)
+		return cnt;
+	PRINT_FAIL("Can't disasm instruction at offset %d:", pc);
+	for (i = 0; i < 16 && pc + i < len; ++i)
+		printf(" %02x", image[pc + i]);
+	printf("\n");
+	return -EINVAL;
+}
+
+static int cmp_u32(const void *_a, const void *_b)
+{
+	__u32 a = *(__u32 *)_a;
+	__u32 b = *(__u32 *)_b;
+
+	if (a < b)
+		return -1;
+	if (a > b)
+		return 1;
+	return 0;

tools/testing/selftests/bpf/jit_disasm_helpers.c new file +26 -0

+
+	triple = LLVMGetDefaultTargetTriple();
+	ctx = LLVMCreateDisasm(triple, &labels, 0, NULL, lookup_symbol);
+	if (!ASSERT_OK_PTR(ctx, "LLVMCreateDisasm")) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	cnt = LLVMSetDisasmOptions(ctx, LLVMDisassembler_Option_PrintImmHex);
+	if (!ASSERT_EQ(cnt, 1, "LLVMSetDisasmOptions")) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	/* discover labels */
+	labels.prog_len = len;
+	pc = 0;
+	while (pc < len) {
+		cnt = disasm_insn(ctx, image, len, pc, buf, 1);
+		if (cnt < 0) {
+			err = cnt;
+			goto out;
+		}
+		pc += cnt;
+	}
+	qsort(labels.pcs, labels.cnt, sizeof(*labels.pcs), cmp_u32);

Quoted 2 lines · Eduard Zingerman

int get_jited_program_text(int fd, char *text, size_t text_sz)

You need to give some context before the above function signature.

Quoted 11 lines

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.

Quoted 3 lines

[1] commit eb9d1acf634b ("bpftool: Add LLVM as default library for disassembling JIT-ed programs")

Signed-off-by Eduard Zingerman <[email protected]>

LGTM except a few nits below.

Acked-by Yonghong Song <[email protected]>

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 <[email protected]>

---
 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

tools/testing/selftests/bpf/.gitignore +1 -0

index 8f14d8faeb0b..7de4108771a0 100644
@@ -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

tools/testing/selftests/bpf/Makefile +42 -0

index f54185e96a95..b1a52739d9e7 100644
@@ -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)                            \

tools/testing/selftests/bpf/Makefile +5 -2

+
 # 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

tools/testing/selftests/bpf/Makefile ifneq ($2:$(OUTPUT),:$(shell pwd)) · line 673

Quoted 6 lines

tools/testing/selftests/bpf/Makefile +2 -0

@@ -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

On Thu, 2024-08-15 at 14:06 -0700, Andrii Nakryiko wrote:

[...]

Quoted 10 lines

tools/testing/selftests/bpf/Makefile ifneq ($2:$(OUTPUT),:$(shell pwd)) · line 673

Quoted 6 lines

tools/testing/selftests/bpf/Makefile +2 -0

@@ -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

Quoted 3 lines

$(OUTPUT)/$(TRUNNER_BINARY): LDFLAGS += $(LLVM_LDFLAGS)

I'm not sure I understand the need for extra logical hops to do this

No real reason, that's how it is organized in bpftool makefile, monkey see, monkey do. Will combine to have single LDFLAGS change.

[...]

tools/testing/selftests/bpf/Makefile ifneq ($2:$(OUTPUT),:$(shell pwd)) · line 673

Quoted 6 lines

tools/testing/selftests/bpf/Makefile +2 -0

@@ -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

Quoted 13 lines

tools/testing/selftests/bpf/Makefile +2 -1

+
 # 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

On Thu, 2024-08-15 at 14:06 -0700, Andrii Nakryiko wrote:

[...]

Quoted 10 lines

tools/testing/selftests/bpf/Makefile ifneq ($2:$(OUTPUT),:$(shell pwd)) · line 673

Quoted 6 lines

tools/testing/selftests/bpf/Makefile +2 -0

@@ -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

Quoted 3 lines

$(OUTPUT)/$(TRUNNER_BINARY): LDFLAGS += $(LLVM_LDFLAGS)

I'm not sure I understand the need for extra logical hops to do this

Sorry, I missed this detail on Thursday.
The $$($(TRUNNER_BASE_NAME)-LDLIBS) thing is needed to avoid linking LLVM dependencies for test_maps. Still, I can remove it if you think this does not matter.

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/bpf/Makefile +2 -1

+
 # 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

[...]

Allow to verify jit behaviour by writing tests as below:

    SEC("tp")
    __jit_x86("endbr64")
    __jit_x86("movabs $0x.*,%r9")
    __jit_x86("add    %gs:0x.*,%r9")
    __jit_x86("mov    $0x1,%edi")
    __jit_x86("mov    %rdi,-0x8(%r9)")
    __jit_x86("mov    -0x8(%r9),%rdi")
    __jit_x86("xor    %eax,%eax")
    __jit_x86("lock xchg %rax,-0x8(%r9)")
    __jit_x86("lock xadd %rax,-0x8(%r9)")
    __naked void stack_access_insns(void)
    {
        asm volatile (... ::: __clobber_all);
    }

Use regular expressions by default, use basic regular expressions class in order to avoid escaping symbols like $(), often used in AT&T disassembly syntax.

Signed-off-by Eduard Zingerman <[email protected]>

---
 tools/testing/selftests/bpf/progs/bpf_misc.h |   2 +
 tools/testing/selftests/bpf/test_loader.c    | 156 +++++++++++++------
 2 files changed, 112 insertions(+), 46 deletions(-)

tools/testing/selftests/bpf/test_loader.c +11 -1

+                       printf("%s:SKIP: jited programs disassembly is not supported,\n", __func__);
+                       printf("%s:SKIP: tests are built w/o LLVM development libs\n", __func__);
+                       test__skip();
+                       goto tobj_cleanup;
+               }
+               if (!ASSERT_EQ(err, 0, "get_jited_program_text"))
+                       goto tobj_cleanup;
+               emit_jited(tester->log_buf, false /*force*/);
+               validate_msgs(tester->log_buf, &subspec->jited[arch], emit_jited);
+       }
+
        if (should_do_test_run(spec, subspec)) {
                /* For some reason test_verifier executes programs
                 * with all capabilities restored. Do the same here.
--

2.45.2

tools/testing/selftests/bpf/test_loader.c void run_subtest(struct test_loader *tester, · line 872

Quoted 8 lines

tools/testing/selftests/bpf/test_loader.c +4 -0

@@ -817,6 +866,21 @@ void run_subtest(struct test_loader *tester,
                validate_msgs(tester->log_buf, &subspec->expect_xlated, emit_xlated);
        }
 
+       if (arch > 0 && subspec->jited[arch].cnt) {
+               err = get_jited_program_text(bpf_program__fd(tprog),
+                                            tester->log_buf, tester->log_buf_sz);
+               if (err == -ENOTSUP) {

nit: let's use EOPNOTSUPP, ENOTSUP is internal Linux error

A program calling sub-program which does a tail call. The idea is to verify instructions generated by jit for tail calls:
- in program and sub-program prologues;
- for subprogram call instruction;
- for tail call itself.

Signed-off-by Eduard Zingerman <[email protected]>

---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_tailcall_jit.c         | 103 ++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c

tools/testing/selftests/bpf/prog_tests/verifier.c +2 -0

index f8f546eba488..cf3662dbd24f 100644
@@ -75,6 +75,7 @@
 #include "verifier_stack_ptr.skel.h"
 #include "verifier_subprog_precision.skel.h"
 #include "verifier_subreg.skel.h"
+#include "verifier_tailcall_jit.skel.h"
 #include "verifier_typedef.skel.h"
 #include "verifier_uninit.skel.h"
 #include "verifier_unpriv.skel.h"

@@ -198,6 +199,7 @@ void test_verifier_spin_lock(void)            { RUN(verifier_spin_lock); }
 void test_verifier_stack_ptr(void)            { RUN(verifier_stack_ptr); }
 void test_verifier_subprog_precision(void)    { RUN(verifier_subprog_precision); }
 void test_verifier_subreg(void)               { RUN(verifier_subreg); }
+void test_verifier_tailcall_jit(void)         { RUN(verifier_tailcall_jit); }
 void test_verifier_typedef(void)              { RUN(verifier_typedef); }
 void test_verifier_uninit(void)               { RUN(verifier_uninit); }
 void test_verifier_unpriv(void)               { RUN(verifier_unpriv); }

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +28 -0

new file mode 100644
index 000000000000..1a09c76d7be0
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+int main(void);
+
+struct {
+       __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+       __uint(max_entries, 1);
+       __uint(key_size, sizeof(__u32));
+       __array(values, void (void));
+} jmp_table SEC(".maps") = {
+       .values = {
+               [0] = (void *) &main,
+       },
+};
+
+__noinline __auxiliary
+static __naked int sub(void)
+{
+       asm volatile (
+       "r2 = %[jmp_table] ll;"
+       "r3 = 0;"
+       "call 12;"
+       "exit;"
+       :
+       : __imm_addr(jmp_table)

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +65 -1

+/* tail call prologue for program:
+ * - establish memory location for tail call counter at &rbp[-8];
+ * - spill tail_call_cnt_ptr at &rbp[-16];
+ * - expect tail call counter to be passed in rax;
+ * - for entry program rax is a raw counter, value < 33;
+ * - for tail called program rax is tail_call_cnt_ptr (value > 33).
+ */
+__jit_x86("    endbr64")
+__jit_x86("    cmpq    $0x21, %rax")
+__jit_x86("    ja      L0")
+__jit_x86("    pushq   %rax")
+__jit_x86("    movq    %rsp, %rax")
+__jit_x86("    jmp     L1")
+__jit_x86("L0: pushq   %rax")                  /* rbp[-8]  = rax         */
+__jit_x86("L1: pushq   %rax")                  /* rbp[-16] = rax         */
+/* on subprogram call restore rax to be tail_call_cnt_ptr from rbp[-16]
+ * (cause original rax might be clobbered by this point)
+ */
+__jit_x86("    movq    -0x10(%rbp), %rax")
+__jit_x86("    callq   0x[0-9a-f]\\+")         /* call to sub()          */
+__jit_x86("    xorl    %eax, %eax")
+__jit_x86("    leave")
+__jit_x86("    retq")
+/* subprogram entry for sub(), regular function prologue */
+__jit_x86("    endbr64")
+__jit_x86("    nopl    (%rax,%rax)")
+__jit_x86("    nopl    (%rax)")
+__jit_x86("    pushq   %rbp")
+__jit_x86("    movq    %rsp, %rbp")
+/* tail call prologue for subprogram address of tail call counter
+ * stored at rbp[-16].
+ */
+__jit_x86("    endbr64")
+__jit_x86("    pushq   %rax")                  /* rbp[-8]  = rax          */
+__jit_x86("    pushq   %rax")                  /* rbp[-16] = rax          */
+__jit_x86("    movabsq $-0x[0-9a-f]\\+, %rsi") /* r2 = &jmp_table         */
+__jit_x86("    xorl    %edx, %edx")            /* r3 = 0                  */
+/* bpf_tail_call implementation:
+ * - load tail_call_cnt_ptr from rbp[-16];
+ * - if *tail_call_cnt_ptr < 33, increment it and jump to target;
+ * - otherwise do nothing.
+ */
+__jit_x86("    movq    -0x10(%rbp), %rax")
+__jit_x86("    cmpq    $0x21, (%rax)")
+__jit_x86("    jae     L0")
+__jit_x86("    nopl    (%rax,%rax)")
+__jit_x86("    addq    $0x1, (%rax)")          /* *tail_call_cnt_ptr += 1 */
+__jit_x86("    popq    %rax")
+__jit_x86("    popq    %rax")
+__jit_x86("    jmp     0x[0-9a-f]\\+")         /* jump to tail call tgt   */
+__jit_x86("L0: leave")
+__jit_x86("    retq")
+SEC("tc")
+__naked int main(void)
+{
+       asm volatile (
+       "call %[sub];"
+       "r0 = 0;"
+       "exit;"
+       :
+       : __imm(sub)
+       : __clobber_all);
+}
+
+char __license[] SEC("license") = "GPL";
--

2.45.2

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c line 38

Quoted 6 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +6 -0

+/* program entry for main(), regular function prologue */
+__jit_x86("    endbr64")
+__jit_x86("    nopl    (%rax,%rax)")
+__jit_x86("    xorq    %rax, %rax")
+__jit_x86("    pushq   %rbp")
+__jit_x86("    movq    %rsp, %rbp")

I'm a bit too lazy to fish it out of the code, so I'll just ask. Does matching of __jit_x86() string behave in the same way as __msg(). I.e., there could be unexpected lines that would be skipped, as long as we find a match for each __jit_x86() one?

Isn't that a bit counter-intuitive and potentially dangerous behavior for checking disassembly? If my assumption is correct, maybe we should add some sort of __jit_x86("...") placeholder to explicitly mark that we allow some amount of lines to be skipped, but otherwise be strict and require matching line-by-line?

On Thu, 2024-08-15 at 14:15 -0700, Andrii Nakryiko wrote:

[...]

Quoted 11 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c line 38

Quoted 6 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +6 -0

+/* program entry for main(), regular function prologue */
+__jit_x86("    endbr64")
+__jit_x86("    nopl    (%rax,%rax)")
+__jit_x86("    xorq    %rax, %rax")
+__jit_x86("    pushq   %rbp")
+__jit_x86("    movq    %rsp, %rbp")

I'm a bit too lazy to fish it out of the code, so I'll just ask. Does matching of __jit_x86() string behave in the same way as __msg(). I.e., there could be unexpected lines that would be skipped, as long as we find a match for each __jit_x86() one?

Yes, behaves same way as __msg().

Quoted 5 lines

Isn't that a bit counter-intuitive and potentially dangerous behavior for checking disassembly? If my assumption is correct, maybe we should add some sort of __jit_x86("...") placeholder to explicitly mark that we allow some amount of lines to be skipped, but otherwise be strict and require matching line-by-line?

This is a valid concern.
What you suggest with "..." looks good.

Another option is to follow llvm-lit conventions and add __jit_x86_next("<whatever>"), which would only match if pattern is on line below any previous match.
(And later add __jit_x86_next_not, and make these *_not, *_next
variants accessible for every __msg-like macro).

Link https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-next-directive

[...]

Quoted 8 lines

Quoted 5 lines

Isn't that a bit counter-intuitive and potentially dangerous behavior for checking disassembly? If my assumption is correct, maybe we should add some sort of __jit_x86("...") placeholder to explicitly mark that we allow some amount of lines to be skipped, but otherwise be strict and require matching line-by-line?

This is a valid concern.
What you suggest with "..." looks good.

I'd add just that for now. _not and _next might be useful in the future, but meh.

On Thu, 2024-08-15 at 15:07 -0700, Andrii Nakryiko wrote:

[...]

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Isn't that a bit counter-intuitive and potentially dangerous behavior for checking disassembly? If my assumption is correct, maybe we should add some sort of __jit_x86("...") placeholder to explicitly mark that we allow some amount of lines to be skipped, but otherwise be strict and require matching line-by-line?

This is a valid concern.
What you suggest with "..." looks good.

Quoted 2 lines

I'd add just that for now. _not and _next might be useful in the future, but meh.

If we commit to "..." now and decide to add _not and _next in the future this would make __jit macro special. Which is not ideal, imo. (on the other hand, tests can always be adjusted).

[...]

It is already special with a different flavor of regex. And I assume we won't have that many jit-testing tests, so yeah, could be adjusted, if necessary. But just in general, while __msg() works with large verifier logs, __jit() is much more narrow-focused, so even if it behaves differently from __msg() I don't really see much difference.

But we also have __xlated() with similar semantics, so I'd say we should keep __jit() and __xlated() behaving similarly.

A program calling sub-program which does a tail call. The idea is to verify instructions generated by jit for tail calls:
- in program and sub-program prologues;
- for subprogram call instruction;
- for tail call itself.

Signed-off-by Eduard Zingerman <[email protected]>

---
  .../selftests/bpf/prog_tests/verifier.c       |   2 +
  .../bpf/progs/verifier_tailcall_jit.c         | 103 ++++++++++++++++++
  2 files changed, 105 insertions(+)

create mode 100644 tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c

tools/testing/selftests/bpf/prog_tests/verifier.c +2 -0

index f8f546eba488..cf3662dbd24f 100644
@@ -75,6 +75,7 @@
  #include "verifier_stack_ptr.skel.h"
  #include "verifier_subprog_precision.skel.h"
  #include "verifier_subreg.skel.h"
+#include "verifier_tailcall_jit.skel.h"
  #include "verifier_typedef.skel.h"
  #include "verifier_uninit.skel.h"
  #include "verifier_unpriv.skel.h"

@@ -198,6 +199,7 @@ void test_verifier_spin_lock(void)            { RUN(verifier_spin_lock); }
  void test_verifier_stack_ptr(void)            { RUN(verifier_stack_ptr); }
  void test_verifier_subprog_precision(void)    { RUN(verifier_subprog_precision); }
  void test_verifier_subreg(void)               { RUN(verifier_subreg); }
+void test_verifier_tailcall_jit(void)         { RUN(verifier_tailcall_jit); }
  void test_verifier_typedef(void)              { RUN(verifier_typedef); }
  void test_verifier_uninit(void)               { RUN(verifier_uninit); }
  void test_verifier_unpriv(void)               { RUN(verifier_unpriv); }

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +28 -0

new file mode 100644
index 000000000000..1a09c76d7be0
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+int main(void);
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+	__uint(max_entries, 1);
+	__uint(key_size, sizeof(__u32));
+	__array(values, void (void));
+} jmp_table SEC(".maps") = {
+	.values = {
+		[0] = (void *) &main,
+	},
+};
+
+__noinline __auxiliary
+static __naked int sub(void)
+{
+	asm volatile (
+	"r2 = %[jmp_table] ll;"
+	"r3 = 0;"
+	"call 12;"
+	"exit;"
+	:
+	: __imm_addr(jmp_table)

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +18 -0

+/* tail call prologue for program:
+ * - establish memory location for tail call counter at &rbp[-8];
+ * - spill tail_call_cnt_ptr at &rbp[-16];
+ * - expect tail call counter to be passed in rax;
+ * - for entry program rax is a raw counter, value < 33;
+ * - for tail called program rax is tail_call_cnt_ptr (value > 33).
+ */
+__jit_x86("	endbr64")
+__jit_x86("	cmpq	$0x21, %rax")
+__jit_x86("	ja	L0")
+__jit_x86("	pushq	%rax")
+__jit_x86("	movq	%rsp, %rax")
+__jit_x86("	jmp	L1")
+__jit_x86("L0:	pushq	%rax")			/* rbp[-8]  = rax         */
+__jit_x86("L1:	pushq	%rax")			/* rbp[-16] = rax         */
+/* on subprogram call restore rax to be tail_call_cnt_ptr from rbp[-16]
+ * (cause original rax might be clobbered by this point)
+ */

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c line 38

Quoted 7 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +7 -0

+__success
+/* program entry for main(), regular function prologue */
+__jit_x86("	endbr64")
+__jit_x86("	nopl	(%rax,%rax)")
+__jit_x86("	xorq	%rax, %rax")
+__jit_x86("	pushq	%rbp")
+__jit_x86("	movq	%rsp, %rbp")

How do we hanble multi architectures (x86, arm64, riscv64)?

Do we support the following?

__jit_x86(...)
__jit_x86(...)
...

__jit_arm64(...)
__jit_arm64(...)
...

__jit_riscv64(...)
__jit_riscv64(...)
...

Or we can use macro like

#ifdef __TARGET_ARCH_x86
__jit(...)
...
#elif defined(__TARGET_ARCH_arm64)
__jit(...)
...
#elif defined(...)

Or we can have

__arch_x86_64
__jit(...) // code for x86
...

__arch_arm64
__jit(...) // code for arm64
...

__arch_riscv
__jit(...) // code for riscv
...

For xlated, different archs could share the same code. Bot for jited code, different arch has different encoding, so we need to figure out a format suitable for multiple archs.

On Thu, 2024-08-15 at 14:32 -0700, Yonghong Song wrote:

[...]

Quoted 16 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c line 38

Quoted 7 lines

tools/testing/selftests/bpf/progs/verifier_tailcall_jit.c new file +7 -0

+__success
+/* program entry for main(), regular function prologue */
+__jit_x86("        endbr64")
+__jit_x86("        nopl    (%rax,%rax)")
+__jit_x86("        xorq    %rax, %rax")
+__jit_x86("        pushq   %rbp")
+__jit_x86("        movq    %rsp, %rbp")

How do we hanble multi architectures (x86, arm64, riscv64)?

Do we support the following?

__jit_x86(...)
__jit_x86(...)
...

Quoted 7 lines

__jit_arm64(...)
__jit_arm64(...)
...

__jit_riscv64(...)
__jit_riscv64(...)
...

^^^^
I was thinking about this variant (and this is how things are now implemented). Whenever there would be a need for that, just add one more arch specific macro.

Quoted 21 lines

Or we can use macro like

#ifdef __TARGET_ARCH_x86
__jit(...)
...
#elif defined(__TARGET_ARCH_arm64)
__jit(...)
...
#elif defined(...)

Or we can have

__arch_x86_64
__jit(...) // code for x86
...

__arch_arm64
__jit(...) // code for arm64
...

Quoted 3 lines

__arch_riscv
__jit(...) // code for riscv
...

This also looks good, and will work better with "*_next" and "*_not" variants if we are going to borrow from llvm-lit/FileCheck.

Quoted 4 lines

For xlated, different archs could share the same code. Bot for jited code, different arch has different encoding, so we need to figure out a format suitable for multiple archs.

I'll go with whatever way mailing list likes better.

[...]

Quoted 24 lines

Quoted 23 lines

Or we can use macro like

#ifdef __TARGET_ARCH_x86
__jit(...)
...
#elif defined(__TARGET_ARCH_arm64)
__jit(...)
...
#elif defined(...)

Or we can have

__arch_x86_64
__jit(...) // code for x86
...

__arch_arm64
__jit(...) // code for arm64
...

__arch_riscv
__jit(...) // code for riscv
...

Quoted 3 lines

This also looks good, and will work better with "*_next" and "*_not" variants if we are going to borrow from llvm-lit/FileCheck.

shorter __jit() and then arch-specific __arch_blah seems pretty clean, so if it's not too hard, let's do this.

BTW, in your implementation you are collecting expected messages for all specified architectures, but really there will always be just one valid subset. So maybe just discard all non-host architectures upfront during "parsing" of decl tags?