[2/4] eu-stacktrace: configury for initial version (x86/sysprof only)

Message ID 8db81b6e-f3ed-49c2-9302-421247bec92c@app.fastmail.com
State Superseded
Headers
Series [1/4] eu-stacktrace: add eu-stacktrace tool |

Commit Message

Serhei Makarov Oct. 15, 2024, 3:25 p.m. UTC
  Due to the x86-specific code in the initial version the configury has
significant restrictions. If --enable-stacktrace is not explicitly
provided, then eu-stacktrace will be disabled by default when
x86 architecture or sysprof headers are absent.

The way we test for x86 is a bit unusual. What we actually care about
is that the register file provided by perf_events on the system is an
x86 register file; this is done by checking that <asm/perf_regs.h> is
Linux kernel arch/x86/include/uapi/asm/perf_regs.h.

Once eu-stacktrace is properly portable across architectures,
these grody checks can be simplified.

* configure.ac: Add configure checks and conditionals for stacktrace tool.
* src/Makefile.am: Add stacktrace tool conditional on ENABLE_STACKTRACE.

Signed-off-by: Serhei Makarov <serhei@serhei.io>
---
 configure.ac    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/Makefile.am |  9 ++++++-
 2 files changed, 73 insertions(+), 2 deletions(-)
  

Comments

Aaron Merey Oct. 17, 2024, 3:38 a.m. UTC | #1
Hi Serhei,

On Tue, Oct 15, 2024 at 11:28 AM Serhei Makarov <serhei@serhei.io> wrote:
>
> Due to the x86-specific code in the initial version the configury has
> significant restrictions. If --enable-stacktrace is not explicitly
> provided, then eu-stacktrace will be disabled by default when
> x86 architecture or sysprof headers are absent.
> 
> The way we test for x86 is a bit unusual. What we actually care about
> is that the register file provided by perf_events on the system is an
> x86 register file; this is done by checking that <asm/perf_regs.h> is
> Linux kernel arch/x86/include/uapi/asm/perf_regs.h.
> 
> Once eu-stacktrace is properly portable across architectures,
> these grody checks can be simplified.
> 
> * configure.ac: Add configure checks and conditionals for stacktrace tool.
> * src/Makefile.am: Add stacktrace tool conditional on ENABLE_STACKTRACE.
> 
> Signed-off-by: Serhei Makarov <serhei@serhei.io>
> ---
>  configure.ac    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/Makefile.am |  9 ++++++-
>  2 files changed, 73 insertions(+), 2 deletions(-)


Using less wide indentation throughout would help make the configure.ac
changes more readable.  Otherwise this patch LGTM.

Aaron
  
Serhei Makarov Oct. 17, 2024, 3:28 p.m. UTC | #2
On Wed, Oct 16, 2024, at 11:38 PM, Aaron Merey wrote:
> Using less wide indentation throughout would help make the configure.ac
> changes more readable.  Otherwise this patch LGTM.
Ok, my code imitated the offset for the debuginfod configury immediately above,
but I'll update the patch to use 3 spaces instead (seen elsewhere in the file).
Or, do you have a different preference for the indentation?
  
Aaron Merey Oct. 17, 2024, 3:44 p.m. UTC | #3
On Thu, Oct 17, 2024 at 11:28 AM Serhei Makarov <serhei@serhei.io> wrote:
>
> On Wed, Oct 16, 2024, at 11:38 PM, Aaron Merey wrote:
> > Using less wide indentation throughout would help make the configure.ac
> > changes more readable.  Otherwise this patch LGTM.
> Ok, my code imitated the offset for the debuginfod configury immediately above,
> but I'll update the patch to use 3 spaces instead (seen elsewhere in the file).
> Or, do you have a different preference for the indentation?

You're right we are inconsistent with indentation. Because this patch
adds particularly long lines, I'd use 3 spaces here.

Aaron
  

Patch

diff --git a/configure.ac b/configure.ac
index 8f5901a2..1af1c708 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,7 +1,7 @@ 
 dnl Process this file with autoconf to produce a configure script.
 dnl Configure input file for elfutils.                     -*-autoconf-*-
 dnl
-dnl Copyright (C) 1996-2019 Red Hat, Inc.
+dnl Copyright (C) 1996-2019, 2024 Red Hat, Inc.
 dnl Copyright (C) 2022, 2023 Mark J. Wielaard <mark@klomp.org>
 dnl
 dnl This file is part of elfutils.
