[RFC] updated api for libdwfl_stacktrace

  Hello,

I've been concerned to get libdwfl_stacktrace out of the weird 'experimental' limbo we put it in for the last elfutils release. To do that, we need to stabilize the API.

My full prototype of a more mature API is delayed for reasons beyond my control, but sometime in September is the likely ETA. For now I want to get people's opinions (mjw? amerey?) on the most crucial change. To avoid depending on the Linux kernel specific perf_events API, we need a dwflst_sample_getframes() function that doesn't take perf_events formatted data.

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.

All the best,
    Serhei

                                  const void *stack, size_t stack_size,
                                  const Dwarf_Word *regs, uint32_t n_regs,
  

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,
  

Initial response from what I know, pending being actually sure about the answers.

On Wed, Sep 3, 2025, at 9:51 AM, Mark Wielaard wrote:
> 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.
I'm not sure if these args should be reduced. I do know that pid is wanted by dwfl_attach_state whereas tid is wanted by dwfl_getthread_frames. Pretty sure pid is not derivable from tid, so we end up wanting both.

For now the questions reduce to:
* Can we drop pid and derive it from dwfl?
  Pretty sure no -- the Dwfl would not have that info pre-dwfl_attach_state, surely?
* Does this make sense for Dwfl with multiple Dwfl_Module/Elf?
  Has worked perfectly well in my testing, but I'm willing to pretzel my brain to come up with the explanation why.

What I really need to think about, I guess, is whether the design with dwflst_sample_getframes calling dwfl_attach_state() dynamically can be improved on, since it's quite different from the ordinary Dwfl workflow.

> Side note, there is dwfl_getthreads, but I assume that isn't how you
> iterate through threads with an Dwflst_Process_Tracker derived Dwfl?
Correct, we're dealing with one sample at a time, and the sample corresponds to one thread at a time. So there isn't really a need to use dwfl_getthreads() iteration at any point.

> 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.
I think it is not a problem: regs_mapping is meant to map from the small contiguous register sample to the (large, noncontiguous) DWARF register numbers,
so it is a compact array of potentially large DWARF register numbers, rather than a large sparse array of small numbers.

I do need to make ebl_set_initial_registers_sample work efficiently for the architectures that have a large/noncontiguous DWARF layout, but the ebl/backends setup makes this trivial to handle.

> Should the regmapping maybe be stored or set with the Dwfl or
> Dwflst_Process_Tracker once?
That's an option.
The current design allows handling a stream of data with a mix of different register files, which is a fairly theoretical need as of now.
  

Hi Serhei,

On Wed, Aug 20, 2025 at 6:12 PM Serhei Makarov <serhei@serhei.io> wrote:
>
> Hello,
>
> I've been concerned to get libdwfl_stacktrace out of the weird 'experimental' limbo we put it in for the last elfutils release. To do that, we need to stabilize the API.
>
> My full prototype of a more mature API is delayed for reasons beyond my control, but sometime in September is the likely ETA. For now I want to get people's opinions (mjw? amerey?) on the most crucial change. To avoid depending on the Linux kernel specific perf_events API, we need a dwflst_sample_getframes() function that doesn't take perf_events formatted data.
>
> 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 would still be concerned about needing
dwflst_perf_sample_getframes_v2, _v3, etc. if the perf sample format
or dwflst needs change in ways that would require API/ABI changes.
The same concern also applies to the generic dwflst_sample_getframes
if we decide to support a sample type that needs a different set of
parameters.

Instead of specifying the sample parameters as arguments to
dwflst_sample_getframes, what do you think about adding an API for
setting key/value pairs for the Dwflst_Process_Tracker (or related
object)?  Then we can always add more key/value pairs without API/ABI
consequences.  Something like the following:

dwflst_set_tracker_attr (Dwflst_Process_tracker *tracker,
    dwflst_attr_type type, void *value);

where dwflst_attr_type is an enum of every parameter dwflst might need
and we can easily add more types over time. This would eliminate the
need for dwflst_perf_sample_getframes and dwflst_sample_getframes can
adapt to any future changes we might want to make.

Aaron
  

Hi Serhei,

On Wed, 2025-09-03 at 10:14 -0400, Serhei Makarov wrote:
> Initial response from what I know, pending being actually sure about the answers.
> 
> On Wed, Sep 3, 2025, at 9:51 AM, Mark Wielaard wrote:
> > 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.
> I'm not sure if these args should be reduced. I do know that pid is wanted by dwfl_attach_state whereas tid is wanted by dwfl_getthread_frames. Pretty sure pid is not derivable from tid, so we end up wanting both.

Maybe Aaron's suggestion to use a key/value pairs like API makes sense
here?

> For now the questions reduce to:
> * Can we drop pid and derive it from dwfl?
>   Pretty sure no -- the Dwfl would not have that info pre-dwfl_attach_state, surely?

But if the Dwfl isn't attached, are we even interested in the samples?
This might be my confusion on the ordering of events. Who/what triggers
a dwflst_perf_sample_getframes call? What prevents us from requiring it
is only called DWfl's with attached state? If it doesn't have a
state/pid, then we just ignore it?

> * Does this make sense for Dwfl with multiple Dwfl_Module/Elf?
>   Has worked perfectly well in my testing, but I'm willing to pretzel my brain to come up with the explanation why.

My point is more that it looks like you are interested in the Elf just
for the architecture. But all Dwfl_Modules in a Dwfl map to Elf with
the same architecture. So normally just having the Dwfl is enough.

> > 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.
> I think it is not a problem: regs_mapping is meant to map from the small contiguous register sample to the (large, noncontiguous) DWARF register numbers,
> so it is a compact array of potentially large DWARF register numbers, rather than a large sparse array of small numbers.

Ah, OK, I had it reverse in my head. But that makes sense, map from the
register numbers to the DWARF register numbers.

> > Should the regmapping maybe be stored or set with the Dwfl or
> > Dwflst_Process_Tracker once?
> That's an option.
> The current design allows handling a stream of data with a mix of different register files, which is a fairly theoretical need as of now.

I guess it is possible for 32-on-64bit arches. And I guess you could
use it for emulation/jits/interpreters, but this is probably the wrong
level to do that kind translation.

Cheers,

Mark
  

Hi -

> [...]
> Maybe Aaron's suggestion to use a key/value pairs like API makes sense
> here?

Or a struct containing properly named and **typed** parameters.

(Coercing everything into general key/value is kind of papering over
the compatibility / width / usability problem.  Both caller and callee
side need as much or more code that way.)

- FChE