Skip to content

Commit

Permalink
libdrgn: add lifetime to drgn_symbol, reduce copying
Browse files Browse the repository at this point in the history
The SymbolIndex structure owns the memory for symbols and names, and
once added to the Program, it cannot be removed. Making copies of any of
these symbols is purely a waste: we should be able to treat them as
static. So add a lifetime and allow the SymbolIndex to specify static,
avoiding unnecessary copies and frees.

Signed-off-by: Stephen Brennan <[email protected]>
  • Loading branch information
brenns10 committed Sep 30, 2024
1 parent 61c95ab commit 963370d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 19 deletions.
1 change: 1 addition & 0 deletions libdrgn/kallsyms.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ static void symbol_from_kallsyms(uint64_t address, char *name, char kind,
ret->kind = DRGN_SYMBOL_KIND_UNKNOWN;
}
ret->name_lifetime = DRGN_LIFETIME_STATIC;
ret->lifetime = DRGN_LIFETIME_STATIC; // avoid copying
}

/** Compute an address via the CONFIG_KALLSYMS_ABSOLUTE_PERCPU method*/
Expand Down
29 changes: 10 additions & 19 deletions libdrgn/symbol.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ DEFINE_VECTOR_FUNCTIONS(symbol_vector);

LIBDRGN_PUBLIC void drgn_symbol_destroy(struct drgn_symbol *sym)
{
if (sym && sym->lifetime == DRGN_LIFETIME_STATIC)
return;
if (sym && sym->name_lifetime == DRGN_LIFETIME_OWNED)
/* Cast here is necessary - we want symbol users to
* never modify sym->name, but when we own the name,
Expand All @@ -36,6 +38,7 @@ void drgn_symbol_from_elf(const char *name, uint64_t address,
{
ret->name = name;
ret->name_lifetime = DRGN_LIFETIME_STATIC;
ret->lifetime = DRGN_LIFETIME_OWNED;
ret->address = address;
ret->size = elf_sym->st_size;
int binding = GELF_ST_BIND(elf_sym->st_info);
Expand Down Expand Up @@ -83,6 +86,7 @@ drgn_symbol_create(const char *name, uint64_t address, uint64_t size,
sym->binding = binding;
sym->kind = kind;
sym->name_lifetime = name_lifetime;
sym->lifetime = DRGN_LIFETIME_OWNED;
*ret = sym;
return NULL;
}
Expand Down Expand Up @@ -330,21 +334,6 @@ static void address_search_range(struct drgn_symbol_index *index, uint64_t addre
*start_ret = lo;
}

/** Allocate a copy of the symbol and add to it the builder */
static bool add_symbol_result(struct drgn_symbol_result_builder *builder,
struct drgn_symbol *symbol)
{
struct drgn_symbol *copy = malloc(sizeof(*copy));
if (!copy)
return false;
*copy = *symbol;
if (!drgn_symbol_result_builder_add(builder, copy)) {
free(copy);
return false;
}
return true;
}

struct drgn_error *
drgn_symbol_index_find(const char *name, uint64_t address,
enum drgn_find_symbol_flags flags, void *arg,
Expand All @@ -368,7 +357,7 @@ drgn_symbol_index_find(const char *name, uint64_t address,
if ((flags & DRGN_FIND_SYMBOL_NAME) &&
strcmp(s->name, name) != 0)
continue;
if (!add_symbol_result(builder, s))
if (!drgn_symbol_result_builder_add(builder, s))
return &drgn_enomem;
if (flags & DRGN_FIND_SYMBOL_ONE)
break;
Expand All @@ -380,15 +369,15 @@ drgn_symbol_index_find(const char *name, uint64_t address,
return NULL;
for (uint32_t i = it.entry->value.start; i < it.entry->value.end; i++) {
struct drgn_symbol *s = &index->symbols[index->name_sort[i]];
if (!add_symbol_result(builder, s))
if (!drgn_symbol_result_builder_add(builder, s))
return &drgn_enomem;
if (flags & DRGN_FIND_SYMBOL_ONE)
break;
}
} else {
for (int i = 0; i < index->num_syms; i++) {
struct drgn_symbol *s = &index->symbols[i];
if (!add_symbol_result(builder, s))
if (!drgn_symbol_result_builder_add(builder, s))
return &drgn_enomem;
if (flags & DRGN_FIND_SYMBOL_ONE)
break;
Expand Down Expand Up @@ -444,11 +433,13 @@ drgn_symbol_index_init_from_builder(struct drgn_symbol_index *index,
// Now that the name array is finalized, resolve the names to real
// pointers. Update the name lifetime to static, reflecting that the
// symbol name is owned by the finder whose lifetime is bound to the
// program's once it is attached.
// program's once it is attached. The same goes for the symbol. Using
// static lifetimes helps avoid unnecessary copying.
for (size_t i = 0; i < num_syms; i++) {
size_t string_index = (size_t)symbols[i].name;
symbols[i].name = &names[string_index];
symbols[i].name_lifetime = DRGN_LIFETIME_STATIC;
symbols[i].lifetime = DRGN_LIFETIME_STATIC;
}

if (num_syms > UINT32_MAX) {
Expand Down
1 change: 1 addition & 0 deletions libdrgn/symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct drgn_symbol {
enum drgn_symbol_binding binding;
enum drgn_symbol_kind kind;
enum drgn_lifetime name_lifetime;
enum drgn_lifetime lifetime;
};

struct drgn_symbol_finder {
Expand Down

0 comments on commit 963370d

Please sign in to comment.