Hi Serhei,
On Sun, Mar 16, 2025 at 07:13:41PM -0400, Serhei Makarov wrote:
> Change the sample_set_initial_registers callback in eu-stacktrace to
> use the proper libebl ebl_set_initial_registers_sample function.
>
> * src/Makefile.am (stacktrace_LDADD): Add libebl.
> * src/stacktrace.c (sample_registers_cb): New function,
> though identical to pid_thread_state_registers_cb.
> (sample_set_initial_registers): Invoke
> ebl_set_initial_registers_sample instead of containing
> platform-specific code directly.
Yes, this seems the logical consequence of patch 01/13 (it could even
be folded into it.
Cheers,
Mark
> ---
> src/Makefile.am | 4 ++--
> src/stacktrace.c | 48 +++++++++++++-----------------------------------
> 2 files changed, 15 insertions(+), 37 deletions(-)
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index ed245fc1..6d713e88 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1,6 +1,6 @@
> ## Process this file with automake to create Makefile.in
> ##
> -## Copyright (C) 1996-2014, 2016, 2024 Red Hat, Inc.
> +## Copyright (C) 1996-2014, 2016, 2024-2025 Red Hat, Inc.
> ## This file is part of elfutils.
> ##
> ## This file is free software; you can redistribute it and/or modify
> @@ -105,7 +105,7 @@ ar_LDADD = libar.a $(libelf) $(libeu) $(argp_LDADD)
> $(obstack_LIBS)
> unstrip_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> stack_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> $(demanglelib)
> if ENABLE_STACKTRACE
> -stacktrace_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> +stacktrace_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> endif
> elfcompress_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> elfclassify_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD)
> diff --git a/src/stacktrace.c b/src/stacktrace.c
> index d8699ce5..7b498e40 100644
> --- a/src/stacktrace.c
> +++ b/src/stacktrace.c
> @@ -1,5 +1,5 @@
> /* Process a stream of stack samples into stack traces.
> - Copyright (C) 2023-2024 Red Hat, Inc.
> + Copyright (C) 2023-2025 Red Hat, Inc.
> This file is part of elfutils.
>
> This file is free software; you can redistribute it and/or modify
> @@ -93,9 +93,11 @@
> * Includes: libdwfl data structures *
> *************************************/
>
> +#include ELFUTILS_HEADER(ebl)
> /* #include ELFUTILS_HEADER(dwfl) */
> #include "../libdwfl/libdwflP.h"
> -/* XXX: Private header needed for find_procfile, sysprof_init_dwfl */
> +/* XXX: Private header needed for find_procfile, sysprof_init_dwfl,
> + sample_set_initial_registers. */
>
> /*************************************
> * Includes: sysprof data structures *
> @@ -574,42 +576,18 @@ sample_memory_read (Dwfl *dwfl, Dwarf_Addr addr,
> Dwarf_Word *result, void *arg)
> return true;
> }
>
> -/* TODO: Need to generalize this code beyond x86 architectures. */
> static bool
> -sample_set_initial_registers (Dwfl_Thread *thread, void *thread_arg)
> +sample_set_initial_registers (Dwfl_Thread *thread, void *arg)
> {
> - /* The following facts are needed to translate x86 registers correctly:
> - - perf register order seen in linux
> arch/x86/include/uapi/asm/perf_regs.h
> - The registers array is built in the same order as the enum!
> - (See the code in tools/perf/util/intel-pt.c intel_pt_add_gp_regs().)
> - - sysprof libsysprof/perf-event-stream-private.h records all registers
> - except segment and flags.
> - - TODO: Should include the perf regs mask in sysprof data and
> - translate registers in fully-general fashion, removing this
> assumption.
> - - dwarf register order seen in elfutils
> backends/{x86_64,i386}_initreg.c;
> - and it's a fairly different register order!
> -
> - For comparison, you can study
> codereview.qt-project.org/gitweb?p=qt-creator/perfparser.git;a=blob;f=app/perfregisterinfo.cpp;hb=HEAD
> - and follow the code which uses those tables of magic numbers.
> - But it's better to follow original sources of truth for this. */
> - struct __sample_arg *sample_arg = (struct __sample_arg *)thread_arg;
> - bool is_abi32 = (sample_arg->abi == PERF_SAMPLE_REGS_ABI_32);
> - static const int regs_i386[] = {0, 2, 3, 1, 7/*sp*/, 6, 4, 5, 8/*ip*/};
> - static const int regs_x86_64[] = {0, 3, 2, 1, 4, 5, 6, 7/*sp*/, 9, 10, 11,
> 12, 13, 14, 15, 16, 8/*ip*/};
> - const int *reg_xlat = is_abi32 ? regs_i386 : regs_x86_64;
> - int n_regs = is_abi32 ? 9 : 17;
> + struct __sample_arg *sample_arg = (struct __sample_arg *)arg;
> dwfl_thread_state_register_pc (thread, sample_arg->pc);
> - if (sample_arg->n_regs < (uint64_t)n_regs && show_failures)
> - fprintf(stderr, N_("sample_set_initial_regs: n_regs=%ld, expected %d\n"),
> - sample_arg->n_regs, n_regs);
> - for (int i = 0; i < n_regs; i++)
> - {
> - int j = reg_xlat[i];
> - if (j < 0) continue;
> - if (sample_arg->n_regs <= (uint64_t)j) continue;
> - dwfl_thread_state_registers (thread, i, 1, &sample_arg->regs[j]);
> - }
> - return true;
> + Dwfl_Process *process = thread->process;
> + Ebl *ebl = process->ebl;
> + /* XXX Sysprof provides exactly the required registers for unwinding: */
> + uint64_t regs_mask = ebl_perf_frame_regs_mask (ebl);
> + return ebl_set_initial_registers_sample
> + (ebl, sample_arg->regs, sample_arg->n_regs, regs_mask, sample_arg->abi,
> + dwfl_set_initial_registers_thread, thread);
> }
>
> static void
> --
> 2.47.0
>
