Hi Serhei, On Tue, Mar 24, 2026 at 6:38 PM Serhei Makarov <[email protected]> wrote: > > A bunch of x86 sample_regs code should be common across > architectures. In particular, sample_sp_pc() implementation looks to > be universally applicable apart from the register positions. > > * backends/x86_initreg_sample.c (x86_sample_sp_pc): Remove in favour > of generic_sample_sp_pc (added to libebl_PERF_FLAGS.h in prev patch). > (x86_sample_perf_regs_mapping): This should have been an inline > function. Fix. > * backends/i386_init.c (i386_init): Use > __libebl_init_cached_regs_mapping. > * backends/x86_64_init.c (x86_64_init): Use > __libebl_init_cached_regs_mapping. > * backends/i386_initreg_sample.c (i386_sample_sp_pc): Use > generic_sample_sp_pc. > * backends/x86_64_init.c (x86_64_initreg_sample): Use > generic_sample_sp_pc.
Missing Signed-off-by: here and in the other patches in this series. Otherwise this patch LGTM. Aaron > --- > backends/i386_init.c | 8 +++--- > backends/i386_initreg_sample.c | 18 +++---------- > backends/x86_64_init.c | 8 +++--- > backends/x86_64_initreg_sample.c | 14 ++-------- > backends/x86_initreg_sample.c | 44 +++----------------------------- > 5 files changed, 15 insertions(+), 77 deletions(-) > > diff --git a/backends/i386_init.c b/backends/i386_init.c > index a980e71a..bdd07508 100644 > --- a/backends/i386_init.c > +++ b/backends/i386_init.c > @@ -1,5 +1,5 @@ > /* Initialization of i386 specific backend library. > - Copyright (C) 2000-2009, 2013, 2017, 2025 Red Hat, Inc. > + Copyright (C) 2000-2009, 2013, 2017, 2025-2026 Red Hat, Inc. > This file is part of elfutils. > Written by Ulrich Drepper <[email protected]>, 2000. > > @@ -60,13 +60,11 @@ i386_init (Elf *elf __attribute__ ((unused)), > (Likely an artifact of reusing that header between i386/x86_64.) */ > eh->frame_nregs = 9; > HOOK (eh, set_initial_registers_tid); > - /* set_initial_registers_sample is default ver */ > + /* set_initial_registers_sample is default ver */ > HOOK (eh, sample_sp_pc); > HOOK (eh, sample_perf_regs_mapping); > eh->perf_frame_regs_mask = PERF_FRAME_REGISTERS_I386; > - eh->cached_perf_regs_mask = 0; > - eh->cached_regs_mapping = NULL; > - eh->cached_n_regs_mapping = -1; > + __libebl_init_cached_regs_mapping (eh); > HOOK (eh, unwind); > > return eh; > diff --git a/backends/i386_initreg_sample.c b/backends/i386_initreg_sample.c > index d7d312b0..ae3ab11e 100644 > --- a/backends/i386_initreg_sample.c > +++ b/backends/i386_initreg_sample.c > @@ -1,5 +1,5 @@ > /* Populate process registers from a linux perf_events sample. > - Copyright (C) 2025 Red Hat, Inc. > + Copyright (C) 2025-2026 Red Hat, Inc. > This file is part of elfutils. > > This file is free software; you can redistribute it and/or modify > @@ -47,23 +47,13 @@ > > bool > i386_sample_sp_pc (const Dwarf_Word *regs, uint32_t n_regs, > - const int *regs_mapping, uint32_t n_regs_mapping, > - Dwarf_Word *sp, Dwarf_Word *pc) > + const int *regs_mapping, uint32_t n_regs_mapping, > + Dwarf_Word *sp, Dwarf_Word *pc) > { > -#ifdef HAVE_X86_INITREG_SAMPLE > /* XXX for dwarf_regs indices, compare i386_initreg.c */ > - return x86_sample_sp_pc (regs, n_regs, regs_mapping, n_regs_mapping, > + return generic_sample_sp_pc (regs, n_regs, regs_mapping, n_regs_mapping, > sp, 4 /* index of sp in dwarf_regs */, > pc, 8 /* index of pc in dwarf_regs */); > -#else > - (void) regs; > - (void) n_regs; > - (void) regs_mapping; > - (void) n_regs_mapping; > - (void) sp; > - (void) pc; > - return false; > -#endif > } > > bool > diff --git a/backends/x86_64_init.c b/backends/x86_64_init.c > index 5f929758..a3190fbc 100644 > --- a/backends/x86_64_init.c > +++ b/backends/x86_64_init.c > @@ -1,5 +1,5 @@ > /* Initialization of x86-64 specific backend library. > - Copyright (C) 2002-2009, 2013, 2018, 2025 Red Hat, Inc. > + Copyright (C) 2002-2009, 2013, 2018, 2025-2026 Red Hat, Inc. > Copyright (C) H.J. Lu <[email protected]>, 2015. > This file is part of elfutils. > Written by Ulrich Drepper <[email protected]>, 2002. > @@ -63,13 +63,11 @@ x86_64_init (Elf *elf __attribute__ ((unused)), > /* gcc/config/ #define DWARF_FRAME_REGISTERS. */ > eh->frame_nregs = 17; > HOOK (eh, set_initial_registers_tid); > - /* set_initial_registers_sample is default ver */ > + /* set_initial_registers_sample is default ver */ > HOOK (eh, sample_sp_pc); > HOOK (eh, sample_perf_regs_mapping); > eh->perf_frame_regs_mask = PERF_FRAME_REGISTERS_X86_64; > - eh->cached_perf_regs_mask = 0; > - eh->cached_regs_mapping = NULL; > - eh->cached_n_regs_mapping = -1; > + __libebl_init_cached_regs_mapping (eh); > HOOK (eh, unwind); > HOOK (eh, check_reloc_target_type); > > diff --git a/backends/x86_64_initreg_sample.c > b/backends/x86_64_initreg_sample.c > index 200a94a1..83966ff9 100644 > --- a/backends/x86_64_initreg_sample.c > +++ b/backends/x86_64_initreg_sample.c > @@ -1,5 +1,5 @@ > /* Populate process registers from a linux perf_events sample. > - Copyright (C) 2025 Red Hat, Inc. > + Copyright (C) 2025-2026 Red Hat, Inc. > This file is part of elfutils. > > This file is free software; you can redistribute it and/or modify > @@ -50,20 +50,10 @@ x86_64_sample_sp_pc (const Dwarf_Word *regs, uint32_t > n_regs, > const int *regs_mapping, uint32_t n_regs_mapping, > Dwarf_Word *sp, Dwarf_Word *pc) > { > -#ifdef HAVE_X86_INITREG_SAMPLE > /* XXX for dwarf_regs indices, compare x86_64_initreg.c */ > - return x86_sample_sp_pc (regs, n_regs, regs_mapping, n_regs_mapping, > + return generic_sample_sp_pc (regs, n_regs, regs_mapping, n_regs_mapping, > sp, 7 /* index of sp in dwarf_regs */, > pc, 16 /* index of pc in dwarf_regs */); > -#else > - (void) regs; > - (void) n_regs; > - (void) regs_mapping; > - (void) n_regs_mapping; > - (void) sp; > - (void) pc; > - return false; > -#endif > } > > bool > diff --git a/backends/x86_initreg_sample.c b/backends/x86_initreg_sample.c > index 6f99bf53..fe870e61 100644 > --- a/backends/x86_initreg_sample.c > +++ b/backends/x86_initreg_sample.c > @@ -1,5 +1,5 @@ > -/* x86 stack sample register handling, pieces common to x86-64 and i386. > - Copyright (C) 2025 Red Hat, Inc. > +/* x86 stack sample register handling helper common to x86-64 and i386. > + Copyright (C) 2025-2026 Red Hat, Inc. > This file is part of elfutils. > > This file is free software; you can redistribute it and/or modify > @@ -26,45 +26,7 @@ > the GNU Lesser General Public License along with this program. If > not, see <http://www.gnu.org/licenses/>. */ > > -static bool > -x86_sample_sp_pc (const Dwarf_Word *regs, uint32_t n_regs, > - const int *regs_mapping, uint32_t n_regs_mapping, > - Dwarf_Word *sp, uint sp_index /* into dwarf_regs */, > - Dwarf_Word *pc, uint pc_index /* into dwarf_regs */) > -{ > - if (sp != NULL) *sp = 0; > - if (pc != NULL) *pc = 0; > -#if !defined(__x86_64__) > - (void)regs; > - (void)n_regs; > - (void)regs_mapping; > - (void)n_regs_mapping; > - (void)sp; > - (void)sp_index; > - (void)pc; > - (void)pc_index; > - return false; > -#else /* __x86_64__ */ > - /* TODO: Register locations could be cached and rechecked on a > - fastpath without needing to loop? */ > - int j, need_sp = (sp != NULL), need_pc = (pc != NULL); > - for (j = 0; (need_sp || need_pc) && n_regs_mapping > (uint32_t)j; j++) > - { > - if (n_regs < (uint32_t)j) break; > - if (need_sp && regs_mapping[j] == (int)sp_index) > - { > - *sp = regs[j]; need_sp = false; > - } > - if (need_pc && regs_mapping[j] == (int)pc_index) > - { > - *pc = regs[j]; need_pc = false; > - } > - } > - return (!need_sp && !need_pc); > -#endif > -} > - > -static bool > +static inline bool > x86_sample_perf_regs_mapping (Ebl *ebl, > uint64_t perf_regs_mask, uint32_t abi, > const int **regs_mapping, > -- > 2.53.0 >