@@ -918,6 +918,69 @@  AC_ARG_ENABLE(debuginfod-ima-cert-path,
 AC_SUBST(DEBUGINFOD_IMA_CERT_PATH, $default_debuginfod_ima_cert_path)
 AC_CONFIG_FILES([config/profile.sh config/profile.csh config/profile.fish])
 
+# XXX Currently, eu-stacktrace can only work with sysprof/x86, hence:
+AC_ARG_ENABLE([stacktrace],AS_HELP_STRING([--enable-stacktrace], [Enable eu-stacktrace]),
+            stacktrace_option_given=yes,
+            stacktrace_option_given=no)
+# check for x86, or more precisely _ASM_X86_PERF_REGS_H
+AS_IF([test "x$enable_stacktrace" != "xno"], [
+            enable_stacktrace=no
+            AC_LANG([C])
+            AC_CACHE_CHECK([for _ASM_X86_PERF_REGS_H], ac_cv_has_asm_x86_perf_regs_h,
+              [AC_COMPILE_IFELSE([AC_LANG_SOURCE([
+#include <asm/perf_regs.h>
+#ifndef _ASM_X86_PERF_REGS_H
+#error "_ASM_X86_PERF_REGS_H not found"
+#endif
+])], ac_cv_has_asm_x86_perf_regs_h=yes, ac_cv_has_asm_x86_perf_regs_h=no)])
+            AS_IF([test "x$ac_cv_has_asm_x86_perf_regs_h" = xyes], [
+                        enable_stacktrace=yes
+            ])
+            if test "x$enable_stacktrace" = "xno"; then
+                        if test "x$stacktrace_option_given" = "xyes"; then
+                                    AC_MSG_ERROR([${program_prefix}stacktrace currently only supports x86, use --disable-stacktrace to disable.])
+                        else
+                                    AC_MSG_WARN([${program_prefix}stacktrace currently only supports x86, disabling by default on other architectures.])
+                        fi
+            fi
+])
+# check for sysprof headers:
+AS_IF([test "x$enable_stacktrace" != "xno"], [
+            enable_stacktrace=no
+            AC_CACHE_CHECK([for sysprof-6/sysprof-capture-types.h], ac_cv_has_sysprof_6_headers,
+              [AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#include <sysprof-6/sysprof-capture-types.h>]])],
+                         ac_cv_has_sysprof_6_headers=yes, ac_cv_has_sysprof_6_headers=no)])
+            AS_IF([test "x$ac_cv_has_sysprof_6_headers" = xyes], [
+                        AC_DEFINE(HAVE_SYSPROF_6_HEADERS)
+                        enable_stacktrace=yes
+            ])
+            AH_TEMPLATE([HAVE_SYSPROF_6_HEADERS], [Define to 1 if `sysprof-6/sysprof-capture-types.h`
+                                                   is provided by the system, 0 otherwise.])
+            AC_CACHE_CHECK([for sysprof-4/sysprof-capture-types.h], ac_cv_has_sysprof_4_headers,
+              [AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#include <sysprof-4/sysprof-capture-types.h>]])],
+                         ac_cv_has_sysprof_4_headers=yes, ac_cv_has_sysprof_4_headers=no)])
+            AS_IF([test "x$ac_cv_has_sysprof_4_headers" = xyes], [
+                        AC_DEFINE(HAVE_SYSPROF_4_HEADERS)
+                        enable_stacktrace=yes
+            ])
+            AH_TEMPLATE([HAVE_SYSPROF_4_HEADERS], [Define to 1 if `sysprof-4/sysprof-capture-types.h`
+                                                   is provided by the system, 0 otherwise.])
+            if test "x$enable_stacktrace" = "xno"; then
+                        if test "x$stacktrace_option_given" = "xyes"; then
+                                    AC_MSG_ERROR([sysprof headers for ${program_prefix}stacktrace not found, use --disable-stacktrace to disable.])
+                        else
+                                    AC_MSG_WARN([sysprof headers for ${program_prefix}stacktrace not found, disabling by default.])
+                        fi
+            fi
+],[
+# If eu-stacktrace is disabled, also disable the automake conditionals:
+ac_cv_has_sysprof_6_headers=no
+ac_cv_has_sysprof_4_headers=no
+])
+AM_CONDITIONAL([HAVE_SYSPROF_6_HEADERS],[test "x$ac_cv_has_sysprof_6_headers" = xyes])
+AM_CONDITIONAL([HAVE_SYSPROF_4_HEADERS],[test "x$ac_cv_has_sysprof_4_headers" = xyes])
+AM_CONDITIONAL([ENABLE_STACKTRACE],[test "x$enable_stacktrace" = "xyes"])
+
 AC_OUTPUT
 
 AC_MSG_NOTICE([
@@ -957,6 +1020,7 @@  AC_MSG_NOTICE([
     Default DEBUGINFOD_URLS            : ${default_debuginfod_urls}
     Debuginfod RPM sig checking        : ${enable_debuginfod_ima_verification} 
     Default DEBUGINFOD_IMA_CERT_PATH   : ${default_debuginfod_ima_cert_path}
+    ${program_prefix}stacktrace support              : ${enable_stacktrace}
 
   EXTRA TEST FEATURES (used with make check)
     have bunzip2 installed (required)  : ${HAVE_BUNZIP2}
diff --git a/src/Makefile.am b/src/Makefile.am
index e0267d96..6bdf2dfb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,6 +1,6 @@ 
 ## Process this file with automake to create Makefile.in
 ##
-## Copyright (C) 1996-2014, 2016 Red Hat, Inc.
+## Copyright (C) 1996-2014, 2016, 2024 Red Hat, Inc.
 ## This file is part of elfutils.
 ##
 ## This file is free software; you can redistribute it and/or modify
@@ -31,6 +31,10 @@  bin_PROGRAMS = readelf nm size strip elflint findtextrel addr2line \
 	       elfcmp objdump ranlib strings ar unstrip stack elfcompress \
 	       elfclassify srcfiles
 
+if ENABLE_STACKTRACE
+bin_PROGRAMS += stacktrace
+endif
+
 noinst_LIBRARIES = libar.a
 
 libar_a_SOURCES = arlib.c arlib2.c arlib-argp.c
@@ -94,6 +98,9 @@  strings_LDADD = $(libelf) $(libeu) $(argp_LDADD)
 ar_LDADD = libar.a $(libelf) $(libeu) $(argp_LDADD) $(obstack_LIBS)
 unstrip_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
 stack_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD) $(demanglelib)
+if ENABLE_STACKTRACE
+stacktrace_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD)
+endif
 elfcompress_LDADD = $(libebl) $(libelf) $(libdw) $(libeu) $(argp_LDADD)
 elfclassify_LDADD = $(libelf) $(libdw) $(libeu) $(argp_LDADD)
 srcfiles_SOURCES = srcfiles.cxx