On 8/15/24 19:39, Sami Tolvanen wrote:
> Add support for passing a list of exported symbols to gendwarfksyms
> via stdin and filter out non-exported symbols from the output.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> ---
> scripts/gendwarfksyms/Makefile | 1 +
> scripts/gendwarfksyms/dwarf.c | 53 ++++++++++++++-
> scripts/gendwarfksyms/gendwarfksyms.c | 4 +-
> scripts/gendwarfksyms/gendwarfksyms.h | 21 ++++++
> scripts/gendwarfksyms/symbols.c | 96 +++++++++++++++++++++++++++
> 5 files changed, 171 insertions(+), 4 deletions(-)
> create mode 100644 scripts/gendwarfksyms/symbols.c
>
> diff --git a/scripts/gendwarfksyms/Makefile b/scripts/gendwarfksyms/Makefile
> index c1389c161f9c..623f8fc975ea 100644
> --- a/scripts/gendwarfksyms/Makefile
> +++ b/scripts/gendwarfksyms/Makefile
> @@ -2,6 +2,7 @@ hostprogs-always-y += gendwarfksyms
>
> gendwarfksyms-objs += gendwarfksyms.o
> gendwarfksyms-objs += dwarf.o
> +gendwarfksyms-objs += symbols.o
>
> HOST_EXTRACFLAGS := -I $(srctree)/tools/include
> HOSTLDLIBS_gendwarfksyms := -ldw -lelf
> diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
> index 65a29d0bd8f4..71cfab0553da 100644
> --- a/scripts/gendwarfksyms/dwarf.c
> +++ b/scripts/gendwarfksyms/dwarf.c
> @@ -5,6 +5,48 @@
>
> #include "gendwarfksyms.h"
>
> +static bool get_ref_die_attr(Dwarf_Die *die, unsigned int id, Dwarf_Die *value)
> +{
> + Dwarf_Attribute da;
> +
> + /* dwarf_formref_die returns a pointer instead of an error value. */
> + return dwarf_attr(die, id, &da) && dwarf_formref_die(&da, value);
> +}
> +
> +static const char *get_name(Dwarf_Die *die)
> +{
> + Dwarf_Attribute attr;
> +
> + /* rustc uses DW_AT_linkage_name for exported symbols */
> + if (dwarf_attr(die, DW_AT_linkage_name, &attr) ||
> + dwarf_attr(die, DW_AT_name, &attr)) {
> + return dwarf_formstring(&attr);
> + }
> +
> + return NULL;
> +}
> +
> +static bool is_export_symbol(struct state *state, Dwarf_Die *die)
> +{
> + Dwarf_Die *source = die;
> + Dwarf_Die origin;
> +
> + state->sym = NULL;
Nit: This assignment isn't strictly necessary, the value is overwritten
a few lines below and isn't used in between.
> +
> + /* If the DIE has an abstract origin, use it for type information. */
> + if (get_ref_die_attr(die, DW_AT_abstract_origin, &origin))
> + source = &origin;
> +
> + state->sym = symbol_get(get_name(die));
> +
> + /* Look up using the origin name if there are no matches. */
> + if (!state->sym && source != die)
> + state->sym = symbol_get(get_name(source));
> +
> + state->die = *source;
> + return !!state->sym;
> +}
> +
> /*
> * Type string processing
> */
> @@ -40,7 +82,7 @@ int process_die_container(struct state *state, Dwarf_Die *die,
> }
>
> /*
> - * Symbol processing
> + * Exported symbol processing
> */
> static int process_subprogram(struct state *state, Dwarf_Die *die)
> {
> @@ -67,10 +109,15 @@ static int process_exported_symbols(struct state *state, Dwarf_Die *die)
> /* Possible exported symbols */
> case DW_TAG_subprogram:
> case DW_TAG_variable:
> + if (!is_export_symbol(state, die))
> + return 0;
> +
> + debug("%s", state->sym->name);
> +
> if (tag == DW_TAG_subprogram)
> - check(process_subprogram(state, die));
> + check(process_subprogram(state, &state->die));
> else
> - check(process_variable(state, die));
> + check(process_variable(state, &state->die));
>
> return 0;
> default:
> diff --git a/scripts/gendwarfksyms/gendwarfksyms.c b/scripts/gendwarfksyms/gendwarfksyms.c
> index 27f2d6423c45..d209b237766b 100644
> --- a/scripts/gendwarfksyms/gendwarfksyms.c
> +++ b/scripts/gendwarfksyms/gendwarfksyms.c
> @@ -27,7 +27,7 @@ static const struct {
>
> static int usage(void)
> {
> - error("usage: gendwarfksyms [options] elf-object-file ...");
> + error("usage: gendwarfksyms [options] elf-object-file ... < symbol-list");
> return -1;
> }
>
> @@ -105,6 +105,8 @@ int main(int argc, const char **argv)
> if (parse_options(argc, argv) < 0)
> return usage();
>
> + check(symbol_read_exports(stdin));
> +
> for (n = 0; n < object_count; n++) {
> Dwfl *dwfl;
> int fd;
> diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
> index 5ab7ce7d4efb..03f3e408a839 100644
> --- a/scripts/gendwarfksyms/gendwarfksyms.h
> +++ b/scripts/gendwarfksyms/gendwarfksyms.h
> @@ -7,9 +7,11 @@
> #include <elfutils/libdw.h>
> #include <elfutils/libdwfl.h>
> #include <linux/hashtable.h>
> +#include <linux/jhash.h>
> #include <inttypes.h>
> #include <stdlib.h>
> #include <stdio.h>
> +#include <string.h>
>
> #ifndef __GENDWARFKSYMS_H
> #define __GENDWARFKSYMS_H
> @@ -56,6 +58,23 @@ extern bool debug;
> /* Error == negative values */
> #define checkp(expr) __check(expr, __res < 0, __res)
>
> +/*
> + * symbols.c
> + */
> +
> +static inline u32 name_hash(const char *name)
> +{
> + return jhash(name, strlen(name), 0);
> +}
> +
> +struct symbol {
> + const char *name;
> + struct hlist_node name_hash;
> +};
> +
> +extern int symbol_read_exports(FILE *file);
> +extern struct symbol *symbol_get(const char *name);
Nit: extern isn't necessary here and in other similar cases throughout
the series. It should be removed per
Documentation/process/coding-style.rst, 6.1) Function prototypes.
> +
> /*
> * dwarf.c
> */
> @@ -63,6 +82,8 @@ extern bool debug;
> struct state {
> Dwfl_Module *mod;
> Dwarf *dbg;
> + struct symbol *sym;
> + Dwarf_Die die;
> };
>
> typedef int (*die_callback_t)(struct state *state, Dwarf_Die *die);
> diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
> new file mode 100644
> index 000000000000..673ad9cf9e77
> --- /dev/null
> +++ b/scripts/gendwarfksyms/symbols.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 Google LLC
> + */
> +
> +#include "gendwarfksyms.h"
> +
> +#define SYMBOL_HASH_BITS 15
> +static DEFINE_HASHTABLE(symbol_names, SYMBOL_HASH_BITS);
> +
> +typedef int (*symbol_callback_t)(struct symbol *, void *arg);
> +
> +static int for_each(const char *name, symbol_callback_t func, void *data)
> +{
> + struct hlist_node *tmp;
> + struct symbol *match;
> +
> + if (!name || !*name)
> + return 0;
> +
> + hash_for_each_possible_safe(symbol_names, match, tmp, name_hash,
> + name_hash(name)) {
> + if (strcmp(match->name, name))
> + continue;
> +
> + if (func)
> + check(func(match, data));
> +
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static bool is_exported(const char *name)
> +{
> + return checkp(for_each(name, NULL, NULL)) > 0;
> +}
> +
> +int symbol_read_exports(FILE *file)
> +{
> + struct symbol *sym;
> + char *line = NULL;
> + char *name = NULL;
> + size_t size = 0;
> + int nsym = 0;
> +
> + while (getline(&line, &size, file) > 0) {
> + if (sscanf(line, "%ms\n", &name) != 1) {
> + error("malformed input line: %s", line);
> + return -1;
> + }
> +
> + free(line);
> + line = NULL;
> +
> + if (is_exported(name))
> + continue; /* Ignore duplicates */
> +
> + sym = malloc(sizeof(struct symbol));
> + if (!sym) {
> + error("malloc failed");
> + return -1;
> + }
> +
> + sym->name = name;
> + name = NULL;
> +
> + hash_add(symbol_names, &sym->name_hash, name_hash(sym->name));
> + ++nsym;
> +
> + debug("%s", sym->name);
> + }
> +
> + if (line)
> + free(line);
The loop leaks line on a potential sscanf() error and name if the symbol
is a duplicate or malloc(sizeof(struct symbol)) fails. Additionally, it
should be possible to avoid allocating line by getline() on each
iteration.
I would change it to something like this (not tested):
int symbol_read_exports(FILE *file)
{
struct symbol *sym;
char *line = NULL;
char *name = NULL;
size_t size = 0;
int nsym = 0;
int ret = -1;
while (getline(&line, &size, file) > 0) {
if (sscanf(line, "%ms\n", &name) != 1) {
error("malformed input line: %s", line);
goto out;
}
if (is_exported(name)) {
/* Ignore duplicates */
free(name);
name = NULL;
continue;
}
sym = malloc(sizeof(struct symbol));
if (!sym) {
error("malloc failed");
goto out;
}
sym->name = name;
name = NULL;
hash_add(symbol_names, &sym->name_hash, name_hash(sym->name));
++nsym;
debug("%s", sym->name);
}
debug("%d exported symbols", nsym);
ret = 0;
out:
free(line);
free(name);
return ret;
}
> +
> + debug("%d exported symbols", nsym);
> + return 0;
> +}
> +
> +static int get_symbol(struct symbol *sym, void *arg)
> +{
> + struct symbol **res = arg;
> +
> + *res = sym;
> + return 0;
> +}
> +
> +struct symbol *symbol_get(const char *name)
> +{
> + struct symbol *sym = NULL;
> +
> + for_each(name, get_symbol, &sym);
> + return sym;
> +}
--
Thanks,
Petr
