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
