From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1088 invoked by alias); 14 Feb 2012 12:14:38 -0000 Received: (qmail 1054 invoked by uid 22791); 14 Feb 2012 12:14:34 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Feb 2012 12:14:18 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1ECDtI3010053 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 Feb 2012 07:13:55 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q1ECDrSE016667; Tue, 14 Feb 2012 07:13:54 -0500 Message-ID: <4F3A5001.4090500@redhat.com> Date: Tue, 14 Feb 2012 12:14:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120131 Thunderbird/10.0 MIME-Version: 1.0 To: Jan Kratochvil CC: Tristan Gingold , "gdb-patches@sourceware.org ml" Subject: Re: RFA: Try to include libunwind-ia64.h in libunwind-frame.h References: <5D1CD28F-F628-475C-B6D8-5FCBF5290C63@adacore.com> <20120210182705.GA32459@host2.jankratochvil.net> <4F3562FE.7050106@redhat.com> <20120211140919.GA24043@host2.jankratochvil.net> <4F395D17.5070303@redhat.com> <20120213190223.GA8851@host2.jankratochvil.net> <4F396251.9020409@redhat.com> <20120213192652.GA11522@host2.jankratochvil.net> <4F396CDC.7020504@redhat.com> <20120214072735.GA21362@host2.jankratochvil.net> In-Reply-To: <20120214072735.GA21362@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-02/txt/msg00259.txt.bz2 On 02/14/2012 07:27 AM, Jan Kratochvil wrote: > On Mon, 13 Feb 2012 21:04:44 +0100, Pedro Alves wrote: >> On 02/13/2012 07:26 PM, Jan Kratochvil wrote: >>> On Mon, 13 Feb 2012 20:19:45 +0100, Pedro Alves wrote: >>>> On 02/13/2012 07:02 PM, Jan Kratochvil wrote: >>>>> It is required for ia64 but it can be used even with non-ia64 archs. >>>> >>>> How? AFAICS, no other target installs the libunwind sniffer. It's just >>>> dead code on other archs, if I'm reading the code correctly. >>> >>> I see now. I did not know. Sure in this case this patch of mine was wrong. >>> I will therefore make libunwind usable only with ia64, this will be different >>> patch removing some parts of gdb/ code. >> >> I was only thinking of the below. Would this work for everyone? >> >> I don't have a cross build of libunwind for ia64 handy, but I assume >> this works, given the previous patches... > > the patch does not apply to HEAD, Huh. It did for me. > with hand-application and CPPFLAGS="-I/tmp/libunwind-root-ia64/include" CFLAGS="-g $CPPFLAGS" LDFLAGS=-L/tmp/libunwind-root-ia64/lib ./configure --enable-targets=all > > getting: > > ia64-tdep.c: In function ‘ia64_pseudo_register_read’: > ia64-tdep.c:946:7: error: implicit declaration of function ‘libunwind_is_initialized’ [-Werror=implicit-function-declaration] > ia64-tdep.c:947:4: error: implicit declaration of function ‘libunwind_get_reg_special’ [-Werror=implicit-function-declaration] Ah. libunwind-frame.h is wrapped in HAVE_LIBUNWIND_H... > config.h: > /* #undef HAVE_LIBUNWIND */ > #define HAVE_LIBUNWIND_IA64_H 1 I missed a "test" in configure.ac. > > If the non-ia64 libunwind support is therefore really removed the dead code in > libunwind-frame.c should be also removed with some comments making it ia64 > specific. What dead code? The code is not really ia64 specific. The endianess checks? I don't think it's really worth the bother. The problem is that libunwind-frame.c is supposedly a host|target-independent file, but you you have things like: /* Required function pointers from libunwind. */ static int (*unw_get_reg_p) (unw_cursor_t *, unw_regnum_t, unw_word_t *); static int (*unw_get_fpreg_p) (unw_cursor_t *, unw_regnum_t, unw_fpreg_t *); static int (*unw_get_saveloc_p) (unw_cursor_t *, unw_regnum_t, unw_save_loc_t *); static int (*unw_is_signal_frame_p) (unw_cursor_t *); static int (*unw_step_p) (unw_cursor_t *); static int (*unw_init_remote_p) (unw_cursor_t *, unw_addr_space_t, void *); static unw_addr_space_t (*unw_create_addr_space_p) (unw_accessors_t *, int); static void (*unw_destroy_addr_space_p) (unw_addr_space_t); static int (*unw_search_unwind_table_p) (unw_addr_space_t, unw_word_t, unw_dyn_info_t *, unw_proc_info_t *, int, void *); static unw_word_t (*unw_find_dyn_list_p) (unw_addr_space_t, unw_dyn_info_t *, void *); /* We need to qualify the function names with a platform-specific prefix to match the names used by the libunwind library. The UNW_OBJ macro is provided by the libunwind.h header file. */ #define STRINGIFY2(name) #name #define STRINGIFY(name) STRINGIFY2(name) #ifndef LIBUNWIND_SO /* Use the stable ABI major version number. `libunwind-ia64.so' is a link time only library, not a runtime one. */ #define LIBUNWIND_SO "libunwind-" STRINGIFY(UNW_TARGET) ".so.7" #endif static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg)); static char *get_fpreg_name = STRINGIFY(UNW_OBJ(get_fpreg)); static char *get_saveloc_name = STRINGIFY(UNW_OBJ(get_save_loc)); static char *is_signal_frame_name = STRINGIFY(UNW_OBJ(is_signal_frame)); static char *step_name = STRINGIFY(UNW_OBJ(step)); static char *init_remote_name = STRINGIFY(UNW_OBJ(init_remote)); static char *create_addr_space_name = STRINGIFY(UNW_OBJ(create_addr_space)); static char *destroy_addr_space_name = STRINGIFY(UNW_OBJ(destroy_addr_space)); static char *search_unwind_table_name = STRINGIFY(UNW_OBJ(search_unwind_table)); static char *find_dyn_list_name = STRINGIFY(UNW_OBJ(find_dyn_list)); and this all depends at compile time, on a specific libunwind-$arch.h header having been included. Also notice that the unw_word_t type appears in the libunwind-frame.h interface. unw_word_t is either 64-bit or 32-bit depending on which libunwind-$arch.h header you include. So if we cared for any other arch, we'd need to make all these run-time dependent on the target we're talking to. Maybe we'd add a new libunwind-frame-$arch.c file for each arch we supported, which included libunwind-$arch.h, and filled a structure that contained function pointers that libunwind-frame.c would use. We'd also have to do something about unw_word_t. Probably export from libunwind-frame.c two versions of the current libunwind_find_dyn_list function: -unw_word_t libunwind_find_dyn_list (unw_addr_space_t, unw_dyn_info_t *, void *); +uint32_t libunwind_find_dyn_list32 (unw_addr_space_t, unw_dyn_info_t *, void *); +uint64_t libunwind_find_dyn_list64 (unw_addr_space_t, unw_dyn_info_t *, void *); in order to have the libunwind-frame user to call the proper function in the libunwind-$arch.so with the right prototype. > I believe the original goal was to make the libunwind support in GDB > arch-independent but it has been done only half-way and I agree it is OK to > make libunwind support really ia64-only. > RFA: libunwind basic support > http://sourceware.org/ml/gdb-patches/2003-10/msg00504.html > > > + AC_CHECK_HEADERS(libunwind-ia64.h) > + if x"$ac_cv_header_libunwind_ia64_h" = xyes; then > > This should use threfore AC_CHECK_HEADER. AC_CHECK_HEADERS takes care of defining HAVE_HEADER_FOO_H, AC_CHECK_HEADER does not, so AC_CHECK_HEADERS is a better fit. This version is compile tested with current git libunwind built with --target=ia64-linux-gnu (also --enable-targets=all), and also compile tested without libunwind headers in the include path (and confirmed libunwind-frame.o wasn't built). 2012-02-14 Tristan Gingold Pedro Alves * ia64-tdep.c: Do not include libunwind-ia64.h. * libunwind-frame.h: Remove #ifdef HAVE_LIBUNWIND_H guard. Include libunwind-ia64.h instead of libunwind.h. * configure.ac (--with-libunwind, $enable_libunwind): Don't check for libunwind.h existence. * configure, config.in: Regenerate. --- gdb/config.in | 3 --- gdb/configure | 22 +++++++++------------- gdb/configure.ac | 6 +++--- gdb/ia64-tdep.c | 1 - gdb/libunwind-frame.h | 12 +++++++----- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/gdb/config.in b/gdb/config.in index bae1763..194cc7d 100644 --- a/gdb/config.in +++ b/gdb/config.in @@ -242,9 +242,6 @@ /* Define if libunwind library is being used. */ #undef HAVE_LIBUNWIND -/* Define to 1 if you have the header file. */ -#undef HAVE_LIBUNWIND_H - /* Define to 1 if you have the header file. */ #undef HAVE_LIBUNWIND_IA64_H diff --git a/gdb/configure b/gdb/configure index 2566410..9cb3742 100755 --- a/gdb/configure +++ b/gdb/configure @@ -8252,21 +8252,19 @@ if test "${with_libunwind+set}" = set; then : esac else - for ac_header in libunwind.h libunwind-ia64.h + for ac_header in libunwind-ia64.h do : - as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` -ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" -eval as_val=\$$as_ac_Header - if test "x$as_val" = x""yes; then : + ac_fn_c_check_header_mongrel "$LINENO" "libunwind-ia64.h" "ac_cv_header_libunwind_ia64_h" "$ac_includes_default" +if test "x$ac_cv_header_libunwind_ia64_h" = x""yes; then : cat >>confdefs.h <<_ACEOF -#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1 +#define HAVE_LIBUNWIND_IA64_H 1 _ACEOF fi done - if test x"$ac_cv_header_libunwind_h" = xyes -a x"$ac_cv_header_libunwind_ia64_h" = xyes; then + if test x"$ac_cv_header_libunwind_ia64_h" = xyes; then enable_libunwind=yes; fi @@ -8274,14 +8272,12 @@ fi if test x"$enable_libunwind" = xyes; then - for ac_header in libunwind.h libunwind-ia64.h + for ac_header in libunwind-ia64.h do : - as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` -ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" -eval as_val=\$$as_ac_Header - if test "x$as_val" = x""yes; then : + ac_fn_c_check_header_mongrel "$LINENO" "libunwind-ia64.h" "ac_cv_header_libunwind_ia64_h" "$ac_includes_default" +if test "x$ac_cv_header_libunwind_ia64_h" = x""yes; then : cat >>confdefs.h <<_ACEOF -#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1 +#define HAVE_LIBUNWIND_IA64_H 1 _ACEOF fi diff --git a/gdb/configure.ac b/gdb/configure.ac index 1b11adb..c4c84a0 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -348,14 +348,14 @@ AS_HELP_STRING([--with-libunwind], [use libunwind frame unwinding support]), 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 + AC_CHECK_HEADERS(libunwind-ia64.h) + if test x"$ac_cv_header_libunwind_ia64_h" = xyes; then enable_libunwind=yes; fi ]) if test x"$enable_libunwind" = xyes; then - AC_CHECK_HEADERS(libunwind.h libunwind-ia64.h) + AC_CHECK_HEADERS(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/ia64-tdep.c b/gdb/ia64-tdep.c index a297ecc..a36dc22 100644 --- a/gdb/ia64-tdep.c +++ b/gdb/ia64-tdep.c @@ -43,7 +43,6 @@ #ifdef HAVE_LIBUNWIND_IA64_H #include "elf/ia64.h" /* for PT_IA_64_UNWIND value */ #include "libunwind-frame.h" -#include "libunwind-ia64.h" /* Note: KERNEL_START is supposed to be an address which is not going to ever contain any valid unwind info. For ia64 linux, the choice diff --git a/gdb/libunwind-frame.h b/gdb/libunwind-frame.h index 0251819..ef98177 100644 --- a/gdb/libunwind-frame.h +++ b/gdb/libunwind-frame.h @@ -19,8 +19,6 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ -#ifdef HAVE_LIBUNWIND_H - struct frame_info; struct frame_id; struct regcache; @@ -29,7 +27,13 @@ struct gdbarch; #ifndef LIBUNWIND_FRAME_H #define LIBUNWIND_FRAME_H 1 -#include "libunwind.h" +/* IA-64 is the only target that currently uses libunwind. If some + other target wants to use it, we will need to do some abstracting + in order to make it possible to have more than one libunwind-frame + instance. Including "libunwind.h" is wrong as that ends up + including the libunwind-$(arch).h for the host gdb is running + on. */ +#include "libunwind-ia64.h" struct libunwind_descr { @@ -72,5 +76,3 @@ int libunwind_get_reg_special (struct gdbarch *gdbarch, int regnum, void *buf); #endif /* libunwind-frame.h */ - -#endif /* HAVE_LIBUNWIND_H */