Re: [PATCH 14/19] libbpf: Add btf__find_by_pattern_kind function

Andrii Nakryiko
2021-06-09T05:30:47Z

On Sat, Jun 5, 2021 at 4:14 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding btf__find_by_pattern_kind function that returns
> array of BTF ids for given function name pattern.
>
> Using libc's regex.h support for that.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/bpf/btf.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/btf.h | 3 ++
> 2 files changed, 71 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index b46760b93bb4..421dd6c1e44a 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> /* Copyright (c) 2018 Facebook */
>
> +#define _GNU_SOURCE
> #include <byteswap.h>
> #include <endian.h>
> #include <stdio.h>
> @@ -16,6 +17,7 @@
> #include <linux/err.h>
> #include <linux/btf.h>
> #include <gelf.h>
> +#include <regex.h>
> #include "btf.h"
> #include "bpf.h"
> #include "libbpf.h"
> @@ -711,6 +713,72 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
> return libbpf_err(-ENOENT);
> }
>
> +static bool is_wildcard(char c)
> +{
> + static const char *wildchars = "*?[|";
> +
> + return strchr(wildchars, c);
> +}
> +
> +int btf__find_by_pattern_kind(const struct btf *btf,
> + const char *type_pattern, __u32 kind,
> + __s32 **__ids)
> +{
> + __u32 i, nr_types = btf__get_nr_types(btf);
> + __s32 *ids = NULL;
> + int cnt = 0, alloc = 0, ret;
> + regex_t regex;
> + char *pattern;
> +
> + if (kind == BTF_KIND_UNKN || !strcmp(type_pattern, "void"))
> + return 0;
> +
> + /* When the pattern does not start with wildcard, treat it as
> + * if we'd want to match it from the beginning of the string.
> + */

This assumption is absolutely atrocious. If we say it's regexp, then
it has to always be regexp, not something based on some random
heuristic based on the first character.

Taking a step back, though. Do we really need to provide this API? Why
applications can't implement it on their own, given regexp
functionality is provided by libc. Which I didn't know, actually, so
that's pretty nice, assuming that it's also available in more minimal
implementations like musl.

> + asprintf(&pattern, "%s%s",
> + is_wildcard(type_pattern[0]) ? "^" : "",
> + type_pattern);
> +
> + ret = regcomp(&regex, pattern, REG_EXTENDED);
> + if (ret) {
> + pr_warn("failed to compile regex\n");
> + free(pattern);
> + return -EINVAL;
> + }
> +
> + free(pattern);
> +
> + for (i = 1; i <= nr_types; i++) {
> + const struct btf_type *t = btf__type_by_id(btf, i);
> + const char *name;
> + __s32 *p;
> +
> + if (btf_kind(t) != kind)
> + continue;
> + name = btf__name_by_offset(btf, t->name_off);
> + if (name && regexec(&regex, name, 0, NULL, 0))
> + continue;
> + if (cnt == alloc) {
> + alloc = max(100, alloc * 3 / 2);
> + p = realloc(ids, alloc * sizeof(__u32));

this memory allocation and re-allocation on behalf of users is another
argument against this API

> + if (!p) {
> + free(ids);
> + regfree(&regex);
> + return -ENOMEM;
> + }
> + ids = p;
> + }
> +
> + ids[cnt] = i;
> + cnt++;
> + }
> +
> + regfree(&regex);
> + *__ids = ids;
> + return cnt ?: -ENOENT;
> +}
> +
> static bool btf_is_modifiable(const struct btf *btf)
> {
> return (void *)btf->hdr != btf->raw_data;
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index b54f1c3ebd57..036857aded94 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -371,6 +371,9 @@ btf_var_secinfos(const struct btf_type *t)
> return (struct btf_var_secinfo *)(t + 1);
> }
>
> +int btf__find_by_pattern_kind(const struct btf *btf,
> + const char *type_pattern, __u32 kind,
> + __s32 **__ids);
> #ifdef __cplusplus
> } /* extern "C" */
> #endif
> --
> 2.31.1
>