On 16/10/2024 01:10, Ihor Solodrai wrote:
> Introduce a global elf_functions_list variable in btf_encoder.c that
> contains an elf_functions per ELF.
>
Arnaldo can help provide context here, but at least notionally I think
the idea of maintaining libdwarves as a library has value. In that
context, avoiding global lists where possible is a good thing I think,
since if it was used as a library, multiple invokations could confuse
the elf_functions list. To that end, can we make the elf_functions_list
a field in the conf_load perhaps? It already contains base_btf so there
is a precedent for storing data relevant to all encoders there, and
btf_encoder__new() has conf_load as a parameter, so the elf functions
list can still always be retrieved on encoder creation. In addition the
parameter to cus__process_dwflmod() has the parms structure which
contains the conf_load; you'd just need to pass that through to your
pre_load_module() callback I think.
It shouldn't be a massive change but I think it would be worthwhile.
Thanks!
> An elf_functions structure is allocated and filled out by
> btf_encoder__pre_load_module() hook, and the list is cleared after
> btf_encoder__encode() is done.
>
> At this point btf_encoders don't use shared elf_functions yet (each
> maintains their own copy as before), but it is built before encoders
> are initialized.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxx>
> ---
> btf_encoder.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> btf_encoder.h | 2 ++
> pahole.c | 3 +++
> 3 files changed, 71 insertions(+)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 9c840fa..8e8fd05 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -103,6 +103,8 @@ struct elf_secinfo {
> };
>
> struct elf_functions {
> + struct list_head node; /* for elf_functions_list */
> + Elf *elf; /* source ELF */
> struct elf_symtab *symtab;
> struct elf_function *entries;
> int cnt;
> @@ -147,6 +149,67 @@ struct btf_kfunc_set_range {
> uint64_t end;
> };
>
> +
> +/* In principle, multiple ELFs can be processed in one pahole run,
> + * so we have to store elf_functions table per ELF.
> + * An element is added to the list on btf_encoder__pre_load_module,
> + * and removed after btf_encoder__encode is done.
> + */
> +static LIST_HEAD(elf_functions_list);
> +
> +static inline void elf_functions__delete(struct elf_functions *funcs)
> +{
> + free(funcs->entries);
> + elf_symtab__delete(funcs->symtab);
> + list_del(&funcs->node);
> + free(funcs);
> +}
> +
> +static inline void elf_functions__delete_all(void)
> +{
> + struct list_head *pos, *tmp;
> +
> + list_for_each_safe(pos, tmp, &elf_functions_list) {
> + struct elf_functions *funcs = list_entry(pos, struct elf_functions, node);
> +
> + elf_functions__delete(funcs);
> + }
> +}
> +
> +static int elf_functions__collect(struct elf_functions *functions);
> +
> +int btf_encoder__pre_load_module(Dwfl_Module *mod, Elf *elf)
> +{
> + struct elf_functions *funcs;
> + int err;
> +
> + funcs = calloc(1, sizeof(*funcs));
> + if (!funcs) {
> + err = -ENOMEM;
> + goto out_delete;
> + }
> +
> + funcs->symtab = elf_symtab__new(NULL, elf);
> + if (!funcs->symtab) {
> + err = -1;
> + goto out_delete;
> + }
> +
> + funcs->elf = elf;
> + err = elf_functions__collect(funcs);
> + if (err)
> + goto out_delete;
> +
> + list_add_tail(&funcs->node, &elf_functions_list);
> +
> + return 0;
> +
> +out_delete:
> + elf_functions__delete(funcs);
> + return err;
> +}
> +
> +
> static LIST_HEAD(encoders);
> static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
>
> @@ -2071,6 +2134,8 @@ int btf_encoder__encode(struct btf_encoder *encoder)
> #endif
> err = btf_encoder__write_elf(encoder, encoder->btf, BTF_ELF_SEC);
> }
> +
> + elf_functions__delete_all();
> return err;
> }
>
> @@ -2387,6 +2452,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
> goto out;
> }
> encoder->functions.symtab = encoder->symtab;
> + encoder->functions.elf = cu->elf;
>
> /* index the ELF sections for later lookup */
>
> diff --git a/btf_encoder.h b/btf_encoder.h
> index 824963b..7debd67 100644
> --- a/btf_encoder.h
> +++ b/btf_encoder.h
> @@ -34,4 +34,6 @@ struct btf *btf_encoder__btf(struct btf_encoder *encoder);
>
> int btf_encoder__add_encoder(struct btf_encoder *encoder, struct btf_encoder *other);
>
> +int btf_encoder__pre_load_module(Dwfl_Module *mod, Elf *elf);
> +
> #endif /* _BTF_ENCODER_H_ */
> diff --git a/pahole.c b/pahole.c
> index b9e97ef..891af3a 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -3814,6 +3814,9 @@ int main(int argc, char *argv[])
> conf_load.threads_collect = pahole_threads_collect;
> }
>
> + if (btf_encode)
> + conf_load.pre_load_module = btf_encoder__pre_load_module;
> +
> // Make 'pahole --header type < file' a shorter form of 'pahole -C type --count 1 < file'
> if (conf.header_type && !class_name && prettify_input) {
> conf.count = 1;
