[PATCH bpf-next] selftests/bpf: support struct/union presets in veristat

[PATCH bpf-next] selftests/bpf: support struct/union presets in veristat

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Extend commit e3c9abd0d14b ("selftests/bpf: Implement setting global
variables in veristat") to support applying presets to members of
the global structs or unions in veristat.
For example:
```
./veristat set_global_vars.bpf.o  -G "union1.struct3.var_u8_h = 0xBB"
```

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 .../selftests/bpf/prog_tests/test_veristat.c  |   5 +
 tools/testing/selftests/bpf/progs/prepare.c   |   1 -
 .../selftests/bpf/progs/set_global_vars.c     |  38 +++++
 tools/testing/selftests/bpf/veristat.c        | 148 +++++++++++++++++-
 4 files changed, 184 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_veristat.c b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
index a95b42bf744a..47b56c258f3f 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_veristat.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_veristat.c
@@ -63,6 +63,9 @@ static void test_set_global_vars_succeeds(void)
 	    " -G \"var_eb = EB2\" "\
 	    " -G \"var_ec = EC2\" "\
 	    " -G \"var_b = 1\" "\
+	    " -G \"struct1.struct2.u.var_u8 = 170\" "\
+	    " -G \"union1.struct3.var_u8_l = 0xaa\" "\
+	    " -G \"union1.struct3.var_u8_h = 0xaa\" "\
 	    "-vl2 > %s", fix->veristat, fix->tmpfile);
 
 	read(fix->fd, fix->output, fix->sz);
@@ -78,6 +81,8 @@ static void test_set_global_vars_succeeds(void)
 	__CHECK_STR("_w=12 ", "var_eb = EB2");
 	__CHECK_STR("_w=13 ", "var_ec = EC2");
 	__CHECK_STR("_w=1 ", "var_b = 1");
+	__CHECK_STR("_w=170 ", "struct1.struct2.u.var_u8 = 170");
+	__CHECK_STR("_w=0xaaaa ", "union1.var_u16 = 0xaaaa");
 
 out:
 	teardown_fixture(fix);
diff --git a/tools/testing/selftests/bpf/progs/prepare.c b/tools/testing/selftests/bpf/progs/prepare.c
index 1f1dd547e4ee..cfc1f48e0d28 100644
--- a/tools/testing/selftests/bpf/progs/prepare.c
+++ b/tools/testing/selftests/bpf/progs/prepare.c
@@ -2,7 +2,6 @@
 /* Copyright (c) 2025 Meta */
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-//#include <bpf/bpf_tracing.h>
 
 char _license[] SEC("license") = "GPL";
 
diff --git a/tools/testing/selftests/bpf/progs/set_global_vars.c b/tools/testing/selftests/bpf/progs/set_global_vars.c
index 9adb5ba4cd4d..f2b354decbb8 100644
--- a/tools/testing/selftests/bpf/progs/set_global_vars.c
+++ b/tools/testing/selftests/bpf/progs/set_global_vars.c
@@ -24,6 +24,41 @@ const volatile enum Enumu64 var_eb = EB1;
 const volatile enum Enums64 var_ec = EC1;
 const volatile bool var_b = false;
 
+struct Struct {
+	__u16 filler;
+	struct {
+		__u16 filler2;
+	};
+	struct Struct2 {
+		__u16 filler;
+		volatile struct {
+			__u32 filler2;
+			union {
+				const volatile __u8 var_u8;
+				const volatile __s16 filler3;
+			} u;
+		};
+	} struct2;
+};
+const volatile __u32 struc = 0; /* same prefix as below */
+const volatile struct Struct struct1 = {.struct2 = {.u = {.var_u8 = 1}}};
+
+union Union {
+	__u16 var_u16;
+	struct Struct3 {
+		struct {
+			__u8 var_u8_l;
+		};
+		struct {
+			struct {
+				__u8 var_u8_h;
+			};
+		};
+	} struct3;
+};
+
+const volatile union Union union1 = {.var_u16 = -1};
+
 char arr[4] = {0};
 
 SEC("socket")
@@ -43,5 +78,8 @@ int test_set_globals(void *ctx)
 	a = var_eb;
 	a = var_ec;
 	a = var_b;
