Merge branch 'bpftool-selftests-add-tests-for-c-dump-and-fix-a-dropped-type'

Ihor Solodrai says:

====================
bpftool, selftests: Add tests for C dump and fix a dropped type

"bpftool btf dump format c" generates the vmlinux.h that BPF programs
are built against, and it has no direct test coverage. Building
against the header is not a substitute for testing it, nothing a
compiler accepts would break the build.

Add selftests targeting C dump in particular.

While we are here, refactor btf_dump selftests to use a new
compare_text_to_expected() helper and become more efficient.

Fix a bug in bpftool dump that dropped a type when sorting.
---

v1->v2:
  * new patches (#1-#3) introducing compare_text_to_expected() helper
    and refactoring btf_dump selftests
  * bpftool dump selftest compares the output to the expected files
    exactly (Eduard)
  * use VALIDATE_RAW_BTF macro in the new test (Eduard)
  * drop v1 patch #4 ("selftests/bpf: Check that sorting preserves
    types in bpftool dump") as it became unnecessary
  * nit in the bpftool fix (Andrii)

v1: https://lore.kernel.org/bpf/20260820000627.3826188-1-ihor.solodrai@linux.dev/
====================

Link: https://patch.msgid.link/20260828215207.3105313-1-ihor.solodrai@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

tools/bpf/bpftool/btf.c

@@ -805,9 +805,13 @@ static int dump_btf_c(const struct btf *btf,
 
 		if (sort_dump)
 			datums = sort_btf_c(btf);
-		for (i = 1; i < cnt; i++) {
+		for (i = 0; i < cnt; i++) {
 			int idx = datums ? datums[i].index : i;
 
+			/* type ID 0 is void, skip it */
+			if (idx == 0)
+				continue;
+
 			err = btf_dump__dump_type(d, idx);
 			if (err)
 				goto done;

tools/testing/selftests/bpf/Makefile

@@ -119,7 +119,9 @@ TEST_GEN_PROGS += test_progs-cpuv4
 TEST_INST_SUBDIRS += cpuv4
 endif
 
-TEST_FILES = xsk_prereqs.sh $(wildcard progs/btf_dump_test_case_*.c)
+TEST_FILES = xsk_prereqs.sh $(wildcard progs/btf_dump_test_case_*.c)	\
+	     bpftool_btf_dump_sorted.expected					\
+	     bpftool_btf_dump_unsorted.expected
 
 # Order correspond to 'make run_tests' order
 TEST_PROGS := test_kmod.sh \
@@ -887,7 +889,9 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read				\
 		       ima_setup.sh 					\
 		       $(VERIFY_SIG_SETUP)				\
 		       $(wildcard progs/btf_dump_test_case_*.c)		\
-		       $(wildcard progs/*.bpf.o)
+		       $(wildcard progs/*.bpf.o)			\
+		       bpftool_btf_dump_sorted.expected			\
+		       bpftool_btf_dump_unsorted.expected
 TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
 TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS
 $(eval $(call DEFINE_TEST_RUNNER,test_progs))

tools/testing/selftests/bpf/bpftool_btf_dump_sorted.expected

@@ -0,0 +1,48 @@
+#ifndef __VMLINUX_H__
+#define __VMLINUX_H__
+
+#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
+#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
+#endif
+
+#ifndef __ksym
+#define __ksym __attribute__((section(".ksyms")))
+#endif
+
+#ifndef __weak
+#define __weak __attribute__((weak))
+#endif
+
+#ifndef __bpf_fastcall
+#if __has_attribute(bpf_fastcall)
+#define __bpf_fastcall __attribute__((bpf_fastcall))
+#else
+#define __bpf_fastcall
+#endif
+#endif
+
+enum {
+	E0 = 1,
+};
+
+struct holey {
+	int c;
+	long: 32;
+	long: 32;
+	int tail;
+};
+
+struct s {
+	int f;
+};
+
+
+/* BPF kfuncs */
+#ifndef BPF_NO_KFUNC_PROTOTYPES
+#endif
+
+#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
+#pragma clang attribute pop
+#endif
+
+#endif /* __VMLINUX_H__ */

tools/testing/selftests/bpf/bpftool_btf_dump_unsorted.expected

@@ -0,0 +1,48 @@
+#ifndef __VMLINUX_H__
+#define __VMLINUX_H__
+
+#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
+#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
+#endif
+
+#ifndef __ksym
+#define __ksym __attribute__((section(".ksyms")))
+#endif
+
+#ifndef __weak
+#define __weak __attribute__((weak))
+#endif
+
+#ifndef __bpf_fastcall
+#if __has_attribute(bpf_fastcall)
+#define __bpf_fastcall __attribute__((bpf_fastcall))
+#else
+#define __bpf_fastcall
+#endif
+#endif
+
+struct holey {
+	int c;
+	long: 32;
+	long: 32;
+	int tail;
+};
+
+enum {
+	E0 = 1,
+};
+
+struct s {
+	int f;
+};
+
+
+/* BPF kfuncs */
+#ifndef BPF_NO_KFUNC_PROTOTYPES
+#endif
+
+#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
+#pragma clang attribute pop
+#endif
+
+#endif /* __VMLINUX_H__ */

tools/testing/selftests/bpf/bpftool_helpers.c

@@ -53,6 +53,7 @@ static int run_command(char *args, char *output_buf, size_t output_max_len)
 	static char bpftool_path[PATH_MAX] = {};
 	bool suppress_output = !(output_buf && output_max_len);
 	char command[BPFTOOL_FULL_CMD_MAX_LEN];
+	size_t n;
 	FILE *f;
 	int ret;
 
@@ -68,8 +69,10 @@ static int run_command(char *args, char *output_buf, size_t output_max_len)
 	if (!f)
 		return 1;
 
-	if (!suppress_output)
-		fread(output_buf, 1, output_max_len, f);
+	if (!suppress_output) {
+		n = fread(output_buf, 1, output_max_len - 1, f);
+		output_buf[n] = '\0';
+	}
 	ret = pclose(f);
 
 	return ret;

tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c

@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <bpftool_helpers.h>
+#include <bpf/btf.h>
+#include <limits.h>
+#include <unistd.h>
+#include "btf_helpers.h"
+#include "testing_helpers.h"
+
+#define DUMP_BUF_SZ		(16 * 1024)
+
+#define EXPECTED_SORTED		"bpftool_btf_dump_sorted.expected"
+#define EXPECTED_UNSORTED	"bpftool_btf_dump_unsorted.expected"
+
+/*
+ *	struct holey {
+ *		int c;
+ *		<64-bit hole>
+ *		int tail;
+ *	};
+ *	enum { E0 = 1 };
+ *	struct s { int f; };
+ */
+static struct btf *mk_btf(void)
+{
+	struct btf *btf;
+
+	btf = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf, "new_empty"))
+		return NULL;
+
+	btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+	btf__add_int(btf, "long int", 4, BTF_INT_SIGNED);
+
+	btf__add_struct(btf, "holey", 16);
+	btf__add_field(btf, "c", 1, 0, 0);
+	btf__add_field(btf, "tail", 1, 96, 0);
+
+	btf__add_enum(btf, NULL, 4);
+	btf__add_enum_value(btf, "E0", 1);
+
+	btf__add_struct(btf, "s", 4);
+	btf__add_field(btf, "f", 1, 0, 0);
+
+	VALIDATE_RAW_BTF(
+		btf,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] INT 'long int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[3] STRUCT 'holey' size=16 vlen=2\n"
+		"\t'c' type_id=1 bits_offset=0\n"
+		"\t'tail' type_id=1 bits_offset=96",
+		"[4] ENUM '(anon)' encoding=UNSIGNED size=4 vlen=1\n"
+		"\t'E0' val=1",
+		"[5] STRUCT 's' size=4 vlen=1\n"
+		"\t'f' type_id=1 bits_offset=0");
+
+	return btf;
+}
+
+static int btf_to_tmpfile(const struct btf *btf, char *path)
+{
+	ssize_t written;
+	const void *raw;
+	__u32 sz;
+	int fd;
+
+	raw = btf__raw_data(btf, &sz);
+	if (!ASSERT_OK_PTR(raw, "raw_data"))
+		return -1;
+
+	snprintf(path, PATH_MAX, "/tmp/bpftool_btf_dump.XXXXXX");
+	fd = mkstemp(path);
+	if (!ASSERT_OK_FD(fd, "mkstemp_btf"))
+		return -1;
+
+	written = write(fd, raw, sz);
+	close(fd);
+	if (!ASSERT_EQ(written, sz, "write_btf")) {
+		unlink(path);
+		return -1;
+	}
+
+	return 0;
+}
+
+static char *dump_c(const char *btf_path, bool sorted)
+{
+	char args[MAX_BPFTOOL_CMD_LEN];
+	char *buf;
+	int err;
+
+	buf = malloc(DUMP_BUF_SZ);
+	if (!ASSERT_OK_PTR(buf, "alloc_dump"))
+		return NULL;
+
+	snprintf(args, sizeof(args), "btf dump file %s format c%s",
+		 btf_path, sorted ? "" : " unsorted");
+
+	err = get_bpftool_command_output(args, buf, DUMP_BUF_SZ);
+	if (!ASSERT_OK(err, "btf_dump_format_c")) {
+		free(buf);
+		return NULL;
+	}
+
+	return buf;
+}
+
+static char *read_expected(const char *path)
+{
+	char *buf = NULL;
+	size_t cap = 0;
+	FILE *f;
+	int err;
+
+	f = fopen(path, "r");
+	if (!f) {
+		err = errno;
+		PRINT_FAIL("can't open expected output '%s': errno %d\n", path, err);
+		return NULL;
+	}
+
+	/* no NUL in a generated header, so this reads to the end */
+	if (getdelim(&buf, &cap, '\0', f) < 0) {
+		err = errno;
+		PRINT_FAIL("can't read expected output '%s': errno %d\n", path, err);
+		free(buf);
+		buf = NULL;
+	}
+
+	fclose(f);
+	return buf;
+}
+
+static void test_dump(const char *btf_path, bool sorted)
+{
+	const char *exp_path;
+	char *dump, *exp;
+	int err;
+
+	exp_path = sorted ? EXPECTED_SORTED : EXPECTED_UNSORTED;
+
+	dump = dump_c(btf_path, sorted);
+	if (!dump)
+		return;
+
+	exp = read_expected(exp_path);
+	if (!exp)
+		goto out_dump;
+
+	err = compare_text_to_expected(dump, exp);
+	ASSERT_OK(err, sorted ? "cmp_sorted" : "cmp_unsorted");
+
+	free(exp);
+out_dump:
+	free(dump);
+}
+
+void test_bpftool_btf_dump(void)
+{
+	char path[PATH_MAX];
+	struct btf *btf;
+
+	btf = mk_btf();
+	if (!btf)
+		return;
+
+	if (btf_to_tmpfile(btf, path))
+		goto out_btf;
+
+	if (test__start_subtest("c_sorted"))
+		test_dump(path, true);
+	if (test__start_subtest("c_unsorted"))
+		test_dump(path, false);
+
+	unlink(path);
+out_btf:
+	btf__free(btf);
+}

tools/testing/selftests/bpf/prog_tests/btf_dump.c

@@ -1,8 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include <bpf/btf.h>
-
-static int duration = 0;
+#include "testing_helpers.h"
 
 void btf_dump_printf(void *ctx, const char *fmt, va_list args)
 {
@@ -45,12 +44,75 @@ static int btf_dump_all_types(const struct btf *btf, void *ctx)
 	return err;
 }
 
+/*
+ * Expected output is embedded in the test case source, between
+ * START-EXPECTED-OUTPUT and END-EXPECTED-OUTPUT markers. A region is either
+ * plain C, where a declaration doubles as its own expectation, or C wrapped in
+ * a block comment, where the rendered form differs from the source. All
+ * regions of a file concatenate into one expectation, compared against one
+ * whole-file dump.
+ *
+ * Returns a malloc'd buffer for the caller to free, or NULL on failure.
+ */
+static char *read_expected_output(const char *path)
+{
+	size_t out_sz = 0, line_cap = 0;
+	char *out = NULL, *line = NULL;
+	bool in_region = false;
+	FILE *f, *out_file;
+
+	f = fopen(path, "r");
+	if (!f)
+		return NULL;
+
+	out_file = open_memstream(&out, &out_sz);
+	if (!out_file) {
+		fclose(f);
+		return NULL;
+	}
+
+	while (getline(&line, &line_cap, f) > 0) {
+		const char *p;
+
+		if (strstr(line, "START-EXPECTED-OUTPUT")) {
+			in_region = true;
+			continue;
+		}
+		if (strstr(line, "END-EXPECTED-OUTPUT"))
+			in_region = false;
+		if (!in_region)
+			continue;
+
+		p = line + strspn(line, " \t");
+
+		/* opening or closing line of a commented out region */
+		if (!strncmp(p, "/*", 2) || !strncmp(p, "*/", 2))
+			continue;
+
+		/*
+		 * Only a '*' directly after the indentation is a comment
+		 * prefix. Without one the line is taken as it is, leading
+		 * whitespace included.
+		 */
+		p = *p == '*' ? p + 1 : line;
+
+		fputs(p, out_file);
+	}
+
+	free(line);
+	fclose(f);
+	fclose(out_file);
+	return out;
+}
+
 static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
 {
-	char test_file[256], out_file[256], diff_cmd[1024];
+	char *dump = NULL, *expected = NULL;
 	struct btf *btf = NULL;
-	int err = 0, fd = -1;
-	FILE *f = NULL;
+	char test_file[256];
+	size_t dump_sz = 0;
+	int err = 0;
+	FILE *f;
 
 	snprintf(test_file, sizeof(test_file), "%s.bpf.o", t->file);
 
@@ -69,29 +131,21 @@ static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
 	if (!t->known_ptr_sz) {
 		btf__set_pointer_size(btf, 8);
 	} else {
-		CHECK(btf__pointer_size(btf) != 8, "ptr_sz", "exp %d, got %zu\n",
-		      8, btf__pointer_size(btf));
+		size_t ptr_sz = btf__pointer_size(btf);
+
+		ASSERT_EQ(ptr_sz, (size_t)8, "ptr_sz");
 	}
 
-	snprintf(out_file, sizeof(out_file), "/tmp/%s.output.XXXXXX", t->file);
-	fd = mkstemp(out_file);
-	if (!ASSERT_GE(fd, 0, "create_tmp")) {
-		err = fd;
-		goto done;
-	}
-	f = fdopen(fd, "w");
-	if (CHECK(f == NULL, "open_tmp",  "failed to open file: %s(%d)\n",
-		  strerror(errno), errno)) {
-		close(fd);
+	f = open_memstream(&dump, &dump_sz);
+	if (!ASSERT_OK_PTR(f, "open_memstream")) {
+		err = -errno;
 		goto done;
 	}
 
 	err = btf_dump_all_types(btf, f);
 	fclose(f);
-	close(fd);
-	if (CHECK(err, "btf_dump", "failure during C dumping: %d\n", err)) {
+	if (!ASSERT_OK(err, "btf_dump"))
 		goto done;
-	}
 
 	snprintf(test_file, sizeof(test_file), "progs/%s.c", t->file);
 	if (access(test_file, R_OK) == -1)
@@ -100,28 +154,23 @@ static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
 		 * without preserving the directory structure.
 		 */
 		snprintf(test_file, sizeof(test_file), "%s.c", t->file);
-	/*
-	 * Diff test output and expected test output, contained between
-	 * START-EXPECTED-OUTPUT and END-EXPECTED-OUTPUT lines in test case.
-	 * For expected output lines, everything before '*' is stripped out.
-	 * Also lines containing comment start and comment end markers are
-	 * ignored. 
-	 */
-	snprintf(diff_cmd, sizeof(diff_cmd),
-		 "awk '/START-EXPECTED-OUTPUT/{out=1;next} "
-		 "/END-EXPECTED-OUTPUT/{out=0} "
-		 "/\\/\\*|\\*\\//{next} " /* ignore comment start/end lines */
-		 "out {sub(/^[ \\t]*\\*/, \"\"); print}' '%s' | diff -u - '%s'",
-		 test_file, out_file);
-	err = system(diff_cmd);
-	if (CHECK(err, "diff",
-		  "differing test output, output=%s, err=%d, diff cmd:\n%s\n",
-		  out_file, err, diff_cmd))
-		goto done;
 
-	remove(out_file);
+	expected = read_expected_output(test_file);
+	if (!ASSERT_OK_PTR(expected, "read_expected_output")) {
+		err = -errno;
+		goto done;
+	}
+
+	/*
+	 * The mismatch has already been reported, so this only has to
+	 * register the failure. ASSERT_OK() would append a stale errno to it.
+	 */
+	err = compare_text_to_expected(dump, expected);
+	ASSERT_EQ(err, 0, "compare_text_to_expected");
 
 done:
+	free(expected);
+	free(dump);
 	btf__free(btf);
 	return err;
 }

tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c

@@ -179,3 +179,26 @@ void test_prog_tests_framework_expected_msgs(void)
 		}
 	}
 }
