diff mbox series

libdwfl_stacktrace: fix non-Linux build dep on PERF_SAMPLE_REGS_ABI

Message ID 20250908162313.343213-1-serhei@serhei.io
State Committed
Delegated to: Mark Wielaard
Headers
Series libdwfl_stacktrace: fix non-Linux build dep on PERF_SAMPLE_REGS_ABI |

Commit Message

Serhei Makarov Sept. 8, 2025, 4:23 p.m. UTC 
  Reported on a GNU Hurd build of elfutils. This is a quick fix pending
my more complex patch to reduce dependency on linux perf concepts for
the libdwfl_stacktrace code.

* libdwfl_stacktrace/dwflst_perf_frame.c (perf_sample_regs_abi):
  Define this Linux enum on non-Linux platforms.
---
 libdwfl_stacktrace/dwflst_perf_frame.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Mark Wielaard Sept. 8, 2025, 5:38 p.m. UTC | #1 
Hi Serhei,

On Mon, 2025-09-08 at 12:23 -0400, Serhei Makarov wrote:
> Reported on a GNU Hurd build of elfutils. This is a quick fix pending
> my more complex patch to reduce dependency on linux perf concepts for
> the libdwfl_stacktrace code.
> 
> * libdwfl_stacktrace/dwflst_perf_frame.c (perf_sample_regs_abi):
>   Define this Linux enum on non-Linux platforms.
> ---
>  libdwfl_stacktrace/dwflst_perf_frame.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libdwfl_stacktrace/dwflst_perf_frame.c b/libdwfl_stacktrace/dwflst_perf_frame.c
> index dc274e8e..4fc60183 100644
> --- a/libdwfl_stacktrace/dwflst_perf_frame.c
> +++ b/libdwfl_stacktrace/dwflst_perf_frame.c
> @@ -32,6 +32,13 @@
>  
>  #if defined(__linux__)
>  # include <linux/perf_event.h>
> +#else
> +/* XXX required by copy_word() below */
> +enum perf_sample_regs_abi {
> +    PERF_SAMPLE_REGS_ABI_NONE       = 0,
> +    PERF_SAMPLE_REGS_ABI_32         = 1,
> +    PERF_SAMPLE_REGS_ABI_64         = 2,
> +};
>  #endif
>  
>  #include "libdwfl_stacktraceP.h"

Yeah, that is the simplest fix for now. Lets just go with it.

Could you also take a peek at the Debian workaround for dealing with
arches that don't support perf?
https://salsa.debian.org/toolchain-team/elfutils/-/commit/2cd08fdcfcaf15d6a2b7b085faedadfc94a4729c
I am not sure just listing all the arches that don't support perf is
really the way to go. But maybe it is the best there is?

Cheers,

Mark
Serhei Makarov Oct. 20, 2025, 6:04 p.m. UTC | #2 
On Mon, Sep 8, 2025, at 1:38 PM, Mark Wielaard wrote:
> Yeah, that is the simplest fix for now. Lets just go with it.
Somewhat delayed, but I've cherry-picked the fix onto main branch.

> Could you also take a peek at the Debian workaround for dealing with
> arches that don't support perf?
> https://salsa.debian.org/toolchain-team/elfutils/-/commit/2cd08fdcfcaf15d6a2b7b085faedadfc94a4729c
> I am not sure just listing all the arches that don't support perf is
> really the way to go. But maybe it is the best there is?
The other option is to keep the definition restricted to _linux_ and x86 / x86_64 (the arches where perf register handling is implemented), then extend the list of arches as we implement support for other arches.
Mark Wielaard Oct. 21, 2025, 11:21 a.m. UTC | #3 
Hi Serhei,

