RFA: libunwind basic support

[Andrew Cagney]

Jeff,

Is it possible to post (or commit to a branch) the other 
(work-in-progress?) parts to this change?
Ignoring a few nits, this code appears to be going in the right 
direction, however its hard to tell without seeing things in toto.

> The attached patch adds basic libunwind frame support.  I will be shortly submitting an ia64-tdep.c patch which works in conjunction with this patch.  The libunwind code is protected by looking for the libunwind header files to compile.  At runtime, the libunwind frame sniffer will fail if either the libunwind library cannot be dynamically loaded or the frame in question does not have proper libunwind info.

First some straight legal questions:

- from where can libunwind be obtained?

- who owns it?

- what are its licence terms, and are they GPL compatible?

- is it considered a "system library" (like libc, or libthread_db)?

- have you used, or refered to David Mossberger's [sp] old libunwind 
patch (it was eventually contributed to the FSF).

> For configuration I have added checks for libunwind.h and libunwind-ia64.h as this is being added for ia64 support only at present.  Regarding testing, I have this code working with my ia64 changes.  I have tested on systems with the libunwind-0.92 library/headers installed and not installed.

I see, to make this optional, you've wrapped the code in #ifdef 
HAVE_LIBUNWIND_H, and then modified the Makefile so that it is 
unconditionally built :-(

These files should only be linked in when when needed, or at least only 
when there's a libunwind around.  Have a look at --with-mmalloc and 
--with-sysroot for how to implement the option --with-libunwind which 
lets the user force their inclusion; and --disable-gdbcli, --enable-tui, 
and --enable-sim for idea's on how to make linking the object files 
optional.

> 2003-10-15  Jeff Johnston  <jjohnstn@redhat.com>
> 
>     * libunwind-frame.c: New file.
>     * libunwind-frame.h: New file.
>     * configure.in: Add checks for libunwind.h and libunwind-ia64.h.
>     * configure: Regenerated.
>     * Makefile.in: Add support for libunwind-frame.o.
>     * config.in: Regenerated.

Without seeing the rest of the code I'm really not in a position to 
comment on the technical design.

However, a quick glance did through up a few nits.  I'm noting them now, 
so that, hopefully an additional round trip can be avoided later.

+   Contributed by Jeff Johnston

Legal nit.  "Written by Jeff Johnston, contributed by Red Hat Inc.", you 
don't have an assignment on file.

> +void
> +libunwind_set_descr_handle (void *handle)
> +{
> +  libunwind_descr_handle = handle;
> +}

I don't understand this.  A guess is that this is trying to implement a 
mechanism that lets an external module set this modules per-architecture 
data?  If that is the case, then can you have a look at the 
set_gdbarch_data doco and regrroups.c's reggroup_add method for how to 
do this?

+#define STRINGIFY2(name)	#name
+#define STRINGIFY(name)		STRINGIFY2(name)
+
+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_saveloc));
+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 *search_unwind_table_name = 
STRINGIFY(UNW_OBJ(search_unwind_table));
+static char *find_dyn_list_name = STRINGIFY(UNW_OBJ(find_dyn_list));

I don't understand this.  A guess is that UNW_OBJ() is doing something 
evil (use "include/sym-cat.h") to those names and having the array (use 
"static const char <name>[] = ..." and local to libunwind_load) makes 
ones debugging life much easier?  If this is the case, can you add some 
commentary?

+  memset (valuep, 0, DEPRECATED_REGISTER_RAW_SIZE (regnum));

? you probably want register_size().

+static int
+libunwind_load (void)
+{
+  void *handle;
+
+  handle = dlopen (LIBUNWIND_SO, RTLD_NOW);
+  if (handle == NULL)
+    return 0;

For thread-db.c I added code that printed out the library that was 
loaded.  Should the same be done here?   Note that, due to querks with 
the way GDB starts up, the message needs to be delayed - see thread-db.c 
for more details.

+const struct frame_unwind *
+libunwind_frame_sniffer (struct frame_info *next_frame);

can you please write the declaration thus:

const struct frame_unwind *libunwind_frame_sniffer (....

so that it is consistent with the rest of GDB.

Andrew