Re: [RFC][gdb] Handle static libipt

[Tom de Vries <tdevries@suse.de>]

On 7/27/20 3:44 PM, Metzger, Markus T wrote:
> 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?
> 

Well, I think it's a good idea if this would be fixed in a v2.0.x libipt
release.

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

Ah, I didn't realize that would be that simple.

>>> 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?
> 

No.  [ That is, not unless they're triaging issues. ]

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

True, it's just an inconvenience having to switch from static to shared
for triage, but I suppose I can live with that.

Thanks,
- Tom

> 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
>