+	a = struct1.struct2.u.var_u8;
+	a = union1.var_u16;
+
 	return a;
 }
diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index a18972ffdeb6..babc97b799a2 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -23,6 +23,7 @@
 #include <float.h>
 #include <math.h>
 #include <limits.h>
+#include <linux/err.h>
 
 #ifndef ARRAY_SIZE
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
@@ -1486,7 +1487,131 @@ static bool is_preset_supported(const struct btf_type *t)
 	return btf_is_int(t) || btf_is_enum(t) || btf_is_enum64(t);
 }
 
-static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct btf_type *t,
+struct btf_anon_stack {
+	const struct btf_type *t;
+	__u32 offset;
+};
+
+const struct btf_member *btf_find_member(const struct btf *btf,
+					 const struct btf_type *parent_type,
+					 const char *member_name,
+					 __u32 *anon_offset)
+{
+	struct btf_anon_stack *anon_stack;
+	const struct btf_member *retval = NULL;
+	__u32 cur_offset = 0;
+	const char *name;
+	int top = 0, i;
+
+	if (!btf_is_struct(parent_type) && !btf_is_union(parent_type))
+		return ERR_PTR(-EINVAL);
+
+	anon_stack = malloc(sizeof(*anon_stack));
+	if (!anon_stack)
+		return ERR_PTR(-ENOMEM);
+
+	anon_stack[top].t = parent_type;
+	anon_stack[top++].offset = 0;
+
+	do {
+		parent_type = anon_stack[--top].t;
+		cur_offset = anon_stack[top].offset;
+
+		for (i = 0; i < btf_vlen(parent_type); ++i) {
+			const struct btf_member *member;
+			const struct btf_type *t;
+			int tid;
+
+			member = btf_members(parent_type) + i;
+			tid =  btf__resolve_type(btf, member->type);
+			if (tid < 0) {
+				retval = ERR_PTR(-EINVAL);
+				goto out;
+			}
+			t = btf__type_by_id(btf, tid);
+			if (member->name_off) {
+				name = btf__name_by_offset(btf, member->name_off);
+				if (name && strcmp(member_name, name) == 0) {
+					if (anon_offset)
+						*anon_offset = cur_offset;
+					retval = member;
+					goto out;
+				}
+			} else if (t) {
+				struct btf_anon_stack *tmp;
+
+				tmp = realloc(anon_stack, (top + 1) * sizeof(*anon_stack));
+				if (!tmp) {
+					retval = ERR_PTR(-ENOMEM);
+					goto out;
+				}
+				anon_stack = tmp;
+				/* Anonymous union/struct: push to stack */
+				anon_stack[top].t = t;
+				anon_stack[top++].offset = cur_offset + member->offset;
+			}
+		}
+	} while (top > 0);
+out:
+	free(anon_stack);
+	return retval;
+}
+
+static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
+				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
+{
+	char *name = strtok_r(NULL, ".", name_tok);
+	const struct btf_type *member_type;
+	const struct btf_member *member;
+	int member_tid;
+	__u32 anon_offset = 0;
+
+	if (!name)
+		return 0;
+
+	if (!btf_is_union(t) && !btf_is_struct(t))
+		return -EINVAL;
+
+	member = btf_find_member(btf, t, name, &anon_offset);
+	if (IS_ERR(member))
+		return -EINVAL;
+
+	member_tid = btf__resolve_type(btf, member->type);
+	member_type = btf__type_by_id(btf, member_tid);
+
+	if (btf_kflag(t)) {
+		sinfo->offset += (BTF_MEMBER_BIT_OFFSET(member->offset) + anon_offset) / 8;
+		sinfo->size = BTF_MEMBER_BITFIELD_SIZE(member->offset) / 8;
+	} else {
+		sinfo->offset += (member->offset + anon_offset) / 8;
+		sinfo->size = member_type->size;
+	}
+	sinfo->type = member_tid;
+
+	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
+}
+
+static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
+			      struct btf_var_secinfo *sinfo, const char *var)
+{
+	char expr[256], *saveptr;
+	const struct btf_type *base_type;
+	int err;
+
+	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
+	if (!btf_is_union(base_type) && !btf_is_struct(base_type))
+		return 0;
+
+	strcpy(expr, var);
+	strtok_r(expr, ".", &saveptr);
+	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int set_global_var(struct bpf_object *obj, struct btf *btf,
 			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
 			  struct var_preset *preset)
 {
@@ -1495,9 +1620,9 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
 	long long value = preset->ivalue;
 	size_t size;
 
-	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
+	base_type = btf__type_by_id(btf, btf__resolve_type(btf, sinfo->type));
 	if (!base_type) {
-		fprintf(stderr, "Failed to resolve type %d\n", t->type);
+		fprintf(stderr, "Failed to resolve type %d\n", sinfo->type);
 		return -EINVAL;
 	}
 	if (!is_preset_supported(base_type)) {
@@ -1530,7 +1655,7 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
 		if (value >= max_val || value < -max_val) {
 			fprintf(stderr,
 				"Variable %s value %lld is out of range [%lld; %lld]\n",
-				btf__name_by_offset(btf, t->name_off), value,
+				btf__name_by_offset(btf, base_type->name_off), value,
 				is_signed ? -max_val : 0, max_val - 1);
 			return -EINVAL;
 		}
@@ -1590,7 +1715,12 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
 			var_name = btf__name_by_offset(btf, var_type->name_off);
 
 			for (k = 0; k < npresets; ++k) {
-				if (strcmp(var_name, presets[k].name) != 0)
+				struct btf_var_secinfo tmp_sinfo;
+				int var_len = strlen(var_name);
+
+				if (strncmp(var_name, presets[k].name, var_len) != 0 ||
+				    (presets[k].name[var_len] != '\0' &&
+				     presets[k].name[var_len] != '.'))
 					continue;
 
 				if (presets[k].applied) {
@@ -1598,13 +1728,17 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
 						var_name);
 					return -EINVAL;
 				}
+				memcpy(&tmp_sinfo, sinfo, sizeof(*sinfo));
+				err = adjust_var_secinfo(btf, var_type,
+							 &tmp_sinfo, presets[k].name);
+				if (err)
+					return err;
 
-				err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
+				err = set_global_var(obj, btf, map, &tmp_sinfo, presets + k);
 				if (err)
 					return err;
 
 				presets[k].applied = true;
-				break;
 			}
 		}
 	}
