Patch resubmitted.  See comments below.


Andrew Cagney wrote:
> 
>>> +#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?
> 
> 
>> Yes.  The libunwind code is slightly ugly with respect to the fact 
>> that the function names are not aliased with generic names.  They all 
>> have platform prefixes so I must spell them out.  Function names are 
>> generated automatically using the UNW_OBJ macro.
> 
> 
> Can you please add this to the file as a comment.
> 

Done.

> 
> 
>> +void
>> +libunwind_frame_set_descr (struct gdbarch *gdbarch, struct 
>> libunwind_descr *descr)
>> +{
>> +  struct libunwind_descr *arch_descr;
>> +
>> +  gdb_assert (gdbarch != NULL);
>> +
>> +  arch_descr = gdbarch_data (gdbarch, libunwind_descr_handle);
>> +
>> +  if (arch_descr == NULL)
>> +    {
>> +      /* First time here.  Must initialize data area.  */
>> +      arch_descr = libunwind_descr_init (gdbarch);
>> +      set_gdbarch_data (gdbarch, libunwind_descr_handle, arch_descr);
>> +    }
>> +
>> +  /* Copy new descriptor info into arch descriptor.  */
>> +  arch_descr->gdb2uw = descr->gdb2uw;
>> +  arch_descr->uw2gdb = descr->uw2gdb;
>> +  arch_descr->is_fpreg = descr->is_fpreg;
>> +  arch_descr->accessors = descr->accessors;
>> +}
> 
> 
> Yes!  Now the architecture has-a libunwind structure.
> 
> Doesn't the configury make this:
> 
>> +#ifdef HAVE_LIBUNWIND_H
> 
> 
> redundant?
>

It is possible for the user to enable libunwind but the header files are not 
there.  I am guessing you would prefer compilation to fail in this case so I 
have removed the check.

>> +  libunwind_descr_handle = register_gdbarch_data (libunwind_descr_init);
>> +
>> +  libunwind_initialized = libunwind_load ();
>> +#endif  
> 
> 
>> +   Contributed by Jeff Johnston.
> 
> 
> (redhat)
> 

Changed.

> 
>> +const struct frame_unwind *libunwind_frame_sniffer (struct frame_info 
>> *next_frame);
> 
> 
> Comments for this one ...
> 

Done.

>> +void libunwind_frame_set_descr (struct gdbarch *arch, struct 
>> libunwind_descr *descr);
>> +
>> +void libunwind_frame_this_id (struct frame_info *next_frame, void 
>> **this_cache,
>> +                  struct frame_id *this_id);
>> +void libunwind_frame_prev_register (struct frame_info *next_frame, 
>> void **this_cache,
>> +                    int regnum, int *optimizedp,
>> +                    enum lval_type *lvalp, CORE_ADDR *addrp,
>> +                    int *realnump, void *valuep);
>> +CORE_ADDR libunwind_frame_base_address (struct frame_info 
>> *next_frame, void **this_cache);
>> +
>> +int libunwind_is_initialized (void);
>> +
>> +int libunwind_search_unwind_table (void *as, long ip, void *di,
>> +                   void *pi, int need_unwind_info, void *args);
> 
> 
> Also comments for this one ...
> 

I have added comments for the search unwind table call.

>> +unw_word_t libunwind_find_dyn_list (unw_addr_space_t, void *, size_t,
>> +                    unw_word_t, unw_word_t, void *);
> 
> 
>> Index: Makefile.in
> 
> 
> Yep.
> 
>> +    libunwind-frame.c \
> 
> 
>> -    $(elf_bfd_h) $(dis_asm_h)
>> +    $(elf_bfd_h) $(dis_asm_h) $(libunwind_frame_h)
> 
> 
>> +libunwind-frame.o: libunwind-frame.c $(defs_h) \
>> +    $(frame_h) $(frame_base_h) $(frame_unwind_h) $(gdbcore_h) \
>> +    $(gdbtypes_h) $(symtab_h) $(objfiles_h) $(regcache_h) \
>> +    $(gdb_assert_h) $(gdb_string_h) $(complaints_h) $(libunwind_frame_h)
> 
> 
>> Index: configure.in
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/configure.in,v
>> retrieving revision 1.132
>> diff -u -r1.132 configure.in
>> --- configure.in    3 Sep 2003 15:02:48 -0000    1.132
>> +++ configure.in    22 Oct 2003 23:36:21 -0000
>> @@ -192,6 +192,33 @@
>>      enable_gdbtk=no ;;
>>  esac
> 
> 
> Based on 12.1 Working With External Software
> http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_chapter/autoconf_12.html#SEC130 
> 
> 
> I suspect that this should be --with (external libunwind software), and 
> not --enable (gdb feature), although, to be honest, its hard to tell 
> which it should be.
> 
> Anyone?
>

 From discussions with Jeff Law when I was doing configuration for newlib, he 
told me that --enable should be used for on/off decisions in preference to 
--with.  I don't personally care so I have switched to --with.

>> +# Enable libunwind support.
>> +AC_ARG_ENABLE(libunwind,
>> +[  --enable-libunwind          enable libunwind frame unwinding 
>> support],
>> +  [case $enableval in
>> +    yes | no)
> 
> 
> The behavior should be:
> 
> --{with,enable}-libunwind
> Enable libunwind unconditionally.
> 
> --{without,disable}-libunwind
> Disable libunwind unconditionally
> 
>> +      ;;
>> +    *)
>> +      AC_MSG_ERROR([bad value $enableval for --enable-libunwind]) ;;
>> +  esac]
> 
> 
> <none>
> Check for "libunwind.h" and "libunwind-ia64.h" and if present, enable 
> it.  Something like:
>     AC_CHECK_HEADERS(libunwind.h)
>     AC_CHECK_HEADERS(libunwind-ia64.h)
>     test $ac_cv_have_libunwind_h = yes && enable_libunwind=yes
> 
> )
> 
> And then just:
> 
>> +  if test x"$enable_libunwind" = xyes; then
>> +    AC_CHECK_HEADERS(libunwind.h)
>> +    AC_CHECK_HEADERS(libunwind-ia64.h)
>> +    CONFIG_OBS="$CONFIG_OBS libunwind-frame.o"
>> +    CONFIG_DEPS="$CONFIG_DEPS libunwind-frame.o"
>> +    CONFIG_SRCS="$CONFIG_SRCS libunwind-frame.c"
> 
> 
> but also AC_DEFINE(HAVE_LIBUNWIND) so that it can be used.
> 

Done.