Hi Serhei,
On Sun, Mar 16, 2025 at 07:14:11PM -0400, Serhei Makarov wrote:
> Since libebl is a private interface, subsequent patches in the series
> introduce another api wrapping the libebl perf register handling. In
> this patch, add an interface to access the preferred set of registers
> that libdwfl would like to see to allow proper unwinding of stack
> sample data.
>
> * libdwfl/libdwfl.h (dwfl_perf_sample_preferred_regs_mask):
> New function.
> * libdwfl/dwfl_perf_frame.c: New file.
> (dwfl_perf_sample_preferred_regs_mask): New function.
> * libdw/libdw.map: Add dwfl_perf_sample_preferred_regs_mask.
> * libdwfl/Makefile.am: Add dwfl_perf_frame.c.
I probably need to read the rest of the patch series first. On its own
I am not a fan, it feels very much an implementation detail that
should be abstracted away.
> ---
> libdw/libdw.map | 1 +
> libdwfl/Makefile.am | 3 +-
> libdwfl/dwfl_perf_frame.c | 61 +++++++++++++++++++++++++++++++++++++++
> libdwfl/libdwfl.h | 10 +++++++
> 4 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100644 libdwfl/dwfl_perf_frame.c
>
> diff --git a/libdw/libdw.map b/libdw/libdw.map
> index afbc467f..3218fd9c 100644
> --- a/libdw/libdw.map
> +++ b/libdw/libdw.map
> @@ -389,4 +389,5 @@ ELFUTILS_0.192 {
> ELFUTILS_0.193 {
> global:
> dwfl_set_initial_registers_thread;
> + dwfl_perf_sample_preferred_regs_mask;
> } ELFUTILS_0.192;
> diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
> index b30b86f0..37c57bee 100644
> --- a/libdwfl/Makefile.am
> +++ b/libdwfl/Makefile.am
> @@ -2,7 +2,7 @@
> ##
> ## Process this file with automake to create Makefile.in
> ##
> -## Copyright (C) 2005-2010, 2013 Red Hat, Inc.
> +## Copyright (C) 2005-2010, 2013, 2025 Red Hat, Inc.
> ## This file is part of elfutils.
> ##
> ## This file is free software; you can redistribute it and/or modify
> @@ -71,6 +71,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c
> dwfl_version.c \
> link_map.c core-file.c open.c image-header.c \
> dwfl_frame.c frame_unwind.c dwfl_frame_pc.c \
> linux-pid-attach.c linux-core-attach.c dwfl_frame_regs.c \
> + dwfl_perf_frame.c \
> gzip.c debuginfod-client.c
>
> if BZLIB
> diff --git a/libdwfl/dwfl_perf_frame.c b/libdwfl/dwfl_perf_frame.c
> new file mode 100644
> index 00000000..87db97bc
> --- /dev/null
> +++ b/libdwfl/dwfl_perf_frame.c
> @@ -0,0 +1,61 @@
> +/* Get Dwarf Frame state for perf stack sample data.
> + 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 <linux/perf_event.h>
This should probably be guarded by #ifdef __linux__ or some configure
check? Or... where is this used?
> +#include "libdwflP.h"
> +
> +Ebl *default_ebl = NULL;
> +GElf_Half default_ebl_machine = EM_NONE;
Make these static.
> +uint64_t dwfl_perf_sample_preferred_regs_mask (GElf_Half machine)
> +{
> + /* XXX The most likely case is that this will only be called once,
> + for the current architecture. So we keep one Ebl* around for
> + answering this query and replace it in the unlikely case of
> + getting called with different architectures. */
> + if (default_ebl != NULL && default_ebl_machine != machine)
> + {
> + ebl_closebackend(default_ebl);
> + default_ebl = NULL;
> + }
> + if (default_ebl == NULL)
> + {
> + default_ebl = ebl_openbackend_machine(machine);
> + default_ebl_machine = machine;
> + }
> + if (default_ebl != NULL)
> + return ebl_perf_frame_regs_mask (default_ebl);
> + return 0;
> +}
> +
> +/* XXX dwfl_perf_sample_getframes to be added in subsequent patch */
> diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
> index b3b32d51..29505863 100644
> --- a/libdwfl/libdwfl.h
> +++ b/libdwfl/libdwfl.h
> @@ -825,6 +825,16 @@ int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
> void *arg)
> __nonnull_attribute__ (1, 3);
>
> +/* XXX dwfl_perf_sample_getframes to be added in subsequent patch */
> +
> +/* Returns the linux perf_events register mask describing a set of
> + registers sufficient for unwinding on MACHINE, or 0 if libdwfl does
> + not handle perf_events samples for MACHINE. Does not take a Dwfl*
> + or Elf* since this is meant to allow a profiling tool to configure
> + perf_events to produce meaningful data for a libdwfl session to be
> + opened later. */
> +uint64_t dwfl_perf_sample_preferred_regs_mask (GElf_Half machine);
> +
> /* Return *PC (program counter) for thread-specific frame STATE.
> Set *ISACTIVATION according to DWARF frame "activation" definition.
> Typically you need to subtract 1 from *PC if *ACTIVATION is false to
> safely
> --
> 2.47.0
>
