Re: [PATCH 2/3] btf_encoder: Change functions check due to broken dwarf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 26, 2020 at 5:07 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> We need to generate just single BTF instance for the
> function, while DWARF data contains multiple instances
> of DW_TAG_subprogram tag.
>
> Unfortunately we can no longer rely on DW_AT_declaration
> tag (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060)
>
> Instead we apply following checks:
>   - argument names are defined for the function
>   - there's symbol and address defined for the function
>   - function is generated only once
>
> They might be slightly superfluous together, but it's
> better to be ready for another DWARF mishap.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  btf_encoder.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  elf_symtab.h  |   8 ++++
>  2 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 2dd26c904039..99b9abe36993 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -26,6 +26,62 @@
>   */
>  #define KSYM_NAME_LEN 128
>
> +struct elf_function {
> +       const char *name;
> +       bool generated;
> +};
> +
> +static struct elf_function *functions;
> +static int functions_alloc;
> +static int functions_cnt;
> +
> +static int functions_cmp(const void *_a, const void *_b)
> +{
> +       const struct elf_function *a = _a;
> +       const struct elf_function *b = _b;
> +
> +       return strcmp(a->name, b->name);
> +}
> +
> +static void delete_functions(void)
> +{
> +       free(functions);
> +       functions_alloc = functions_cnt = 0;
> +}
> +
> +static int config_function(struct btf_elf *btfe, GElf_Sym *sym)
> +{
> +       if (!elf_sym__is_function(sym))
> +               return 0;
> +       if (!elf_sym__value(sym))
> +               return 0;
> +
> +       if (functions_cnt == functions_alloc) {
> +               functions_alloc += 5000;

maybe just do a conventional exponential size increase? Not
necessarily * 2, could be (* 3 / 2) or (* 4 / 3), libbpf uses such
approach.

> +               functions = realloc(functions, functions_alloc * sizeof(*functions));
> +               if (!functions)
> +                       return -1;
> +       }
> +
> +       functions[functions_cnt].name = elf_sym__name(sym, btfe->symtab);
> +       functions_cnt++;
> +       return 0;
> +}
> +

[...]

> diff --git a/elf_symtab.h b/elf_symtab.h
> index 359add69c8ab..094ec4683d01 100644
> --- a/elf_symtab.h
> +++ b/elf_symtab.h
> @@ -63,6 +63,14 @@ static inline uint64_t elf_sym__value(const GElf_Sym *sym)
>         return sym->st_value;
>  }
>
> +static inline int elf_sym__is_function(const GElf_Sym *sym)
> +{
> +       return (elf_sym__type(sym) == STT_FUNC ||
> +               elf_sym__type(sym) == STT_GNU_IFUNC) &&

Why do we need to collect STT_GNU_IFUNC? That is some PLT special
magic, does the kernel use that? Even if it does, are we even able to
attach to that? Could that remove some of the assembly functions?

> +               sym->st_name != 0 &&
> +               sym->st_shndx != SHN_UNDEF;
> +}
> +
>  static inline bool elf_sym__is_local_function(const GElf_Sym *sym)
>  {
>         return elf_sym__type(sym) == STT_FUNC &&
> --
> 2.26.2
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux