[Mesa-dev] [PATCH 2/2] gallium/util: libunwind support

[Emil Velikov]

Hi Rob,

On 24 March 2017 at 21:21, Rob Clark <robdclark at gmail.com> wrote:
> It's kinda sad that (a) we don't have debug_backtrace support on !X86
> and that (b) we re-invent our own crude backtrace support in the first
> place.  If available, use libunwind instead.  The backtrace format is
> based on what xserver and weston use, since it is nice not to have to
> figure out a different format.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  configure.ac                               | 24 ++++++++
>  src/gallium/Automake.inc                   |  1 +
>  src/gallium/auxiliary/util/u_debug_stack.c | 91 ++++++++++++++++++++++++++++++
>  src/gallium/auxiliary/util/u_debug_stack.h | 15 ++++-
>  4 files changed, 129 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index a99684b..5046acb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1025,6 +1025,30 @@ AC_SUBST([LLVM_LIBS])
>  AC_SUBST([LLVM_LDFLAGS])
>  AC_SUBST([LLVM_INCLUDEDIR])
>
> +dnl
> +dnl libunwind
> +dnl
> +AC_ARG_ENABLE([libunwind],
> +    [AS_HELP_STRING([--enable-libunwind],
> +            [Use libunwind for backtracing (default: auto)])],
> +        [LIBUNWIND="$enableval"],
> +        [LIBUNWIND="auto"])
> +
> +PKG_CHECK_MODULES(LIBUNWIND, libunwind, [HAVE_LIBUNWIND=yes], [HAVE_LIBUNWIND=no])
> +if test "x$LIBUNWIND" = "xauto"; then
> +    LIBUNWIND="$HAVE_LIBUNWIND"
> +fi
> +
> +if test "x$LIBUNWIND" = "xyes"; then
> +    if test "x$HAVE_LIBUNWIND" != "xyes"; then
> +        AC_MSG_ERROR([libunwind requested but not installed.])
> +    fi
> +    AC_DEFINE(HAVE_LIBUNWIND, 1, [Have libunwind support])
> +fi
> +
> +AM_CONDITIONAL(HAVE_LIBUNWIND, [test "x$LIBUNWIND" = xyes])
> +
> +
>  dnl Options for APIs
>  AC_ARG_ENABLE([opengl],
>      [AS_HELP_STRING([--disable-opengl],
> diff --git a/src/gallium/Automake.inc b/src/gallium/Automake.inc
> index a01fa54..48b5a44 100644
> --- a/src/gallium/Automake.inc
> +++ b/src/gallium/Automake.inc
> @@ -46,6 +46,7 @@ GALLIUM_TARGET_CFLAGS = \
>
>  GALLIUM_COMMON_LIB_DEPS = \
>         -lm \
> +       $(LIBUNWIND_LIBS) \

We're using the LIBUNWIND_LIBS but LIBUNWIND_CFLAGS is missing. Please
it to src/gallium/auxiliary/Makefile.am's AM_CFLAGS


> diff --git a/src/gallium/auxiliary/util/u_debug_stack.h b/src/gallium/auxiliary/util/u_debug_stack.h
> index 04eba08..0effcbe 100644
> --- a/src/gallium/auxiliary/util/u_debug_stack.h
> +++ b/src/gallium/auxiliary/util/u_debug_stack.h
> @@ -30,6 +30,11 @@
>
>  #include <stdio.h>
>
> +#ifdef HAVE_LIBUNWIND
> +#define UNW_LOCAL_ONLY
> +#include <libunwind.h>
> +#endif
> +
This hunk is not needed in the header. Please move it to the C file.

With the above
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>

-Emil