On Wed, Oct 16, 2024, at 11:37 PM, Aaron Merey wrote:
> Please add an eu-stacktrace entry to NEWS.
Ok, will do.
> I tried to run this but I kept getting:
> Failed to complete recording: GDBus.Error[...]: Action
> org.gnome.sysprof3.profile is not registered
Right. To have sysprof work requires the sysprof polkit action to be installed.
Discussed it on IRC, not sure I've figured out yet where your setup is different
from mine and Frank's. I think I'll post PATCH v2 but it might be the case that
we'll need to add more clarifications to the build instructions.
> I ran eu-stacktrace under `valgrind --leak-check=full`. Memory
> allocated at stacktrace.c:1542 and 1543 is leaked.
Ok, added the free() towards the end of main().
> I got a "conflicting types" error when trying to compile this due to
> SysprofCaptureStackUser and SysprofCaptureUserRegs being defined both
> here and in sysprof-capture-types.h.
>
> Should these structs instead be defined when HAVE_SYSPROF_HEADERS is 0?
> I was able to build eu-stacktrace by making that change.
Ok, this one I think I've resolved.
The idea here was to be able to build eu-stacktrace if
<sysprof-*/sysprof-capture-types.h> points to a system sysprof package in
/usr/include rather than the
patched sysprof.
The #ifndef SYSPROF_CAPTURE_FRAME_STACK_USER was supposed to guard
against duplicate declarations (this will be defined in the patched
sysprof-capture-types header). Frank pointed out this won't work
since that symbol is from an enum. Amending to use a macro in the check:
#if SYSPROF_CAPTURE_FRAME_LAST < 19
This should become unnecessary once there's a released version of
sysprof with the declarations. Then we'll just require sysprof version N.M.etc
> Also the patches that modify other parts of elfutils aren't risky IMO.
>
> I'm comfortable with this being merged for the release as long as any
> build errors are sorted out and we emphasize in the usage text and
> NEWS that eu-stacktrace is an experimental feature with a possibly
> unstable ABI.
I think the key point is that for now eu-stacktrace has to be explicitly
enabled in the configury. If the user doesn't ask for it, it won't be built.
We'll revisit the status of the feature when there's a released Sysprof
version with the stack sampling included.
Would the following wording for NEWS work?
stacktrace: Experimental new tool that can process a stream of stack
samples from the Sysprof profiler and unwind them into call
chains. Enable on x86 with --enable-stacktrace. Requires a
matching set of patches for Sysprof (available at
https://git.sr.ht/~serhei/sysprof-experiments/log/serhei/samples-via-fifo).
--
All the best,
Serhei
http://serhei.io
