On Thu, Apr 24, 2025 at 5:52 PM Serhei Makarov <ser...@serhei.io> wrote:
>
> Changes for v4:
>
> - Separate out libdwfl_stacktrace, as requested.
>
> Changes for v3:
>
> - Handle dwfl->process == NULL case in __libdwfl_remove_dwfl_from_tracker.
>
> * * *
>
> The Dwflst_Process_Tracker also includes a dynamicsizehash cache which
> maps process ids to Dwfl * (or rather, dwflst_tracker_dwfl_info *
> allowing the table entry to be replaced). Dwfls created from
> the tracker are automatically added to it, and removed on dwfl_end().
>
> * libdwfl_stacktrace/libdwfl_stacktraceP.h (dwflst_tracker_dwfl_info):
> New typedef, provides indirection to allow a dwfltab entry to be
> invalidated.
> (struct Dwflst_Process_Tracker): add dwfltab.
> (__libdwfl_stacktrace_add_dwfl_to_tracker): New function.
> (__libdwfl_stacktrace_remove_dwfl_from_tracker): New function.
> * libdwfl_stacktrace/dwflst_process_tracker.c
> (dwflst_tracker_begin): Init dwfltab.
> (__libdwfl_stacktrace_add_dwfl_to_tracker): New function; add dwfl
> to dwfltab.
> (__libdwfl_stacktrace_remove_dwfl_from_tracker): New function;
> invalidate dwfl entry, since dynamicsizehash doesn't support
> outright deletion.
> (dwflst_tracker_end): Clean up dwfltab. Lock and iterate the table
> to free tracker->dwfltab.table items.
> * libdwfl_stacktrace/dwflst_tracker_dwfltab.c: New file, instantiates
> lib/dynamicsizehash_concurrent.c to store dwfltracker_dwfl_info
> structs.
> * libdwfl_stacktrace/dwflst_tracker_dwfltab.h: New file, ditto.
> * libdwfl_stacktrace/Makefile.am
> (libdwfl_stacktrace_a_SOURCES): Add dwflst_tracker_dwfltab.c.
> (noinst_HEADERS): Add dwflst_tracker_dwfltab.h.
> * libdwfl/dwfl_frame.c (dwfl_attach_state):
> Call __libdwfl_stacktrace_add_dwfl_to_tracker.
> * libdwfl/dwfl_end.c (dwfl_end): Add INTDEF.
> Call __libdwfl_stacktrace_remove_dwfl_from_tracker.
> * libdwfl/libdwflP.h (INTDECLs): Add dwfl_end.
> ---
> libdwfl/dwfl_end.c | 8 +-
> libdwfl/dwfl_frame.c | 7 +-
> libdwfl/libdwflP.h | 1 +
> libdwfl_stacktrace/Makefile.am | 4 +-
> libdwfl_stacktrace/dwflst_process_tracker.c | 87 ++++++++++++++++++++-
> libdwfl_stacktrace/dwflst_tracker_dwfltab.c | 46 +++++++++++
> libdwfl_stacktrace/dwflst_tracker_dwfltab.h | 42 ++++++++++
> libdwfl_stacktrace/libdwfl_stacktraceP.h | 23 ++++++
> 8 files changed, 213 insertions(+), 5 deletions(-)
> create mode 100644 libdwfl_stacktrace/dwflst_tracker_dwfltab.c
> create mode 100644 libdwfl_stacktrace/dwflst_tracker_dwfltab.h
>
> diff --git a/libdwfl/dwfl_end.c b/libdwfl/dwfl_end.c
> index 7b5ac8a1..d9cf569b 100644
> --- a/libdwfl/dwfl_end.c
> +++ b/libdwfl/dwfl_end.c
> @@ -1,5 +1,5 @@
> /* Finish a session using libdwfl.
> - Copyright (C) 2005, 2008, 2012-2013, 2015 Red Hat, Inc.
> + Copyright (C) 2005, 2008, 2012-2013, 2015, 2025 Red Hat, Inc.
> This file is part of elfutils.
>
> This file is free software; you can redistribute it and/or modify
> @@ -31,6 +31,7 @@
> #endif
>
> #include "libdwflP.h"
> +#include "libdwfl_stacktraceP.h"
>
> void
> dwfl_end (Dwfl *dwfl)
> @@ -42,6 +43,9 @@ dwfl_end (Dwfl *dwfl)
> __libdwfl_debuginfod_end (dwfl->debuginfod);
> #endif
>
> + if (dwfl->tracker != NULL)
> + __libdwfl_stacktrace_remove_dwfl_from_tracker (dwfl);
> +
> if (dwfl->process)
> __libdwfl_process_free (dwfl->process);
>
> @@ -68,3 +72,5 @@ dwfl_end (Dwfl *dwfl)
> }
> free (dwfl);
> }
> +INTDEF(dwfl_end)
> +
> diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
> index 2e6c6de8..0d619887 100644
> --- a/libdwfl/dwfl_frame.c
> +++ b/libdwfl/dwfl_frame.c
> @@ -1,5 +1,5 @@
> /* Get Dwarf Frame state for target PID or core file.
> - Copyright (C) 2013, 2014, 2024 Red Hat, Inc.
> + Copyright (C) 2013, 2014, 2024-2025 Red Hat, Inc.
> This file is part of elfutils.
>
> This file is free software; you can redistribute it and/or modify
> @@ -33,6 +33,7 @@
> #include <system.h>
>
> #include "libdwflP.h"
> +#include "libdwfl_stacktraceP.h"
>
> /* Set STATE->pc_set from STATE->regs according to the backend. Return true
> on
> success, false on error. */
> @@ -206,6 +207,10 @@ dwfl_attach_state (Dwfl *dwfl, Elf *elf, pid_t pid,
> process->pid = pid;
> process->callbacks = thread_callbacks;
> process->callbacks_arg = arg;
> +
> + if (dwfl->tracker != NULL)
> + __libdwfl_stacktrace_add_dwfl_to_tracker (dwfl);
> +
> return true;
> }
> INTDEF(dwfl_attach_state)
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index 57305f81..b622f8aa 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -757,6 +757,7 @@ extern int dwfl_link_map_report (Dwfl *dwfl, const void
> *auxv, size_t auxv_size,
>
> /* Avoid PLT entries. */
> INTDECL (dwfl_begin)
> +INTDECL (dwfl_end)
> INTDECL (dwfl_errmsg)
> INTDECL (dwfl_errno)
> INTDECL (dwfl_addrmodule)
> diff --git a/libdwfl_stacktrace/Makefile.am b/libdwfl_stacktrace/Makefile.am
> index ffddec0c..99a80b5c 100644
> --- a/libdwfl_stacktrace/Makefile.am
> +++ b/libdwfl_stacktrace/Makefile.am
> @@ -43,6 +43,7 @@ pkginclude_HEADERS = libdwfl_stacktrace.h
> libdwfl_stacktrace_a_SOURCES = dwflst_process_tracker.c \
> dwflst_tracker_find_elf.c \
> dwflst_tracker_elftab.c \
> + dwflst_tracker_dwfltab.c \
> libdwfl_stacktrace_next_prime.c \
> dwflst_perf_frame.c
>
> @@ -55,7 +56,8 @@ libeu = ../lib/libeu.a
> libdwfl_stacktrace_pic_a_SOURCES =
> am_libdwfl_stacktrace_pic_a_OBJECTS = $(libdwfl_stacktrace_a_SOURCES:.c=.os)
>
> -noinst_HEADERS = libdwfl_stacktraceP.h dwflst_tracker_elftab.h
> +noinst_HEADERS = libdwfl_stacktraceP.h \
> + dwflst_tracker_elftab.h dwflst_tracker_dwfltab.h
>
> EXTRA_libdwfl_stacktrace_a_DEPENDENCIES = libdwfl_stacktrace.manifest
>
> diff --git a/libdwfl_stacktrace/dwflst_process_tracker.c
> b/libdwfl_stacktrace/dwflst_process_tracker.c
> index 10cd9a7c..f72b72b0 100644
> --- a/libdwfl_stacktrace/dwflst_process_tracker.c
> +++ b/libdwfl_stacktrace/dwflst_process_tracker.c
> @@ -45,6 +45,8 @@ Dwflst_Process_Tracker *dwflst_tracker_begin (const
> Dwfl_Callbacks *callbacks)
>
> dwflst_tracker_elftab_init (&tracker->elftab, HTAB_DEFAULT_SIZE);
> rwlock_init (tracker->elftab_lock);
> + dwflst_tracker_dwfltab_init (&tracker->dwfltab, HTAB_DEFAULT_SIZE);
> + rwlock_init (tracker->dwfltab_lock);
>
> tracker->callbacks = callbacks;
> return tracker;
> @@ -58,19 +60,84 @@ Dwfl *dwflst_tracker_dwfl_begin (Dwflst_Process_Tracker
> *tracker)
>
> /* TODO: Could also share dwfl->debuginfod, but thread-safely? */
> dwfl->tracker = tracker;
> +
> + /* XXX: dwfl added to dwfltab when dwfl->process set in dwfl_attach_state.
> */
> + /* XXX: dwfl removed from dwfltab in dwfl_end() */
> +
> return dwfl;
> }
>
> +
> +void
> +internal_function
> +__libdwfl_stacktrace_add_dwfl_to_tracker (Dwfl *dwfl) {
> + Dwflst_Process_Tracker *tracker = dwfl->tracker;
> + assert (tracker != NULL);
> +
> + /* First try to find an existing entry to replace: */
> + dwflst_tracker_dwfl_info *ent = NULL;
> + unsigned long int hval = dwfl->process->pid;
> +
> + rwlock_wrlock (tracker->dwfltab_lock);
> + ent = dwflst_tracker_dwfltab_find(&tracker->dwfltab, hval);
> + if (ent != NULL)
> + {
> + /* TODO: This is a bare-minimum solution. Ideally
> + we would clean up the existing ent->dwfl, but
> + this needs to be coordinated with any users of
> + the dwfl library that might still be holding it. */
> + ent->dwfl = dwfl;
> + ent->invalid = false;
> + rwlock_unlock (tracker->dwfltab_lock);
> + return;
> + }
> +
> + /* Only otherwise try to insert an entry: */
> + ent = calloc (1, sizeof(dwflst_tracker_dwfl_info));
Missing NULL check.
> + ent->dwfl = dwfl;
> + ent->invalid = false;
> + if (dwflst_tracker_dwfltab_insert(&tracker->dwfltab, hval, ent) != 0)
> + {
> + free(ent);
> + rwlock_unlock (tracker->dwfltab_lock);
> + assert(false); /* Should not occur due to the wrlock on dwfltab. */
> + }
> + rwlock_unlock (tracker->dwfltab_lock);
> +}
> +
> +void
> +internal_function
> +__libdwfl_stacktrace_remove_dwfl_from_tracker (Dwfl *dwfl) {
> + if (dwfl->tracker == NULL)
> + return;
> + Dwflst_Process_Tracker *tracker = dwfl->tracker;
> + dwflst_tracker_dwfl_info *ent = NULL;
> + if (dwfl->process == NULL)
> + return;
> + unsigned long int hval = dwfl->process->pid;
> +
> + rwlock_wrlock (tracker->dwfltab_lock);
> + ent = dwflst_tracker_dwfltab_find(&tracker->dwfltab, hval);
> + if (ent != NULL && ent->dwfl == dwfl)
> + {
> + ent->dwfl = NULL;
> + ent->invalid = true;
> + }
> + rwlock_unlock (tracker->dwfltab_lock);
> +}
> +
> void dwflst_tracker_end (Dwflst_Process_Tracker *tracker)
> {
> if (tracker == NULL)
> return;
>
> + size_t idx;
> +
> /* HACK to allow iteration of dynamicsizehash_concurrent. */
> /* XXX Based on lib/dynamicsizehash_concurrent.c free(). */
> rwlock_fini (tracker->elftab_lock);
> pthread_rwlock_destroy(&tracker->elftab.resize_rwl);
> - for (size_t idx = 1; idx <= tracker->elftab.size; idx++)
> + for (idx = 1; idx <= tracker->elftab.size; idx++)
> {
> dwflst_tracker_elftab_ent *ent = &tracker->elftab.table[idx];
> if (ent->hashval == 0)
> @@ -87,6 +154,22 @@ void dwflst_tracker_end (Dwflst_Process_Tracker *tracker)
> }
> free (tracker->elftab.table);
>
> - /* TODO: Call dwfl_end for each Dwfl connected to this tracker. */
> + /* XXX Based on lib/dynamicsizehash_concurrent.c free(). */
> + rwlock_fini (tracker->dwfltab_lock);
> + pthread_rwlock_destroy(&tracker->dwfltab.resize_rwl);
> + for (idx = 1; idx <= tracker->dwfltab.size; idx++)
> + {
> + dwflst_tracker_dwfltab_ent *ent = &tracker->dwfltab.table[idx];
> + if (ent->hashval == 0)
> + continue;
> + dwflst_tracker_dwfl_info *t =
> + (dwflst_tracker_dwfl_info *) atomic_load_explicit (&ent->val_ptr,
> +
> memory_order_relaxed);
> + if (t->dwfl != NULL)
> + INTUSE(dwfl_end) (t->dwfl);
> + free(t);
> + }
> + free (tracker->dwfltab.table);
> +
> free (tracker);
> }
> diff --git a/libdwfl_stacktrace/dwflst_tracker_dwfltab.c
> b/libdwfl_stacktrace/dwflst_tracker_dwfltab.c
> new file mode 100644
> index 00000000..f4749e29
> --- /dev/null
> +++ b/libdwfl_stacktrace/dwflst_tracker_dwfltab.c
> @@ -0,0 +1,46 @@
> +/* Dwflst_Process_Tracker Dwfl table implementation.
> + Copyright (C) 2025 Red Hat, Inc.
> + This file is part of elfutils.
> +
> + This file is free software; you can redistribute it and/or modify
> + it under the terms of either
> +
> + * the GNU Lesser General Public License as published by the Free
> + Software Foundation; either version 3 of the License, or (at
> + your option) any later version
> +
> + or
> +
> + * the GNU General Public License as published by the Free
> + Software Foundation; either version 2 of the License, or (at
> + your option) any later version
> +
> + or both in parallel, as here.
> +
> + elfutils is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received copies of the GNU General Public License and
> + the GNU Lesser General Public License along with this program. If
> + not, see <http://www.gnu.org/licenses/>. */
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include <libdwfl_stacktraceP.h>
> +
> +/* Definitions for the Dwfl table. */
> +#define TYPE dwflst_tracker_dwfl_info *
> +#define NAME dwflst_tracker_dwfltab
> +#define ITERATE 1
> +/* TODO(REVIEW): Omit reverse? */
> +#define REVERSE 1
My understanding of lib/dynamicsizehash.c is that REVERSE doesn't need
to be defined if you do not require reverse iteration. I'm not sure if
you do or not.
> +#define COMPARE(a, b) \
> + ((a->invalid && b->invalid) || \
> + (!a->invalid && !b->invalid && \
> + (a)->dwfl->process->pid == (b)->dwfl->process->pid))
> +
> +#include "../lib/dynamicsizehash_concurrent.c"
> diff --git a/libdwfl_stacktrace/dwflst_tracker_dwfltab.h
> b/libdwfl_stacktrace/dwflst_tracker_dwfltab.h
> new file mode 100644
> index 00000000..fd687e61
> --- /dev/null
> +++ b/libdwfl_stacktrace/dwflst_tracker_dwfltab.h
> @@ -0,0 +1,42 @@
> +/* Dwflst_Process_Tracker Dwfl table.
> + Copyright (C) 2025 Red Hat, Inc.
> + This file is part of elfutils.
> +
> + This file is free software; you can redistribute it and/or modify
> + it under the terms of either
> +
> + * the GNU Lesser General Public License as published by the Free
> + Software Foundation; either version 3 of the License, or (at
> + your option) any later version
> +
> + or
> +
> + * the GNU General Public License as published by the Free
> + Software Foundation; either version 2 of the License, or (at
> + your option) any later version
> +
> + or both in parallel, as here.
> +
> + elfutils is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received copies of the GNU General Public License and
> + the GNU Lesser General Public License along with this program. If
> + not, see <http://www.gnu.org/licenses/>. */
> +
> +#ifndef DWFLST_TRACKER_DWFLTAB_H
> +#define DWFLST_TRACKER_DWFLTAB_H 1
> +
> +/* Definitions for the Dwfl table. */
> +#define TYPE dwflst_tracker_dwfl_info *
> +#define NAME dwflst_tracker_dwfltab
> +#define ITERATE 1
> +#define COMPARE(a, b) \
> + ((a->invalid && b->invalid) || \
> + (!a->invalid && !b->invalid && \
> + (a)->dwfl->process->pid == (b)->dwfl->process->pid))
> +#include <dynamicsizehash_concurrent.h>
> +
> +#endif
> diff --git a/libdwfl_stacktrace/libdwfl_stacktraceP.h
> b/libdwfl_stacktrace/libdwfl_stacktraceP.h
> index e457d35a..0bfc9c83 100644
> --- a/libdwfl_stacktrace/libdwfl_stacktraceP.h
> +++ b/libdwfl_stacktrace/libdwfl_stacktraceP.h
> @@ -45,6 +45,14 @@ typedef struct
> } dwflst_tracker_elf_info;
> #include "dwflst_tracker_elftab.h"
>
> +/* Hash table for Dwfl *. */
> +typedef struct
> +{
> + Dwfl *dwfl;
> + bool invalid; /* Mark when the dwfl has been removed. */
> +} dwflst_tracker_dwfl_info;
> +#include "dwflst_tracker_dwfltab.h"
> +
> struct Dwflst_Process_Tracker
> {
> const Dwfl_Callbacks *callbacks;
> @@ -52,9 +60,24 @@ struct Dwflst_Process_Tracker
> /* Table of cached Elf * including fd, path, fstat info. */
> dwflst_tracker_elftab elftab;
> rwlock_define(, elftab_lock);
> +
> + /* Table of cached Dwfl * including pid. */
> + dwflst_tracker_dwfltab dwfltab;
> + rwlock_define(, dwfltab_lock);
> };
>
>
> +/* Called when dwfl->process->pid becomes known to add the dwfl to its
> + Dwflst_Process_Tracker's dwfltab: */
> +extern void __libdwfl_stacktrace_add_dwfl_to_tracker (Dwfl *dwfl)
> + internal_function;
> +
> +/* Called from dwfl_end() to remove the dwfl from its
> + Dwfl_Process_Tracker's dwfltab: */
> +extern void __libdwfl_stacktrace_remove_dwfl_from_tracker (Dwfl *dwfl)
> + internal_function;
> +
> +
> /* Avoid PLT entries. */
> INTDECL (dwflst_module_gettracker)
> INTDECL (dwflst_tracker_find_cached_elf)
> --
> 2.47.0
>
Aaron
