Commit 8afb259a authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

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: default avatarAlexei Starovoitov <ast@kernel.org>
parents 207a573c 0b20933d

tools/bpf/bpftool/gen.c

+10 −13
Original line number Diff line number Diff line
@@ -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",

tools/lib/bpf/btf.h

+3 −1
Original line number Diff line number Diff line
@@ -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,

tools/lib/bpf/btf_dump.c

+8 −2
Original line number Diff line number Diff line
@@ -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:

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

+3 −3
Original line number Diff line number Diff line
@@ -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;

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

+4 −2
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE