[RFC/ia64] link against libunwind rather than using dlopen

[RFC/ia64] link against libunwind rather than using dlopen

[Joel Brobecker]

Hello,

This is something that I mentioned back in Aug 2005, and which came back
to bite me a little. Basically, on ia64 hosts, we dlopen libunwind rather
that link GDB against it.

This has two consequences:

  - I cannot link libunwind statically, which is very important
    in my case;

  - But also, I was trying to figure out why Steve Elcey, at HP,
    was having trouble when he's using GDB on ia64-hpux. I thought
    I had reproduced the problem, and it took 30min of debugging
    to realize that the libunwind had not been initialized, due
    to a problem during the dlopen.   The real problem is that this
    failure was completely silent, so there was no way to know
    without debugging that this had happened.

This patch is a first step towards treating libunwind the same way
we treat the other libraries such as expat, iconv, etc. What it does
is change the configury perform a link check against libunwind.
The --with-libunwind option is also extended to allow a path to
be given, when libunwind is installed at a non-standard location.

There are a few adjustements that are needed after that, mostly:
  - Some macros no longer exists, but we can use HAVE_LIBUNWIND
    for that purpose;
  - We set the various function pointers directly, rather than
    doing a dlsym.

This is not the whole series yet, as I want to make sure that we're
still OK with this approach before I waste more time on this.  Next
on the list is to delete the libunwind_load phase, and use the unw_*
symbols directly (so all the function unw_*_p pointers will disappear).

gdb/ChangeLog:

        * configure.ac: Allow the argument of the --with-libunwind
        option to be the prefix where libunwind is installed.
        Perform a link check against libunwind, rather than
        a couple of header file checks.
        * configure, config.in: Regenerate.
        * ia64-tdep.c: Replace uses of HAVE_LIBUNWIND_IA64_H by
        HAVE_LIBUNWIND.
        * libunwind-frame.h: Replace HAVE_LIBUNWIND_H by HAVE_LIBUNWIND.
        * libunwind-frame.c: Remove dlfcn.h include.
        (libunwind_load): Reimplement without using dlopen.

What do you guys think?

Thanks,
-- 
Joel

---
 gdb/config.in         |    6 ---
 gdb/configure         |  107 ++++++++++++++++++++++++++++++++++---------------
 gdb/configure.ac      |   49 +++++++++++++++++-----
 gdb/ia64-tdep.c       |   10 ++--
 gdb/libunwind-frame.c |   59 +++++----------------------
 gdb/libunwind-frame.h |    4 +-
 6 files changed, 128 insertions(+), 107 deletions(-)

diff --git a/gdb/configure.ac b/gdb/configure.ac
index 8c12a44..d2e3792 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -304,20 +304,45 @@ esac
 
 # Libunwind support.
 AC_ARG_WITH(libunwind,
-AS_HELP_STRING([--with-libunwind], [use libunwind frame unwinding support]),
-[case "${withval}" in
-  yes)  enable_libunwind=yes ;;
-  no)   enable_libunwind=no ;;
-  *)    AC_MSG_ERROR(bad value ${withval} for GDB with-libunwind option) ;;
-esac],[
-  AC_CHECK_HEADERS(libunwind.h libunwind-ia64.h)
-  if test x"$ac_cv_header_libunwind_h" = xyes -a x"$ac_cv_header_libunwind_ia64_h" = xyes; then
-    enable_libunwind=yes;
-  fi
+  AS_HELP_STRING([--with-libunwind@<:@=LIBUNWIND@:>@],
+                 [use libunwind frame unwinding support (auto/yes/no/libunwind-path)]),
+  [], [with_libunwind=auto])
+
+AC_DEFUN([AC_CHECK_LIBUNWIND],
+[
+  save_LIBS=$LIBS
+  LIBS="$LIBS -lunwind-ia64 -lunwind"
+  case "$host_os" in
+    hpux*) # On this platform, libunwind depends on libuca...
+           LIBS="$LIBS -luca"
+           ;;
+  esac
+  AC_LINK_IFELSE(AC_LANG_PROGRAM([#include "libunwind.h"],
+                                 [unw_init_local (0, 0);]),
+                 [enable_libunwind=yes],
+                 [enable_libunwind=no
+                  LIBS=$save_LIBS])
 ])
-   
+
+if test "$with_libunwind" = "no"; then
+  AC_MSG_WARN([libunwind support disabled; some features may be unavailable.])
+  enable_libunwind=no
+else
+  case "$with_libunwind" in
+    auto|yes) AC_CHECK_LIBUNWIND()
+          ;;
+    *)    # The argument is the prefix where libunwind has been installed.
+          CPPFLAGS="$CPPFLAGS -I${with_libunwind}/include"
+          LIBS="$LIBS -L${with_libunwind}/lib"
+          AC_CHECK_LIBUNWIND()
+          ;;
+  esac
+  if test "$with_libunwind" != "auto" -a "$enable_libunwind" != "yes"; then
+    AC_MSG_ERROR(libunwind library is missing.)
+  fi
+fi
+
 if test x"$enable_libunwind" = xyes; then
-  AC_CHECK_HEADERS(libunwind.h libunwind-ia64.h)
   AC_DEFINE(HAVE_LIBUNWIND, 1, [Define if libunwind library is being used.])
   CONFIG_OBS="$CONFIG_OBS libunwind-frame.o"
   CONFIG_DEPS="$CONFIG_DEPS libunwind-frame.o"
diff --git a/gdb/config.in b/gdb/config.in
index c1d7c68..7f8d41c 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -236,12 +236,6 @@
 /* Define if libunwind library is being used. */
 #undef HAVE_LIBUNWIND
 
-/* Define to 1 if you have the <libunwind.h> header file. */
-#undef HAVE_LIBUNWIND_H
-
-/* Define to 1 if you have the <libunwind-ia64.h> header file. */
-#undef HAVE_LIBUNWIND_IA64_H
-
 /* Define to 1 if you have the `w' library (-lw). */
 #undef HAVE_LIBW
 
diff --git a/gdb/configure b/gdb/configure
[snip'ed]
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 68e5021..0090e38 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -41,7 +41,7 @@
 #include "ia64-tdep.h"
 #include "cp-abi.h"
 
-#ifdef HAVE_LIBUNWIND_IA64_H
+#ifdef HAVE_LIBUNWIND
 #include "elf/ia64.h"           /* for PT_IA_64_UNWIND value */
 #include "libunwind-frame.h"
 #include "libunwind-ia64.h"
@@ -942,7 +942,7 @@ ia64_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
 
   if (regnum >= V32_REGNUM && regnum <= V127_REGNUM)
     {
-#ifdef HAVE_LIBUNWIND_IA64_H
+#ifdef HAVE_LIBUNWIND
       /* First try and use the libunwind special reg accessor,
 	 otherwise fallback to standard logic.  */
       if (!libunwind_is_initialized ()
@@ -2392,7 +2392,7 @@ static const struct frame_base ia64_frame_base =
   ia64_frame_base_address
 };
 
-#ifdef HAVE_LIBUNWIND_IA64_H
+#ifdef HAVE_LIBUNWIND
 
 struct ia64_unwind_table_entry
   {
@@ -3189,7 +3189,7 @@ static struct libunwind_descr ia64_libunwind_descr =
   &ia64_unw_rse_accessors,
 };
 
-#endif /* HAVE_LIBUNWIND_IA64_H  */
+#endif /* HAVE_LIBUNWIND  */
 
 static int
 ia64_use_struct_convention (struct type *type)
@@ -4031,7 +4031,7 @@ ia64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_dummy_id (gdbarch, ia64_dummy_id);
 
   set_gdbarch_unwind_pc (gdbarch, ia64_unwind_pc);
-#ifdef HAVE_LIBUNWIND_IA64_H
+#ifdef HAVE_LIBUNWIND
   frame_unwind_append_unwinder (gdbarch,
                                 &ia64_libunwind_sigtramp_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &ia64_libunwind_frame_unwind);
diff --git a/gdb/libunwind-frame.c b/gdb/libunwind-frame.c
index 677d4c5..eacd4ff 100644
--- a/gdb/libunwind-frame.c
+++ b/gdb/libunwind-frame.c
@@ -32,8 +32,6 @@
 #include "objfiles.h"
 #include "regcache.h"
 
-#include <dlfcn.h>
-
 #include "gdb_assert.h"
 #include "gdb_string.h"
 
@@ -505,54 +503,17 @@ libunwind_get_reg_special (struct gdbarch *gdbarch, struct regcache *regcache,
 static int
 libunwind_load (void)
 {
-  void *handle;
-
-  handle = dlopen (LIBUNWIND_SO, RTLD_NOW);
-  if (handle == NULL)
-    return 0;
-
-  /* Initialize pointers to the dynamic library functions we will use.  */
-
-  unw_get_reg_p = dlsym (handle, get_reg_name);
-  if (unw_get_reg_p == NULL)
-    return 0;
-
-  unw_get_fpreg_p = dlsym (handle, get_fpreg_name);
-  if (unw_get_fpreg_p == NULL)
-    return 0;
-
-  unw_get_saveloc_p = dlsym (handle, get_saveloc_name);
-  if (unw_get_saveloc_p == NULL)
-    return 0;
-
-  unw_is_signal_frame_p = dlsym (handle, is_signal_frame_name);
-  if (unw_is_signal_frame_p == NULL)
-    return 0;
+  unw_get_reg_p = UNW_OBJ(get_reg);
+  unw_get_fpreg_p = UNW_OBJ(get_fpreg);
+  unw_get_saveloc_p = UNW_OBJ(get_save_loc);
+  unw_is_signal_frame_p = UNW_OBJ(is_signal_frame);
+  unw_step_p = UNW_OBJ(step);
+  unw_init_remote_p = UNW_OBJ(init_remote);
+  unw_create_addr_space_p = UNW_OBJ(create_addr_space);
+  unw_destroy_addr_space_p = UNW_OBJ (destroy_addr_space);
+  unw_search_unwind_table_p = UNW_OBJ(search_unwind_table);
+  unw_find_dyn_list_p = UNW_OBJ(find_dyn_list);
 
-  unw_step_p = dlsym (handle, step_name);
-  if (unw_step_p == NULL)
-    return 0;
-
-  unw_init_remote_p = dlsym (handle, init_remote_name);
-  if (unw_init_remote_p == NULL)
-    return 0;
-
-  unw_create_addr_space_p = dlsym (handle, create_addr_space_name);
-  if (unw_create_addr_space_p == NULL)
-    return 0;
-
-  unw_destroy_addr_space_p = dlsym (handle, destroy_addr_space_name);
-  if (unw_destroy_addr_space_p == NULL)
-    return 0;
-
-  unw_search_unwind_table_p = dlsym (handle, search_unwind_table_name);
-  if (unw_search_unwind_table_p == NULL)
-    return 0;
-
-  unw_find_dyn_list_p = dlsym (handle, find_dyn_list_name);
-  if (unw_find_dyn_list_p == NULL)
-    return 0;
-   
   return 1;
 }
 
diff --git a/gdb/libunwind-frame.h b/gdb/libunwind-frame.h
index 9b88f71..b4c3a4c 100644
--- a/gdb/libunwind-frame.h
+++ b/gdb/libunwind-frame.h
@@ -20,7 +20,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef HAVE_LIBUNWIND_H
+#ifdef HAVE_LIBUNWIND
 
 struct frame_info;
 struct frame_id;
@@ -74,4 +74,4 @@ int libunwind_get_reg_special (struct gdbarch *gdbarch,
 
 #endif /* libunwind-frame.h */
 
-#endif /* HAVE_LIBUNWIND_H  */
+#endif /* HAVE_LIBUNWIND  */
-- 
1.7.0.4

Re: [RFC/ia64] link against libunwind rather than using dlopen

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This is something that I mentioned back in Aug 2005, and which came back
Joel> to bite me a little. Basically, on ia64 hosts, we dlopen libunwind rather
Joel> that link GDB against it.
[...]
Joel> What do you guys think?

Why do we currently use dlopen?

If there is no good reason, or the reason is obsolete, then it is
definitely better to link against a library than to dlopen it.

Tom

Re: [RFC/ia64] link against libunwind rather than using dlopen

[Joel Brobecker]

> Joel> This is something that I mentioned back in Aug 2005, and which came back
> Joel> to bite me a little. Basically, on ia64 hosts, we dlopen libunwind rather
> Joel> that link GDB against it.
> [...]
> Joel> What do you guys think?
> 
> Why do we currently use dlopen?

Hard to answer that question for certain. I had a hunch shortly
after I sent this message, though: On the (relatively old) Linux
machine that we have at AdaCore, I can't link against the system
libunwind. I think it's a linker bug, as the linker is complaining
about missing symbols, whereas I can see them fine.  These are
protected symbols, but they are there. No problem with GNU
libunwind.  But I did send a message to Jan asking him if he could
test on a newer distro, and he said that he'd get back to me
as soon as he has time.

-- 
Joel

Re: [RFC/ia64] link against libunwind rather than using dlopen

[Jan Kratochvil]

On Fri, 17 Jun 2011 18:10:42 +0200, Joel Brobecker wrote:
> Next on the list is to delete the libunwind_load phase, and use the unw_*
> symbols directly (so all the function unw_*_p pointers will disappear).

The gdb binary years ago could be freely copied across hosts and it still
worked.  GDB could also still provide ia64 debugging even if it runtime failed
to find libunwind.so.7.

This seems to be the original goal of the code:
	RFA: libunwind basic support
	http://sourceware.org/ml/gdb-patches/2003-10/msg00504.html
	http://sourceware.org/ml/gdb-patches/2003-11/msg00228.html
	> > - is it considered a "system library" (like libc, or libthread_db)?
	> 
	> I would have to say no.  It is currently an optional library.
	> This is one of the reasons I chose to use a dynamic load mechanism.


Unfortunately since DT_NEEDED libpython this feature is already lost so it
should be OK now even for libunwind.so.7.  Also with ubiquitous virtualization
the matching OS and GDB binary are usually more available now so the binary
build compatibility is not such an issue.


On Wed, 22 Jun 2011 17:36:25 +0200, Joel Brobecker wrote:
> But I did send a message to Jan asking him if he could test on a newer
> distro,

As sent offlist I have not found any problems you described on ia64 RHEL-4 and
RHEL-5 boxes.


Thanks,
Jan

Re: [RFC/ia64] link against libunwind rather than using dlopen

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> The gdb binary years ago could be freely copied across hosts and it
Jan> still worked.
[...]
Jan> Unfortunately since DT_NEEDED libpython this feature is already lost so it
Jan> should be OK now even for libunwind.so.7.

We could make this work if we truly wanted to.  It just seems like a lot
of effort; and building gdb is not so very hard.

The other day I was thinking we could even automate it by reading the
libpython debuginfo to generate weak declarations for the public API :-)
But, ok, that is not practical for regular gdb builds.

Tom

Re: [RFC/ia64] link against libunwind rather than using dlopen

[Joel Brobecker]

> We could make this work if we truly wanted to.  It just seems like a lot
> of effort; and building gdb is not so very hard.

I also think that this is unnecessary. The only reason for doing that
is transportability. But in either case, it's easy to achieve,
especially in the libunwind case where we only have to worry about
a couple of SOs, by also copying the libraries. That's what we do
when we distribute GDB with Python enabled.

-- 
Joel