+
+void test_prog_tests_framework_compare_text(void)
+{
+	int err;
+
+	if (test__start_subtest("compare_text_match")) {
+		err = compare_text_to_expected("same\n", "same\n");
+		ASSERT_EQ(err, 0, "match_rc");
+		test__end_subtest();
+	}
+
+	if (test__start_subtest("compare_text_mismatch")) {
+		err = compare_text_to_expected("line two\n", "line one\n");
+		fflush(stdout);
+
+		ASSERT_EQ(err, -1, "mismatch_rc");
+		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
+				  "diff_has_expected");
+		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
+				  "diff_has_actual");
+		test__end_subtest();
+	}
+}

tools/testing/selftests/bpf/testing_helpers.c

@@ -534,3 +534,46 @@ int stack_mprotect(void)
 		       PROT_READ | PROT_WRITE | PROT_EXEC);
 	return ret;
 }
+
+int compare_text_to_expected(const char *actual, const char *expected)
+{
+	char exp_path[] = "/tmp/selftest_expected.XXXXXX";
+	char act_path[] = "/tmp/selftest_actual.XXXXXX";
+	char buf[512], cmd[128];
+	int exp_fd, act_fd;
+	FILE *p;
+
+	if (!strcmp(actual, expected))
+		return 0;
+
+	exp_fd = mkstemp(exp_path);
+	act_fd = mkstemp(act_path);
+	if (exp_fd < 0 || act_fd < 0) {
+		fprintf(stdout, "output differs, no temp file for a diff\n");
+		goto out;
+	}
+
+	dprintf(exp_fd, "%s", expected);
+	dprintf(act_fd, "%s", actual);
+
+	snprintf(cmd, sizeof(cmd), "diff -u '%s' '%s'", exp_path, act_path);
+	p = popen(cmd, "r");
+	if (!p) {
+		fprintf(stdout, "output differs, '%s' did not run\n", cmd);
+		goto out;
+	}
+	while (fgets(buf, sizeof(buf), p))
+		fputs(buf, stdout);
+	pclose(p);
+
+out:
+	if (exp_fd >= 0) {
+		close(exp_fd);
+		unlink(exp_path);
+	}
+	if (act_fd >= 0) {
+		close(act_fd);
+		unlink(act_path);
+	}
+	return -1;
+}

tools/testing/selftests/bpf/testing_helpers.h

@@ -61,4 +61,7 @@ int testing_prog_flags(void);
 bool is_jit_enabled(void);
 int stack_mprotect(void);
 
+/* Runs diff(1) on mismatch */
+int compare_text_to_expected(const char *actual, const char *expected);
+
 #endif /* __TESTING_HELPERS_H */