Merge branch 'btf2c-padding'

Andrii Nakryiko says:

====================
Fix BTF-to-C logic of handling padding at the end of a struct. Fix existing
test that should have captured this. Also move test_btf_dump into a test_progs
test to leverage common infrastructure.
====================
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

tools/lib/bpf/btf_dump.c

@@ -876,7 +876,6 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
 	__u16 vlen = btf_vlen(t);
 
 	packed = is_struct ? btf_is_struct_packed(d->btf, id, t) : 0;
-	align = packed ? 1 : btf_align_of(d->btf, id);
 
 	btf_dump_printf(d, "%s%s%s {",
 			is_struct ? "struct" : "union",
@@ -906,6 +905,13 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
 		btf_dump_printf(d, ";");
 	}
 
+	/* pad at the end, if necessary */
+	if (is_struct) {
+		align = packed ? 1 : btf_align_of(d->btf, id);
+		btf_dump_emit_bit_padding(d, off, t->size * 8, 0, align,
+					  lvl + 1);
+	}
+
 	if (vlen)
 		btf_dump_printf(d, "\n");
 	btf_dump_printf(d, "%s}", pfx(lvl));

tools/testing/selftests/bpf/Makefile

@@ -29,7 +29,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
 	test_cgroup_storage test_select_reuseport test_section_names \
 	test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
-	test_btf_dump test_cgroup_attach xdping
+	test_cgroup_attach xdping
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)

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

-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <errno.h>
-#include <linux/err.h>
-#include <btf.h>
-
-#define CHECK(condition, format...) ({					\
-	int __ret = !!(condition);					\
-	if (__ret) {							\
-		fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__);	\
-		fprintf(stderr, format);				\
-	}								\
-	__ret;								\
-})
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+static int duration = 0;
 
 void btf_dump_printf(void *ctx, const char *fmt, va_list args)
 {
 	vfprintf(ctx, fmt, args);
 }
 