-- 
2.48.1

Re: [PATCH bpf-next] selftests/bpf: support struct/union presets in veristat

[Eduard Zingerman]

On Thu, 2025-03-20 at 22:45 +0000, Mykyta Yatsenko wrote:

[...]

> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index a18972ffdeb6..babc97b799a2 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -23,6 +23,7 @@
>  #include <float.h>
>  #include <math.h>
>  #include <limits.h>
> +#include <linux/err.h>
>  
>  #ifndef ARRAY_SIZE
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> @@ -1486,7 +1487,131 @@ static bool is_preset_supported(const struct btf_type *t)
>  	return btf_is_int(t) || btf_is_enum(t) || btf_is_enum64(t);
>  }
>  
> -static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct btf_type *t,
> +struct btf_anon_stack {
> +	const struct btf_type *t;
> +	__u32 offset;
> +};
> +
> +const struct btf_member *btf_find_member(const struct btf *btf,
> +					 const struct btf_type *parent_type,
> +					 const char *member_name,
> +					 __u32 *anon_offset)
> +{
> +	struct btf_anon_stack *anon_stack;
> +	const struct btf_member *retval = NULL;
> +	__u32 cur_offset = 0;
> +	const char *name;
> +	int top = 0, i;
> +
> +	if (!btf_is_struct(parent_type) && !btf_is_union(parent_type))
> +		return ERR_PTR(-EINVAL);
> +
> +	anon_stack = malloc(sizeof(*anon_stack));
> +	if (!anon_stack)
> +		return ERR_PTR(-ENOMEM);
> +
> +	anon_stack[top].t = parent_type;
> +	anon_stack[top++].offset = 0;
> +
> +	do {
> +		parent_type = anon_stack[--top].t;
> +		cur_offset = anon_stack[top].offset;
> +
> +		for (i = 0; i < btf_vlen(parent_type); ++i) {
> +			const struct btf_member *member;
> +			const struct btf_type *t;
> +			int tid;
> +
> +			member = btf_members(parent_type) + i;
> +			tid =  btf__resolve_type(btf, member->type);

Nit: these are called member_tid and member_type in the function below.

> +			if (tid < 0) {
> +				retval = ERR_PTR(-EINVAL);
> +				goto out;
> +			}
> +			t = btf__type_by_id(btf, tid);
> +			if (member->name_off) {
> +				name = btf__name_by_offset(btf, member->name_off);
> +				if (name && strcmp(member_name, name) == 0) {
> +					if (anon_offset)

Nit: anon_offset is always non-null.

> +						*anon_offset = cur_offset;
> +					retval = member;
> +					goto out;
> +				}
> +			} else if (t) {

Nit: result of `btf__resolve_type()` is not checked against NULL in
     most places in veristat.c. When bpf object file is opened by
     libbpf the BTF is setup by function btf.c:btf_new(), which
     does some sanity including checks for ids of member types.
     See btf.c:btf_sanity_check().

> +				struct btf_anon_stack *tmp;
> +
> +				tmp = realloc(anon_stack, (top + 1) * sizeof(*anon_stack));
> +				if (!tmp) {
> +					retval = ERR_PTR(-ENOMEM);
> +					goto out;
> +				}
> +				anon_stack = tmp;
> +				/* Anonymous union/struct: push to stack */
> +				anon_stack[top].t = t;
> +				anon_stack[top++].offset = cur_offset + member->offset;

I think it is necessary to check that `t` is struct or union,
otherwise something like 'struct foo { int :64; int bar; }'
will cause trouble.

> +			}
> +		}
> +	} while (top > 0);
> +out:
> +	free(anon_stack);
> +	return retval;
> +}
> +
> +static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
> +				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
> +{
> +	char *name = strtok_r(NULL, ".", name_tok);
> +	const struct btf_type *member_type;
> +	const struct btf_member *member;
> +	int member_tid;
> +	__u32 anon_offset = 0;
> +
> +	if (!name)
> +		return 0;
> +
> +	if (!btf_is_union(t) && !btf_is_struct(t))
> +		return -EINVAL;
> +
> +	member = btf_find_member(btf, t, name, &anon_offset);
> +	if (IS_ERR(member))
> +		return -EINVAL;
> +
> +	member_tid = btf__resolve_type(btf, member->type);
> +	member_type = btf__type_by_id(btf, member_tid);
> +
> +	if (btf_kflag(t)) {
> +		sinfo->offset += (BTF_MEMBER_BIT_OFFSET(member->offset) + anon_offset) / 8;
> +		sinfo->size = BTF_MEMBER_BITFIELD_SIZE(member->offset) / 8;

Bitfields are not handled by `set_global_var`, as ->size is in bytes.
Maybe just error out here saying that setting bitfields is not supported?
Alternatively, there is a utility function btf_member_bit_offset(),
maybe declare a similar btf_member_bit_size() and remove the
btf_kflag(t) condition here? Just to make it a bit easier to understand.

> +	} else {
> +		sinfo->offset += (member->offset + anon_offset) / 8;
> +		sinfo->size = member_type->size;
> +	}
> +	sinfo->type = member_tid;
> +
> +	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
> +}
> +
> +static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
> +			      struct btf_var_secinfo *sinfo, const char *var)
> +{
> +	char expr[256], *saveptr;
> +	const struct btf_type *base_type;
> +	int err;
> +
> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +	if (!btf_is_union(base_type) && !btf_is_struct(base_type))
> +		return 0;

What would happen if preset "foo.bar" would be specified for variable
"foo" being e.g. of type "int"? It seems the ".bar" part would be just
ignored.

> +
> +	strcpy(expr, var);

Nit: strncpy ?

> +	strtok_r(expr, ".", &saveptr);
> +	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int set_global_var(struct bpf_object *obj, struct btf *btf,
>  			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
>  			  struct var_preset *preset)
>  {
> @@ -1495,9 +1620,9 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>  	long long value = preset->ivalue;
>  	size_t size;
>  
> -	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, sinfo->type));
>  	if (!base_type) {
> -		fprintf(stderr, "Failed to resolve type %d\n", t->type);
> +		fprintf(stderr, "Failed to resolve type %d\n", sinfo->type);
>  		return -EINVAL;
>  	}
>  	if (!is_preset_supported(base_type)) {
> @@ -1530,7 +1655,7 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>  		if (value >= max_val || value < -max_val) {
>  			fprintf(stderr,
>  				"Variable %s value %lld is out of range [%lld; %lld]\n",
> -				btf__name_by_offset(btf, t->name_off), value,
> +				btf__name_by_offset(btf, base_type->name_off), value,
>  				is_signed ? -max_val : 0, max_val - 1);
>  			return -EINVAL;
>  		}
> @@ -1590,7 +1715,12 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>  			var_name = btf__name_by_offset(btf, var_type->name_off);
>  
>  			for (k = 0; k < npresets; ++k) {
> -				if (strcmp(var_name, presets[k].name) != 0)
> +				struct btf_var_secinfo tmp_sinfo;
> +				int var_len = strlen(var_name);
> +
> +				if (strncmp(var_name, presets[k].name, var_len) != 0 ||
> +				    (presets[k].name[var_len] != '\0' &&
> +				     presets[k].name[var_len] != '.'))

var_name comes from BTF and presets[k].name comes from command line, right?
Meaning that there might be a case when strlen(presets[k].name) < strlen(var_name)
and access presets[k].name[var_len] would be out of bounds. Wdyt?

>  					continue;
>  
>  				if (presets[k].applied) {
> @@ -1598,13 +1728,17 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>  						var_name);
>  					return -EINVAL;
>  				}
> +				memcpy(&tmp_sinfo, sinfo, sizeof(*sinfo));
> +				err = adjust_var_secinfo(btf, var_type,
> +							 &tmp_sinfo, presets[k].name);
> +				if (err)
> +					return err;
>  
> -				err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
> +				err = set_global_var(obj, btf, map, &tmp_sinfo, presets + k);
>  				if (err)
>  					return err;
>  
>  				presets[k].applied = true;
> -				break;

This is removed to handle cases with presets "foo.bar" and "foo.buz", right?
Maybe extend the test case a bit?

>  			}
>  		}
>  	}

