This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFA: libunwind basic support
- From: "J. Johnston" <jjohnstn at redhat dot com>
- To: Andrew Cagney <ac131313 at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Thu, 16 Oct 2003 17:38:04 -0400
- Subject: Re: RFA: libunwind basic support
- Organization: Red Hat Inc.
- References: <3F8DBC4C.1080304@redhat.com> <3F8EFD49.3060502@redhat.com>
Andrew Cagney wrote:
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?
http://www.hpl.hp.com/research/linux/libunwind/
- who owns it?
Hewlitt-Packard
- what are its licence terms, and are they GPL compatible?
It has a BSD-like license. Yes, it is GPL compatible.
Copyright (c) 2002 Hewlett-Packard Co.
Permission is hereby granted, free of charge, to any person obtaining
a copy of this software and associated documentation files (the
"Software"), to deal in the Software without restriction, including
without limitation the rights to use, copy, modify, merge, publish,
distribute, sublicense, and/or sell copies of the Software, and to
permit persons to whom the Software is furnished to do so, subject to
the following conditions:
The above copyright notice and this permission notice shall be
included in all copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- is it considered a "system library" (like libc, or libthread_db)?
I would have to say no. It is currently an optional library. This is one of
the reasons I chose to use a dynamic load mechanism.
- have you used, or refered to David Mossberger's [sp] old libunwind
patch (it was eventually contributed to the FSF).
No. Not in this code.
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.
I'll take a look. The only thing this allows a user to do is to build gdb on a
system that it can legimately build libunwind support and purposely disable it
at configuration. Can you disable dwarf2 cfi support? If not, why would you
want to do this for libunwind which is a similar level of functionality?
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.
Will change.
+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?
I did look at said functionality originally. I ran into a problem which this
solved. I will have to look again to see what the problem was because I don't
remember off-hand.
+#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.
+ memset (valuep, 0, DEPRECATED_REGISTER_RAW_SIZE (regnum));
? you probably want register_size().
Yes.
+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.
IMO, there is little value-add. There are architecture messages that will print
out whether libunwind is being used or not. The problems with libthread-db
don't exist in this case.
+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.
Ok. I copied the declaration from below and forget to rejoin the lines.
-- Jeff J.