This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFA: Try to include libunwind-ia64.h in libunwind-frame.h
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 <gingold@adacore.com>
Pedro Alves <palves@redhat.com>
* 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 <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
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 <http://www.gnu.org/licenses/>. */
-#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 */