Re: RFA: libunwind basic support

[Andrew Cagney <ac131313@redhat.com>]

>> +#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.



> +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?

> +  libunwind_descr_handle = register_gdbarch_data (libunwind_descr_init);
> +
> +  libunwind_initialized = libunwind_load ();
> +#endif  

> +   Contributed by Jeff Johnston.

(redhat)


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

Comments for this one ...

> +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 ...

> +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?

> +# 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.

Andrew