Hi Serhei, On Wed, 2025-08-20 at 18:12 -0400, Serhei Makarov wrote: > The header-only patch below illustrates the planned API change. > The base solution is for dwflst_sample_getframes() to take a pointer > to regs_mapping, which, for each item in regs[] array, specifies its > position in the full register file expected by the DWARF functionality. > (This avoids the need to take a perf_events regs bitmap that can only > be interpreted with the aid of a weird enum header buried in the linux > kernel tree.) > > The regs_mapping array is most likely to stay constant for an entire > profiling session, so this is not a high-overhead interface. > (A bit controversially, I'm allowing n_regs_mapping != n_regs, which > might smoothly deal with cases where dwflst_sample_getframes is passed > a profiling data packet with malformed or truncated regs array.) > > We *keep* the existing dwflst_perf_sample_getframes as a convenience > variant of dwflst_sample_getframes. It clearly says 'perf', and I > think it's fine to have this perf-specific API as long as it's not > the *only* API. > > I believe this is the most significant obstacle to turning > libdwfl_stacktrace into a stable API. > mjw, do any other major issues come to mind? > (I'm sure we will find minor things to fix when we bikeshed this header, of > course.) > > I could add functions to generate a plausible regs_mapping for various > architectures, > though I'd like a non perf_events test case for that. I've looked at BSD > PmcTools, > for one example, but not 100% sure yet how to bolt libdwfl_stacktrace into it.
I have to admit I had to reread things for the above to make sense. So for libdwfl_stacktrace we have a Dwflst_Process_Tracker. You create a Dwflst_Process_Tracker with a set of Dwfl_Callbacks (from libdwfl.h) which sets up the find_elf, find_debuginfo and debuginfo_path. Then from this Dwflst_Process_Tracker you create new Dwfls with dwflst_tracker_dwfl_begin instead of calling dwfl_begin with a new set of callbacks yourself. You would still setup the Dwfl with dwfl_[linux_proc]_report[_*] calls. But not attach Dwfl_Thread_Callbacks state because that is done dynamically by the dwflst_*_getframes functions. Alternatively you use dwflst_tracker_find_pid to get or create a new Dwflst_Process_Tracker owned Dwfl. Then finally you call dwflst_perf_sample_getframes or the new dwflst_sample_getframes to iterate to the frames of a specific Dwfl. Which finally brings us to the topic of this email :) Could you explain again why we have the first set of arguments? Dwfl *dwfl, Elf *elf, pid_t pid, pid_t tid Some of this seems a little repetitive. Ideally I think you would just provide a Dwflst_Process_Tracker and a tid. Assuming you can easily get the pid from a tid (maybe not?). At least giving both Dwfl and pid seems redundant. Also which Elf would you provide? A Dwfl can contain multiple Dwfl_Modules each with their own associated Elf. Side note, there is dwfl_getthreads, but I assume that isn't how you iterate through threads with an Dwflst_Process_Tracker derived Dwfl? Then you get the stack (and size) and registers (as Dwarf_Words and number of regs). So we just assume this is provided by some external mechanism. And finally the n_regs_mapping which for each DWARF register number maps it to an index in the regs array. So that the callback can get a register value from the given Dwfl_Frame. I think this works, but do note that some arches have gaps in the DWARF register numbers and so might have > 100 nregs. Should the regmapping maybe be stored or set with the Dwfl or Dwflst_Process_Tracker once? Cheers, Mark > diff --git a/libdwfl_stacktrace/libdwfl_stacktrace.h > b/libdwfl_stacktrace/libdw\ > fl_stacktrace.h > index b236ddc4..b8987c6f 100644 > --- a/libdwfl_stacktrace/libdwfl_stacktrace.h > +++ b/libdwfl_stacktrace/libdwfl_stacktrace.h > @@ -113,14 +113,28 @@ extern int dwflst_tracker_linux_proc_find_elf > (Dwfl_Modul\ > e *mod, void **userdata > const char *module_name, > Dwarf_A\ > ddr base, > char **file_name, Elf **); > > - > /* Like dwfl_thread_getframes, but iterates through the frames for a > - linux perf_events stack sample rather than a live thread. Calls > - dwfl_attach_state on DWFL, with architecture specified by ELF, ELF > - must remain valid during Dwfl lifetime. Returns zero if all frames > - have been processed by the callback, returns -1 on error, or the > - value of the callback when not DWARF_CB_OK. -1 returned on error > - will set dwfl_errno (). */ > + stack sample rather than a live thread. Calls dwfl_attach_state on > + DWFL, with architecture specified by ELF, ELF must remain vaild > + during Dwfl lifetime. Returns zero if all frames have been > + processed by the callback, returns -1 on error, or the value of the > + callback when not DWARF_CB_OK. -1 returned on error will set > + dwfl_errno (). */ > +int dwflst_sample_getframes (Dwfl *dwfl, Elf *elf, pid_t pid, pid_t tid, > + const void *stack, size_t stack_size, > + const Dwarf_Word *regs, uint32_t n_regs, > + const int *regs_mapping, uint32_t > n_regs_mapping, > + int (*callback) (Dwfl_Frame *state, void > *arg), > + void *arg) > + __nonnull_attribute__ (1, 5, 7, 9, 11); > + > +/* Adapts dwflst_sample_getframes to linux perf_events stack sample > + and register file data format. Calls dwfl_attach_state on DWFL, > + with architecture specified by ELF, ELF must remain valid during > + Dwfl lifetime. Returns zero if all frames have been processed by > + the callback, returns -1 on error, or the value of the callback > + when not DWARF_CB_OK. -1 returned on error will set dwfl_errno > + (). */ > int dwflst_perf_sample_getframes (Dwfl *dwfl, Elf *elf, pid_t pid, pid_t tid, > const void *stack, size_t stack_size, > const Dwarf_Word *regs, uint32_t n_regs,