On Mon, 2025-10-20 at 14:04 -0400, Serhei Makarov wrote:
> On Mon, Sep 8, 2025, at 1:38 PM, Mark Wielaard wrote:
> 
> > Could you also take a peek at the Debian workaround for dealing with
> > arches that don't support perf?
> > https://salsa.debian.org/toolchain-team/elfutils/-/commit/2cd08fdcfcaf15d6a2b7b085faedadfc94a4729c
> > I am not sure just listing all the arches that don't support perf is
> > really the way to go. But maybe it is the best there is?
> >
> The other option is to keep the definition restricted to _linux_ and x86 / x86_64 (the arches where perf register handling is implemented), then extend the list of arches as we implement support for other arches.

Yes, that seems a better way forward.
Or maybe it could even be a configure set HAVE_PERF_REGS?

Thanks,

Mark
Serhei Makarov Oct. 21, 2025, 4:28 p.m. UTC | #4 
On Tue, Oct 21, 2025, at 7:21 AM, Mark Wielaard wrote:
> Yes, that seems a better way forward.
> Or maybe it could even be a configure set HAVE_PERF_REGS?
Oh, wait, we've fixed it in commit d05241ce69:
https://sourceware.org/cgit/elfutils/commit/?id=d05241ce690018548857199bd34d7734c7164679

The debian fix was done over top of 0.193, which doesn't include this patch.

So there's nothing to be done here at the moment. When adding non-x86 support (?November) I expect to tidy the existing configury and add HAVE_PERF_REGS option as you suggest.

All the best,
      Serhei
Mark Wielaard Oct. 30, 2025, 11 a.m. UTC | #5 
On Tue, Oct 21, 2025 at 12:28:53PM -0400, Serhei Makarov wrote:
> 
> 
> On Tue, Oct 21, 2025, at 7:21 AM, Mark Wielaard wrote:
> > Yes, that seems a better way forward.
> > Or maybe it could even be a configure set HAVE_PERF_REGS?
> Oh, wait, we've fixed it in commit d05241ce69:
> https://sourceware.org/cgit/elfutils/commit/?id=d05241ce690018548857199bd34d7734c7164679
> 
> The debian fix was done over top of 0.193, which doesn't include this patch.
> 
> So there's nothing to be done here at the moment. When adding non-x86 support (?November) I expect to tidy the existing configury and add HAVE_PERF_REGS option as you suggest.

Adding Matthias Klose to the CC. He apparently still needs a patch to
disable some arches:
https://salsa.debian.org/toolchain-team/elfutils/-/blob/master/debian/patches/perf_regs.diff?ref_type=heads

I am not sure what is going on there. Is the current patch not working
correctly?  The intention is to only try to include/support
asm/perf_regs.h on linux for i686 and x86_64 arches.

Cheers,

Mark
Serhei Makarov Oct. 31, 2025, 4:28 p.m. UTC | #6 
On Thu, Oct 30, 2025, at 7:00 AM, Mark Wielaard wrote:
> I am not sure what is going on there. Is the current patch not working
> correctly?  The intention is to only try to include/support
> asm/perf_regs.h on linux for i686 and x86_64 arches.
Perhaps we have been assuming incorrectly, and some of these debian targets define *both* __sparc__ and __x86_64__? Then they don't provide the perf_regs.h?

It may be necessary to go with our alternate plan of adding configury item, to remove all ambiguity. I'll author a patch.
diff mbox series

Patch

diff --git a/libdwfl_stacktrace/dwflst_perf_frame.c b/libdwfl_stacktrace/dwflst_perf_frame.c
index dc274e8e..4fc60183 100644
--- a/libdwfl_stacktrace/dwflst_perf_frame.c
+++ b/libdwfl_stacktrace/dwflst_perf_frame.c
@@ -32,6 +32,13 @@ 
 
 #if defined(__linux__)
 # include <linux/perf_event.h>
+#else
+/* XXX required by copy_word() below */
+enum perf_sample_regs_abi {
+    PERF_SAMPLE_REGS_ABI_NONE       = 0,
+    PERF_SAMPLE_REGS_ABI_32         = 1,
+    PERF_SAMPLE_REGS_ABI_64         = 2,
+};
 #endif
 
 #include "libdwfl_stacktraceP.h"