Merge branch 'strip-mods-from-global-vars'

Andrii Nakryiko says:

====================
Fix bpftool logic of stripping away const/volatile modifiers for all global
variables during BPF skeleton generation. See patch #1 for details on when
existing logic breaks and why it's important. Support special .strip_mods=true
mode in btf_dump__emit_type_decl.

Recent example of when this has caused problems can be found in [0].

  [0] https://github.com/iovisor/bcc/pull/2994#issuecomment-650588533
====================

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

5 files changed, 28 insertions, 21 deletions

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 10de76b296ba..b59d26e89367 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -88,7 +88,7 @@ static const char *get_map_ident(const struct bpf_map *map)
 		return NULL;
 }
 
-static void codegen_btf_dump_printf(void *ct, const char *fmt, va_list args)
+static void codegen_btf_dump_printf(void *ctx, const char *fmt, va_list args)
 {
 	vprintf(fmt, args);
 }
@@ -104,17 +104,20 @@ static int codegen_datasec_def(struct bpf_object *obj,
 	int i, err, off = 0, pad_cnt = 0, vlen = btf_vlen(sec);
 	const char *sec_ident;
 	char var_ident[256];
+	bool strip_mods = false;
 
-	if (strcmp(sec_name, ".data") == 0)
+	if (strcmp(sec_name, ".data") == 0) {
 		sec_ident = "data";
-	else if (strcmp(sec_name, ".bss") == 0)
+	} else if (strcmp(sec_name, ".bss") == 0) {
 		sec_ident = "bss";
-	else if (strcmp(sec_name, ".rodata") == 0)
+	} else if (strcmp(sec_name, ".rodata") == 0) {
 		sec_ident = "rodata";
-	else if (strcmp(sec_name, ".kconfig") == 0)
+		strip_mods = true;
+	} else if (strcmp(sec_name, ".kconfig") == 0) {
 		sec_ident = "kconfig";
-	else
+	} else {
 		return 0;
+	}
 
 	printf("	struct %s__%s {\n", obj_name, sec_ident);
 	for (i = 0; i < vlen; i++, sec_var++) {
@@ -123,16 +126,10 @@ static int codegen_datasec_def(struct bpf_object *obj,
 		DECLARE_LIBBPF_OPTS(btf_dump_emit_type_decl_opts, opts,
 			.field_name = var_ident,
 			.indent_level = 2,
+			.strip_mods = strip_mods,
 		);
 		int need_off = sec_var->offset, align_off, align;
 		__u32 var_type_id = var->type;
-		const struct btf_type *t;
-
-		t = btf__type_by_id(btf, var_type_id);
-		while (btf_is_mod(t)) {
-			var_type_id = t->type;
-			t = btf__type_by_id(btf, var_type_id);
-		}
 
 		if (off > need_off) {
 			p_err("Something is wrong for %s's variable #%d: need offset %d, already at %d.\n",
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index a3b7ef9b737f..491c7b41ffdc 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -144,8 +144,10 @@ struct btf_dump_emit_type_decl_opts {
 	 * necessary indentation already
 	 */
 	int indent_level;
+	/* strip all the const/volatile/restrict mods */
+	bool strip_mods;
 };
-#define btf_dump_emit_type_decl_opts__last_field indent_level
+#define btf_dump_emit_type_decl_opts__last_field strip_mods
 
 LIBBPF_API int
 btf_dump__emit_type_decl(struct btf_dump *d, __u32 id,
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index bbb430317260..e1c344504cae 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -60,6 +60,7 @@ struct btf_dump {
 	const struct btf_ext *btf_ext;
 	btf_dump_printf_fn_t printf_fn;
 	struct btf_dump_opts opts;
+	bool strip_mods;
 
 	/* per-type auxiliary state */
 	struct btf_dump_type_aux_state *type_states;
@@ -1032,7 +1033,9 @@ int btf_dump__emit_type_decl(struct btf_dump *d, __u32 id,
 
 	fname = OPTS_GET(opts, field_name, "");
 	lvl = OPTS_GET(opts, indent_level, 0);
+	d->strip_mods = OPTS_GET(opts, strip_mods, false);
 	btf_dump_emit_type_decl(d, id, fname, lvl);
+	d->strip_mods = false;
 	return 0;
 }
 
@@ -1045,6 +1048,10 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 
 	stack_start = d->decl_stack_cnt;
 	for (;;) {
+		t = btf__type_by_id(d->btf, id);
+		if (d->strip_mods && btf_is_mod(t))
+			goto skip_mod;
+
 		err = btf_dump_push_decl_stack_id(d, id);
 		if (err < 0) {
 			/*
@@ -1056,12 +1063,11 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 			d->decl_stack_cnt = stack_start;
 			return;
 		}
-
+skip_mod:
 		/* VOID */
 		if (id == 0)
 			break;
 
-		t = btf__type_by_id(d->btf, id);
 		switch (btf_kind(t)) {
 		case BTF_KIND_PTR:
 		case BTF_KIND_VOLATILE:
diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c
index fa153cf67b1b..fe87b77af459 100644
--- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
+++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
@@ -41,7 +41,7 @@ void test_skeleton(void)
 	CHECK(bss->in4 != 0, "in4", "got %lld != exp %lld\n", bss->in4, 0LL);
 	CHECK(bss->out4 != 0, "out4", "got %lld != exp %lld\n", bss->out4, 0LL);
 
-	CHECK(rodata->in6 != 0, "in6", "got %d != exp %d\n", rodata->in6, 0);
+	CHECK(rodata->in.in6 != 0, "in6", "got %d != exp %d\n", rodata->in.in6, 0);
 	CHECK(bss->out6 != 0, "out6", "got %d != exp %d\n", bss->out6, 0);
 
 	/* validate we can pre-setup global variables, even in .bss */
@@ -49,7 +49,7 @@ void test_skeleton(void)
 	data->in2 = 11;
 	bss->in3 = 12;
 	bss->in4 = 13;
-	rodata->in6 = 14;
+	rodata->in.in6 = 14;
 
 	err = test_skeleton__load(skel);
 	if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err))
@@ -60,7 +60,7 @@ void test_skeleton(void)
 	CHECK(data->in2 != 11, "in2", "got %lld != exp %lld\n", data->in2, 11LL);
 	CHECK(bss->in3 != 12, "in3", "got %d != exp %d\n", bss->in3, 12);
 	CHECK(bss->in4 != 13, "in4", "got %lld != exp %lld\n", bss->in4, 13LL);
-	CHECK(rodata->in6 != 14, "in6", "got %d != exp %d\n", rodata->in6, 14);
+	CHECK(rodata->in.in6 != 14, "in6", "got %d != exp %d\n", rodata->in.in6, 14);
 
 	/* now set new values and attach to get them into outX variables */
 	data->in1 = 1;
diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c
index 77ae86f44db5..374ccef704e1 100644
--- a/tools/testing/selftests/bpf/progs/test_skeleton.c
+++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
@@ -20,7 +20,9 @@ long long in4 __attribute__((aligned(64))) = 0;
 struct s in5 = {};
 
 /* .rodata section */
-const volatile int in6 = 0;
+const volatile struct {
+	const int in6;
+} in = {};
 
 /* .data section */
 int out1 = -1;
@@ -46,7 +48,7 @@ int handler(const void *ctx)
 	out3 = in3;
 	out4 = in4;
 	out5 = in5;
-	out6 = in6;
+	out6 = in.in6;
 
 	bpf_syscall = CONFIG_BPF_SYSCALL;
 	kern_ver = LINUX_KERNEL_VERSION;