Re: [PATCH bpf-next] selftests/bpf: support struct/union presets in veristat

[Mykyta Yatsenko]

On 22/03/2025 01:09, Eduard Zingerman wrote:
> On Thu, 2025-03-20 at 22:45 +0000, Mykyta Yatsenko wrote:
>
> [...]
>
>> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
>> index a18972ffdeb6..babc97b799a2 100644
>> --- a/tools/testing/selftests/bpf/veristat.c
>> +++ b/tools/testing/selftests/bpf/veristat.c
>> @@ -23,6 +23,7 @@
>>   #include <float.h>
>>   #include <math.h>
>>   #include <limits.h>
>> +#include <linux/err.h>
>>   
>>   #ifndef ARRAY_SIZE
>>   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>> @@ -1486,7 +1487,131 @@ static bool is_preset_supported(const struct btf_type *t)
>>   	return btf_is_int(t) || btf_is_enum(t) || btf_is_enum64(t);
>>   }
>>   
>> -static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct btf_type *t,
>> +struct btf_anon_stack {
>> +	const struct btf_type *t;
>> +	__u32 offset;
>> +};
>> +
>> +const struct btf_member *btf_find_member(const struct btf *btf,
>> +					 const struct btf_type *parent_type,
>> +					 const char *member_name,
>> +					 __u32 *anon_offset)
>> +{
>> +	struct btf_anon_stack *anon_stack;
>> +	const struct btf_member *retval = NULL;
>> +	__u32 cur_offset = 0;
>> +	const char *name;
>> +	int top = 0, i;
>> +
>> +	if (!btf_is_struct(parent_type) && !btf_is_union(parent_type))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	anon_stack = malloc(sizeof(*anon_stack));
>> +	if (!anon_stack)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	anon_stack[top].t = parent_type;
>> +	anon_stack[top++].offset = 0;
>> +
>> +	do {
>> +		parent_type = anon_stack[--top].t;
>> +		cur_offset = anon_stack[top].offset;
>> +
>> +		for (i = 0; i < btf_vlen(parent_type); ++i) {
>> +			const struct btf_member *member;
>> +			const struct btf_type *t;
>> +			int tid;
>> +
>> +			member = btf_members(parent_type) + i;
>> +			tid =  btf__resolve_type(btf, member->type);
> Nit: these are called member_tid and member_type in the function below.
>
>> +			if (tid < 0) {
>> +				retval = ERR_PTR(-EINVAL);
>> +				goto out;
>> +			}
>> +			t = btf__type_by_id(btf, tid);
>> +			if (member->name_off) {
>> +				name = btf__name_by_offset(btf, member->name_off);
>> +				if (name && strcmp(member_name, name) == 0) {
>> +					if (anon_offset)
> Nit: anon_offset is always non-null.
>
Addressing nits in v2.
>> +						*anon_offset = cur_offset;
>> +					retval = member;
>> +					goto out;
>> +				}
>> +			} else if (t) {
> Nit: result of `btf__resolve_type()` is not checked against NULL in
>       most places in veristat.c. When bpf object file is opened by
>       libbpf the BTF is setup by function btf.c:btf_new(), which
>       does some sanity including checks for ids of member types.
>       See btf.c:btf_sanity_check().
>
>> +				struct btf_anon_stack *tmp;
>> +
>> +				tmp = realloc(anon_stack, (top + 1) * sizeof(*anon_stack));
>> +				if (!tmp) {
>> +					retval = ERR_PTR(-ENOMEM);
>> +					goto out;
>> +				}
>> +				anon_stack = tmp;
>> +				/* Anonymous union/struct: push to stack */
>> +				anon_stack[top].t = t;
>> +				anon_stack[top++].offset = cur_offset + member->offset;
> I think it is necessary to check that `t` is struct or union,
> otherwise something like 'struct foo { int :64; int bar; }'
> will cause trouble.
This is a good idea, I agree.
>> +			}
>> +		}
>> +	} while (top > 0);
>> +out:
>> +	free(anon_stack);
>> +	return retval;
>> +}
>> +
>> +static int adjust_var_secinfo_tok(char **name_tok, const struct btf *btf,
>> +				  const struct btf_type *t, struct btf_var_secinfo *sinfo)
>> +{
>> +	char *name = strtok_r(NULL, ".", name_tok);
>> +	const struct btf_type *member_type;
>> +	const struct btf_member *member;
>> +	int member_tid;
>> +	__u32 anon_offset = 0;
>> +
>> +	if (!name)
>> +		return 0;
>> +
>> +	if (!btf_is_union(t) && !btf_is_struct(t))
>> +		return -EINVAL;
>> +
>> +	member = btf_find_member(btf, t, name, &anon_offset);
>> +	if (IS_ERR(member))
>> +		return -EINVAL;
>> +
>> +	member_tid = btf__resolve_type(btf, member->type);
>> +	member_type = btf__type_by_id(btf, member_tid);
>> +
>> +	if (btf_kflag(t)) {
>> +		sinfo->offset += (BTF_MEMBER_BIT_OFFSET(member->offset) + anon_offset) / 8;
>> +		sinfo->size = BTF_MEMBER_BITFIELD_SIZE(member->offset) / 8;
> Bitfields are not handled by `set_global_var`, as ->size is in bytes.
> Maybe just error out here saying that setting bitfields is not supported?
> Alternatively, there is a utility function btf_member_bit_offset(),
> maybe declare a similar btf_member_bit_size() and remove the
> btf_kflag(t) condition here? Just to make it a bit easier to understand.
Right, we don't support setting bitfields, It makes sense to error out.
>> +	} else {
>> +		sinfo->offset += (member->offset + anon_offset) / 8;
>> +		sinfo->size = member_type->size;
>> +	}
>> +	sinfo->type = member_tid;
>> +
>> +	return adjust_var_secinfo_tok(name_tok, btf, member_type, sinfo);
>> +}
>> +
>> +static int adjust_var_secinfo(struct btf *btf, const struct btf_type *t,
>> +			      struct btf_var_secinfo *sinfo, const char *var)
>> +{
>> +	char expr[256], *saveptr;
>> +	const struct btf_type *base_type;
>> +	int err;
>> +
>> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
>> +	if (!btf_is_union(base_type) && !btf_is_struct(base_type))
>> +		return 0;
> What would happen if preset "foo.bar" would be specified for variable
> "foo" being e.g. of type "int"? It seems the ".bar" part would be just
> ignored.
>
good point, I'll change this for v2.
>> +
>> +	strcpy(expr, var);
> Nit: strncpy ?
yes
>
>> +	strtok_r(expr, ".", &saveptr);
>> +	err = adjust_var_secinfo_tok(&saveptr, btf, base_type, sinfo);
>> +	if (err)
>> +		return err;
>> +
>> +	return 0;
>> +}
>> +
>> +static int set_global_var(struct bpf_object *obj, struct btf *btf,
>>   			  struct bpf_map *map, struct btf_var_secinfo *sinfo,
>>   			  struct var_preset *preset)
>>   {
>> @@ -1495,9 +1620,9 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>>   	long long value = preset->ivalue;
>>   	size_t size;
>>   
>> -	base_type = btf__type_by_id(btf, btf__resolve_type(btf, t->type));
>> +	base_type = btf__type_by_id(btf, btf__resolve_type(btf, sinfo->type));
>>   	if (!base_type) {
>> -		fprintf(stderr, "Failed to resolve type %d\n", t->type);
>> +		fprintf(stderr, "Failed to resolve type %d\n", sinfo->type);
>>   		return -EINVAL;
>>   	}
>>   	if (!is_preset_supported(base_type)) {
>> @@ -1530,7 +1655,7 @@ static int set_global_var(struct bpf_object *obj, struct btf *btf, const struct
>>   		if (value >= max_val || value < -max_val) {
>>   			fprintf(stderr,
>>   				"Variable %s value %lld is out of range [%lld; %lld]\n",
>> -				btf__name_by_offset(btf, t->name_off), value,
>> +				btf__name_by_offset(btf, base_type->name_off), value,
>>   				is_signed ? -max_val : 0, max_val - 1);
>>   			return -EINVAL;
>>   		}
>> @@ -1590,7 +1715,12 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>>   			var_name = btf__name_by_offset(btf, var_type->name_off);
>>   
>>   			for (k = 0; k < npresets; ++k) {
>> -				if (strcmp(var_name, presets[k].name) != 0)
>> +				struct btf_var_secinfo tmp_sinfo;
>> +				int var_len = strlen(var_name);
>> +
>> +				if (strncmp(var_name, presets[k].name, var_len) != 0 ||
>> +				    (presets[k].name[var_len] != '\0' &&
>> +				     presets[k].name[var_len] != '.'))
> var_name comes from BTF and presets[k].name comes from command line, right?
> Meaning that there might be a case when strlen(presets[k].name) < strlen(var_name)
> and access presets[k].name[var_len] would be out of bounds. Wdyt?
checks
```
+(presets[k].name[var_len] != '\0' && presets[k].name[var_len] != '.')
```
Are executed only if
```
strncmp(var_name, presets[k].name, var_len) == 0
```
returns 0 (because of ||), which means there are at least var_len 
non-terminal symbols in
presets[k].name.

>>   					continue;
>>   
>>   				if (presets[k].applied) {
>> @@ -1598,13 +1728,17 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>>   						var_name);
>>   					return -EINVAL;
>>   				}
>> +				memcpy(&tmp_sinfo, sinfo, sizeof(*sinfo));
>> +				err = adjust_var_secinfo(btf, var_type,
>> +							 &tmp_sinfo, presets[k].name);
>> +				if (err)
>> +					return err;
>>   
>> -				err = set_global_var(obj, btf, var_type, map, sinfo, presets + k);
>> +				err = set_global_var(obj, btf, map, &tmp_sinfo, presets + k);
>>   				if (err)
>>   					return err;
>>   
>>   				presets[k].applied = true;
>> -				break;
> This is removed to handle cases with presets "foo.bar" and "foo.buz", right?
> Maybe extend the test case a bit?
It has been extended, check this out:
+        " -G \"union1.struct3.var_u8_l = 0xaa\" "\
+        " -G \"union1.struct3.var_u8_h = 0xaa\" "\
>>   			}
>>   		}
>>   	}
>