RE: [RFC][gdb] Handle static libipt

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Hello Tom,

> > Until recently, libipt wouldn't even allow building a static library.
> >
> 
> True, but it's trivial to backport the patch enabling that to older
> releases.  Also, libipt has been build as static lib for Fedora/openSUSE
> for quite a while, at least since v1.5.

Hmmm, the upstream version added this with a 2.0 update based on your
patch.  I'd consider cherry-picking those patches from master a bug-fix update.
We added this in 2.0.1 and fixed it in 2.0.3.  Would this work for you?

You could also backport those patches to 1.6 or even 1.5 if you wanted to.


> > Would it suffice if we fixed this with a libipt release?
> >
> 
> What I'm trying to do in this patch is to make it easy to build with all
> versions of libipt.  For master, nothing is necessary. For v2.x, we
> could fix things with a release, but that already wouldn't work for
> older releases v2.0, v2.01 and v2.0.2, not to mention pre-v2.0 releases.

True.  Are you expecting people to build a new GDB statically linked against
libipt v2.0,  v2.0.1 or v2.0.2 once v2.0.3 is out?

I see the point in replacing libraries for triaging issues.  This could still be done
with dynamic libraries.

Regards,
Markus.

> 
> Thanks,
> - Tom
> 
> > Markus.
> >
> >> -----Original Message-----
> >> From: Tom de Vries <tdevries@suse.de>
> >> Sent: 27 July 2020 09:49
> >> To: gdb-patches@sourceware.org
> >> Cc: Metzger, Markus T <markus.t.metzger@intel.com>
> >> Subject: [RFC][gdb] Handle static libipt
> >>
> >> Hi,
> >>
> >> Libipt can be build as shared library, and used by gdb when f.i. specifying
> >> configure flags --with-libipt-prefix=<path/to/install> and
> >> --with-libipt-type=shared.
> >>
> >> Since libipt release v2.0.1, there's support to build as static library.
> >>
> >> But when trying to use this in gdb by specifying --with-libipt-type=static, we
> >> run into the problem that the constructor is not called ( filed as
> >> https://github.com/intel/libipt/issues/71 ).
> >>
> >> Work around this problem in gdb by inlining the constructor contents in main.
> >>
> >> Tested on x86_64-linux, with static libipt master, v2.0.2 and v1.4.4 (with
> >> static support backported).
> >>
> >> Any comments?
> >>
> >> Thanks,
> >> - Tom
> >>
> >> [gdb] Handle static libipt
> >>
> >> gdb/ChangeLog:
> >>
> >> 2020-07-26  Tom de Vries  <tdevries@suse.de>
> >>
> >> 	* config.in: Regenerate.
> >> 	* configure: Regenerate.
> >> 	* gdb.c (main)[HAVE_LIBIPT_STATIC]: Call libipt constructor.
> >>
> >> gdbserver/ChangeLog:
> >>
> >> 2020-07-26  Tom de Vries  <tdevries@suse.de>
> >>
> >> 	* config.in: Regenerate.
> >> 	* configure: Regenerate.
> >>
> >> gdbsupport/ChangeLog:
> >>
> >> 2020-07-26  Tom de Vries  <tdevries@suse.de>
> >>
> >> 	* common.m4 (HAVE_LIBIPT_STATIC): New AC_DEFINE.
> >> 	(pt_ild_init, pti_ild_init): New AC_CHECK_FUNCS.
> >> 	* config.in: Regenerate.
> >> 	* configure: Regenerate.
> >>
> >> ---
> >>  gdb/config.in        |  9 +++++++++
> >>  gdb/configure        | 29 +++++++++++++++++++++++++++++
> >>  gdb/gdb.c            | 28 ++++++++++++++++++++++++++++
> >>  gdbserver/config.in  |  9 +++++++++
> >>  gdbserver/configure  | 29 +++++++++++++++++++++++++++++
> >>  gdbsupport/common.m4 |  7 +++++++
> >>  gdbsupport/config.in |  9 +++++++++
> >>  gdbsupport/configure | 29 +++++++++++++++++++++++++++++
> >>  8 files changed, 149 insertions(+)
> >>
> >> diff --git a/gdb/config.in b/gdb/config.in
> >> index 340c421ca0..e036ede34d 100644
> >> --- a/gdb/config.in
> >> +++ b/gdb/config.in
> >> @@ -238,6 +238,9 @@
> >>  /* Define if you have the ipt library. */
> >>  #undef HAVE_LIBIPT
> >>
> >> +/* Define if libipt is static lib. */
> >> +#undef HAVE_LIBIPT_STATIC
> >> +
> >>  /* Define if you have the lzma library. */
> >>  #undef HAVE_LIBLZMA
> >>
> >> @@ -346,6 +349,9 @@
> >>  /* Define to 1 if you have the `pthread_sigmask' function. */
> >>  #undef HAVE_PTHREAD_SIGMASK
> >>
> >> +/* Define to 1 if you have the `pti_ild_init' function. */
> >> +#undef HAVE_PTI_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `ptrace64' function. */
> >>  #undef HAVE_PTRACE64
> >>
> >> @@ -364,6 +370,9 @@
> >>  /* Define if sys/ptrace.h defines the PT_GETXMMREGS request. */
> >>  #undef HAVE_PT_GETXMMREGS
> >>
> >> +/* Define to 1 if you have the `pt_ild_init' function. */
> >> +#undef HAVE_PT_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `pt_insn_event' function. */
> >>  #undef HAVE_PT_INSN_EVENT
> >>
> >> diff --git a/gdb/configure b/gdb/configure
> >> index 06b11e2252..16c91ceeca 100755
> >> --- a/gdb/configure
> >> +++ b/gdb/configure
> >> @@ -14802,6 +14802,35 @@ $as_echo "$as_me: WARNING: libipt is missing
> or
> >> unusable; some features may be u
> >>      else
> >>        save_LIBS=$LIBS
> >>        LIBS="$LIBS $LIBIPT"
> >> +      case $LIBIPT in
> >> +	   *.a)
> >> +
> >> +$as_echo "#define HAVE_LIBIPT_STATIC 1" >>confdefs.h
> >> +
> >> +		 for ac_func in pt_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pt_ild_init" "ac_cv_func_pt_ild_init"
> >> +if test "x$ac_cv_func_pt_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PT_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +		 for ac_func in pti_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pti_ild_init" "ac_cv_func_pti_ild_init"
> >> +if test "x$ac_cv_func_pti_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PTI_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +	   ;;
> >> +      esac
> >>        for ac_func in pt_insn_event
> >>  do :
> >>    ac_fn_c_check_func "$LINENO" "pt_insn_event"
> "ac_cv_func_pt_insn_event"
> >> diff --git a/gdb/gdb.c b/gdb/gdb.c
> >> index 1a52a088fc..be51444bfd 100644
> >> --- a/gdb/gdb.c
> >> +++ b/gdb/gdb.c
> >> @@ -20,11 +20,39 @@
> >>  #include "main.h"
> >>  #include "interps.h"
> >>
> >> +#ifdef HAVE_LIBIPT_STATIC
> >> +/* When using libipt as static library, the constructor init won't be called
> >> +   ( https://github.com/intel/libipt/issues/71 ).  Since it's a static
> >> +   function, we can't call it explictly from gdb either.  We could use
> >> +   -Wl,--whole-archive libipt.a -Wl,--no-whole-archive, but that could pull
> >> +   in more code than necessary.  Instead, we (hack alert) inline the contents
> >> +   of init here.  */
> >> +#ifdef HAVE_PT_ILD_INIT
> >> +/* Libipt's init constructor calls lib internal function pt_ild_init starting
> >> +   v1.5.  */
> >> +extern "C" void pt_ild_init (void);
> >> +#endif
> >> +#ifdef HAVE_PTI_ILD_INIT
> >> +/* Libipt's init constructor calls lib internal function pti_ild_init
> >> +   v1.4-v1.4.4.  */
> >> +extern "C" void pti_ild_init (void);
> >> +#endif
> >> +#endif
> >> +
> >>  int
> >>  main (int argc, char **argv)
> >>  {
> >>    struct captured_main_args args;
> >>
> >> +#ifdef HAVE_LIBIPT_STATIC
> >> +#ifdef HAVE_PT_ILD_INIT
> >> +  pt_ild_init ();
> >> +#endif
> >> +#ifdef HAVE_PTI_ILD_INIT
> >> +  pti_ild_init ();
> >> +#endif
> >> +#endif
> >> +
> >>    memset (&args, 0, sizeof args);
> >>    args.argc = argc;
> >>    args.argv = argv;
> >> diff --git a/gdbserver/config.in b/gdbserver/config.in
> >> index 07213aa527..f48ba91c02 100644
> >> --- a/gdbserver/config.in
> >> +++ b/gdbserver/config.in
> >> @@ -143,6 +143,9 @@
> >>  /* Define if you have the ipt library. */
> >>  #undef HAVE_LIBIPT
> >>
> >> +/* Define if libipt is static lib. */
> >> +#undef HAVE_LIBIPT_STATIC
> >> +
> >>  /* Define if the target supports branch tracing. */
> >>  #undef HAVE_LINUX_BTRACE
> >>
> >> @@ -227,6 +230,9 @@
> >>  /* Define to 1 if you have the `pthread_sigmask' function. */
> >>  #undef HAVE_PTHREAD_SIGMASK
> >>
> >> +/* Define to 1 if you have the `pti_ild_init' function. */
> >> +#undef HAVE_PTI_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `ptrace64' function. */
> >>  #undef HAVE_PTRACE64
> >>
> >> @@ -240,6 +246,9 @@
> >>  /* Define to 1 if you have the <ptrace.h> header file. */
> >>  #undef HAVE_PTRACE_H
> >>
> >> +/* Define to 1 if you have the `pt_ild_init' function. */
> >> +#undef HAVE_PT_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `pt_insn_event' function. */
> >>  #undef HAVE_PT_INSN_EVENT
> >>
> >> diff --git a/gdbserver/configure b/gdbserver/configure
> >> index 0f77ac6cb8..93f7a42991 100755
> >> --- a/gdbserver/configure
> >> +++ b/gdbserver/configure
> >> @@ -8566,6 +8566,35 @@ $as_echo "$as_me: WARNING: libipt is missing or
> >> unusable; some features may be u
> >>      else
> >>        save_LIBS=$LIBS
> >>        LIBS="$LIBS $LIBIPT"
> >> +      case $LIBIPT in
> >> +	   *.a)
> >> +
> >> +$as_echo "#define HAVE_LIBIPT_STATIC 1" >>confdefs.h
> >> +
> >> +		 for ac_func in pt_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pt_ild_init" "ac_cv_func_pt_ild_init"
> >> +if test "x$ac_cv_func_pt_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PT_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +		 for ac_func in pti_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pti_ild_init" "ac_cv_func_pti_ild_init"
> >> +if test "x$ac_cv_func_pti_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PTI_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +	   ;;
> >> +      esac
> >>        for ac_func in pt_insn_event
> >>  do :
> >>    ac_fn_c_check_func "$LINENO" "pt_insn_event"
> "ac_cv_func_pt_insn_event"
> >> diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
> >> index b461f5f017..43b2c62270 100644
> >> --- a/gdbsupport/common.m4
> >> +++ b/gdbsupport/common.m4
> >> @@ -166,6 +166,13 @@ AC_DEFUN([GDB_AC_COMMON], [
> >>      else
> >>        save_LIBS=$LIBS
> >>        LIBS="$LIBS $LIBIPT"
> >> +      case $LIBIPT in
> >> +	   *.a)
> >> +	         AC_DEFINE(HAVE_LIBIPT_STATIC, 1, [Define if libipt is static lib. ])
> >> +		 AC_CHECK_FUNCS(pt_ild_init)
> >> +		 AC_CHECK_FUNCS(pti_ild_init)
> >> +	   ;;
> >> +      esac
> >>        AC_CHECK_FUNCS(pt_insn_event)
> >>        AC_CHECK_MEMBERS([struct pt_insn.enabled, struct pt_insn.resynced],
> [],
> >> [],
> >>  		       [#include <intel-pt.h>])
> >> diff --git a/gdbsupport/config.in b/gdbsupport/config.in
> >> index 5556501395..0523192406 100644
> >> --- a/gdbsupport/config.in
> >> +++ b/gdbsupport/config.in
> >> @@ -121,6 +121,9 @@
> >>  /* Define if you have the ipt library. */
> >>  #undef HAVE_LIBIPT
> >>
> >> +/* Define if libipt is static lib. */
> >> +#undef HAVE_LIBIPT_STATIC
> >> +
> >>  /* Define to 1 if you have the <linux/elf.h> header file. */
> >>  #undef HAVE_LINUX_ELF_H
> >>
> >> @@ -181,12 +184,18 @@
> >>  /* Define to 1 if you have the `pthread_sigmask' function. */
> >>  #undef HAVE_PTHREAD_SIGMASK
> >>
> >> +/* Define to 1 if you have the `pti_ild_init' function. */
> >> +#undef HAVE_PTI_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `ptrace64' function. */
> >>  #undef HAVE_PTRACE64
> >>
> >>  /* Define to 1 if you have the <ptrace.h> header file. */
> >>  #undef HAVE_PTRACE_H
> >>
> >> +/* Define to 1 if you have the `pt_ild_init' function. */
> >> +#undef HAVE_PT_ILD_INIT
> >> +
> >>  /* Define to 1 if you have the `pt_insn_event' function. */
> >>  #undef HAVE_PT_INSN_EVENT
> >>
> >> diff --git a/gdbsupport/configure b/gdbsupport/configure
> >> index 51caeeb180..8aaa7e4dbf 100755
> >> --- a/gdbsupport/configure
> >> +++ b/gdbsupport/configure
> >> @@ -10264,6 +10264,35 @@ $as_echo "$as_me: WARNING: libipt is missing
> or
> >> unusable; some features may be u
> >>      else
> >>        save_LIBS=$LIBS
> >>        LIBS="$LIBS $LIBIPT"
> >> +      case $LIBIPT in
> >> +	   *.a)
> >> +
> >> +$as_echo "#define HAVE_LIBIPT_STATIC 1" >>confdefs.h
> >> +
> >> +		 for ac_func in pt_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pt_ild_init" "ac_cv_func_pt_ild_init"
> >> +if test "x$ac_cv_func_pt_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PT_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +		 for ac_func in pti_ild_init
> >> +do :
> >> +  ac_fn_c_check_func "$LINENO" "pti_ild_init" "ac_cv_func_pti_ild_init"
> >> +if test "x$ac_cv_func_pti_ild_init" = xyes; then :
> >> +  cat >>confdefs.h <<_ACEOF
> >> +#define HAVE_PTI_ILD_INIT 1
> >> +_ACEOF
> >> +
> >> +fi
> >> +done
> >> +
> >> +	   ;;
> >> +      esac
> >>        for ac_func in pt_insn_event
> >>  do :
> >>    ac_fn_c_check_func "$LINENO" "pt_insn_event"
> "ac_cv_func_pt_insn_event"
> > Intel Deutschland GmbH
> > Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
> > Tel: +49 89 99 8853-0, www.intel.de
> > Managing Directors: Christin Eisenschmid, Gary Kershaw
> > Chairperson of the Supervisory Board: Nicole Lau
> > Registered Office: Munich
> > Commercial Register: Amtsgericht Muenchen HRB 186928
> >
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928