-struct btf_dump_test_case {
+static struct btf_dump_test_case {
 	const char *name;
+	const char *file;
 	struct btf_dump_opts opts;
 } btf_dump_test_cases[] = {
-	{.name = "btf_dump_test_case_syntax", .opts = {}},
-	{.name = "btf_dump_test_case_ordering", .opts = {}},
-	{.name = "btf_dump_test_case_padding", .opts = {}},
-	{.name = "btf_dump_test_case_packing", .opts = {}},
-	{.name = "btf_dump_test_case_bitfields", .opts = {}},
-	{.name = "btf_dump_test_case_multidim", .opts = {}},
-	{.name = "btf_dump_test_case_namespacing", .opts = {}},
+	{"btf_dump: syntax", "btf_dump_test_case_syntax", {}},
+	{"btf_dump: ordering", "btf_dump_test_case_ordering", {}},
+	{"btf_dump: padding", "btf_dump_test_case_padding", {}},
+	{"btf_dump: packing", "btf_dump_test_case_packing", {}},
+	{"btf_dump: bitfields", "btf_dump_test_case_bitfields", {}},
+	{"btf_dump: multidim", "btf_dump_test_case_multidim", {}},
+	{"btf_dump: namespacing", "btf_dump_test_case_namespacing", {}},
 };
 
 static int btf_dump_all_types(const struct btf *btf,
@@ -55,55 +45,51 @@ static int btf_dump_all_types(const struct btf *btf,
 	return err;
 }
 
-int test_btf_dump_case(int n, struct btf_dump_test_case *test_case)
+static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
 {
 	char test_file[256], out_file[256], diff_cmd[1024];
 	struct btf *btf = NULL;
 	int err = 0, fd = -1;
 	FILE *f = NULL;
 
-	fprintf(stderr, "Test case #%d (%s): ", n, test_case->name);
-
-	snprintf(test_file, sizeof(test_file), "%s.o", test_case->name);
+	snprintf(test_file, sizeof(test_file), "%s.o", t->file);
 
 	btf = btf__parse_elf(test_file, NULL);
-	if (CHECK(IS_ERR(btf),
+	if (CHECK(IS_ERR(btf), "btf_parse_elf",
 	    "failed to load test BTF: %ld\n", PTR_ERR(btf))) {
 		err = -PTR_ERR(btf);
 		btf = NULL;
 		goto done;
 	}
 
-	snprintf(out_file, sizeof(out_file),
-		 "/tmp/%s.output.XXXXXX", test_case->name);
+	snprintf(out_file, sizeof(out_file), "/tmp/%s.output.XXXXXX", t->file);
 	fd = mkstemp(out_file);
-	if (CHECK(fd < 0, "failed to create temp output file: %d\n", fd)) {
+	if (CHECK(fd < 0, "create_tmp", "failed to create file: %d\n", fd)) {
 		err = fd;
 		goto done;
 	}
 	f = fdopen(fd, "w");
-	if (CHECK(f == NULL, "failed to open temp output file: %s(%d)\n",
+	if (CHECK(f == NULL, "open_tmp",  "failed to open file: %s(%d)\n",
 		  strerror(errno), errno)) {
 		close(fd);
 		goto done;
 	}
 
-	test_case->opts.ctx = f;
-	err = btf_dump_all_types(btf, &test_case->opts);
+	t->opts.ctx = f;
+	err = btf_dump_all_types(btf, &t->opts);
 	fclose(f);
 	close(fd);
-	if (CHECK(err, "failure during C dumping: %d\n", err)) {
+	if (CHECK(err, "btf_dump", "failure during C dumping: %d\n", err)) {
 		goto done;
 	}
 
-	snprintf(test_file, sizeof(test_file), "progs/%s.c", test_case->name);
+	snprintf(test_file, sizeof(test_file), "progs/%s.c", t->file);
 	if (access(test_file, R_OK) == -1)
 		/*
 		 * When the test is run with O=, kselftest copies TEST_FILES
 		 * without preserving the directory structure.
 		 */
-		snprintf(test_file, sizeof(test_file), "%s.c",
-			test_case->name);
+		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.
@@ -118,33 +104,27 @@ int test_btf_dump_case(int n, struct btf_dump_test_case *test_case)
 		 "out {sub(/^[ \\t]*\\*/, \"\"); print}' '%s' | diff -u - '%s'",
 		 test_file, out_file);
 	err = system(diff_cmd);
-	if (CHECK(err,
+	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);
-	fprintf(stderr, "OK\n");
 
 done:
 	btf__free(btf);
 	return err;
 }
 
-int main() {
-	int test_case_cnt, i, err, failed = 0;
-
-	test_case_cnt = sizeof(btf_dump_test_cases) /
-			sizeof(btf_dump_test_cases[0]);
+void test_btf_dump() {
+	int i;
 
-	for (i = 0; i < test_case_cnt; i++) {
-		err = test_btf_dump_case(i, &btf_dump_test_cases[i]);
-		if (err)
-			failed++;
-	}
+	for (i = 0; i < ARRAY_SIZE(btf_dump_test_cases); i++) {
+		struct btf_dump_test_case *t = &btf_dump_test_cases[i];
 
-	fprintf(stderr, "%d tests succeeded, %d tests failed.\n",
-		test_case_cnt - failed, failed);
+		if (!test__start_subtest(t->name))
+			continue;
 
-	return failed;
+		 test_btf_dump_case(i, &btf_dump_test_cases[i]);
+	}
 }

tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c

@@ -62,6 +62,10 @@ struct padded_a_lot {
  *	long: 64;
  *	long: 64;
  *	int b;
+ *	long: 32;
+ *	long: 64;
+ *	long: 64;
+ *	long: 64;
  *};
  *
  */
@@ -95,7 +99,6 @@ struct zone_padding {
 struct zone {
 	int a;
 	short b;
-	short: 16;
 	struct zone_padding __pad__;
 };