On Wed, 2025-02-19 at 23:30 +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@xxxxxxxx>
>
> To better verify some complex BPF programs we'd like to preset global
> variables.
> This patch introduces CLI argument `--set-global-vars` or `-G` to
> veristat, that allows presetting values to global variables defined
> in BPF program. For example:
>
> prog.c:
> ```
> enum Enum { ELEMENT1 = 0, ELEMENT2 = 5 };
> const volatile __s64 a = 5;
> const volatile __u8 b = 5;
> const volatile enum Enum c = ELEMENT2;
> const volatile bool d = false;
>
> char arr[4] = {0};
>
> SEC("tp_btf/sched_switch")
> int BPF_PROG(...)
> {
> bpf_printk("%c\n", arr[a]);
> bpf_printk("%c\n", arr[b]);
> bpf_printk("%c\n", arr[c]);
> bpf_printk("%c\n", arr[d]);
> return 0;
> }
> ```
> By default verification of the program fails:
> ```
> ./veristat prog.bpf.o
> ```
> By presetting global variables, we can make verification pass:
> ```
> ./veristat wq.bpf.o -G "a = 0" -G "b = 1" -G "c = 2" -G "d = 3"
> ```
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> ---
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -1292,6 +1320,249 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
> return 0;
> };
>
> +static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr)
> +{
> + void *tmp;
> + struct var_preset *cur;
> + char var[256], val[256];
> +
> + tmp = realloc(*presets, (*cnt + 1) * sizeof(**presets));
> + if (!tmp)
> + return -ENOMEM;
> + *presets = tmp;
> + cur = &(*presets)[*cnt];
> + cur->applied = false;
> +
> + if (sscanf(expr, "%s = %s\n", var, val) != 2) {
> + fprintf(stderr, "Could not parse expression %s\n", expr);
> + return -EINVAL;
> + }
> +
> + if (isalpha(*val)) {
> + cur->svalue = strdup(val);
Nit: when I run veristat under valgrind, it complains that
fields allocated in this function are never freed.
Grepping for 'free', it look like veristat.c prefers to free
any allocated memory before exit.
> + cur->type = NAME;
> + } else if (*val == '-' || isdigit(*val)) {
> + long long value;
> +
> + errno = 0;
> + value = strtoll(val, NULL, 0);
> + if (errno == ERANGE) {
> + errno = 0;
> + value = strtoull(val, NULL, 0);
> + }
> + cur->ivalue = value;
> + cur->type = INTEGRAL;
> + if (errno) {
> + fprintf(stderr, "Could not parse integer value %s\n", val);
> + return -EINVAL;
> + }
> + } else {
> + fprintf(stderr, "Could not parse value %s\n", val);
> + return -EINVAL;
> + }
> + cur->name = strdup(var);
> + (*cnt)++;
> + return 0;
> +}
> +
> +static int append_var_preset_file(const char *filename)
> +{
> + char buf[1024];
> + FILE *f;
> + int err = 0;
> +
> + f = fopen(filename, "rt");
> + if (!f) {
> + err = -errno;
> + fprintf(stderr, "Failed to open presets in '%s': %d\n", filename, err);
Nit: fprintf(stderr, "Failed to open presets in '%s': %s\n",
filename, strerror(errno));
Would make this a bit more friendly
(and it prints error code if string representation does not exist).
> + return -EINVAL;
> + }
> +
> + while (fscanf(f, " %1023[^\n]\n", buf) == 1) {
> + if (buf[0] == '\0' || buf[0] == '#')
> + continue;
> +
> + err = append_var_preset(&env.presets, &env.npresets, buf);
> + if (err)
> + goto cleanup;
> + }
> +
> +cleanup:
> + fclose(f);
> + return err;
> +}
[...]
