Re: External debug symbols

[Alexander Larsson <alexl at redhat dot com>]

On Fri, 20 Sep 2002, Elena Zannoni wrote:

>  > +++ gdb-5.2.1.separate_debug_symbols/gdb/configure.in	Mon Aug 26 17:02:48 2002
>  > @@ -234,6 +234,14 @@
>  >  AC_CHECK_LIB(socket, socketpair)
>  >  AC_CHECK_FUNCS(socketpair)
>  >  
>  > +debugdir=${libdir}/debug
>  > +	 
>  > +AC_ARG_WITH(separate-debug-dir,
>  > +[  --with-separate-debug-dir=path   Look for global separate debug info in this path [LIBDIR/debug]],
>  > +[debugdir="${withval}"])
>  > +	
>  > +AC_DEFINE_DIR(DEBUGDIR, debugdir)
>  > +#AC_DEFINE_UNQUOTED(DEBUGDIR, "$debugdir"),
>  >  
> 
> Could this be done alternatively using an environment variable,
> instead of being set at configure time? something like
> DEBUG_INFO_PATH, or similar.  I don't have a strong preference, but
> seems to me that an environment variable is more flexible. 

It's currently a gdb variable "debug-file-search-path", and you can set
the default value at configure time. I can change the gdb variable
into an enviroment variable if you want. The only thing I need is to be
able to set the default path to /usr/lib/debug, without any
environment variable needed, because that is how we want to ship it by 
default in Red Hat.
 
>  > diff --exclude '*~' --exclude '*.orig' --exclude '*.rej' --exclude '.#*' --exclude '*.o' --exclude CVS --exclude SCCS --exclude RCS --exclude '.*.flags' --exclude .cvsignore --exclude .depend -ur gdb-5.2.1/gdb/objfiles.c gdb-5.2.1.separate_debug_symbols/gdb/objfiles.c
>  > --- gdb-5.2.1/gdb/objfiles.c	Thu Dec  6 21:59:11 2001
>  > +++ gdb-5.2.1.separate_debug_symbols/gdb/objfiles.c	Mon Sep 16 14:01:45 2002
>  > @@ -325,6 +325,27 @@
>  >    return (objfile);
>  >  }
>  >  
>  > +void
>  > +put_objfile_before (struct objfile *objfile, struct objfile *before_this)
>  > +{
>  > +  struct objfile **objp;
>  > +
>  > +  unlink_objfile (objfile);
>  > +  
>  > +  for (objp = &object_files; *objp != NULL; objp = &((*objp)->next))
>  > +    {
>  > +      if (*objp == before_this)
>  > +	{
>  > +	  objfile->next = *objp;
>  > +	  *objp = objfile;
>  > +	  return;
>  > +	}
>  > +    }
>  > +  
>  > +  internal_error (__FILE__, __LINE__,
>  > +		  "put_objfile_before: before objfile not in list");
>  > +}
>  > +
> 
> Thinking out loud...
> 
> I wonder if it wouldn't be more intuitive to add the debug info to the
> objfile, instead of creating another objfile.  So that in the end we
> would have just a new field added to the objfile, indicating the name
> of the debug info file.
> 
> It would definitely require more work. There are a few function you
> can look at to see how to do this: add_symbol_file itself, of course,
> and syms_from_objfile() (which is used in rs6000-tdep.c, in
> objfile_symbol_add) (btw, would objfile_symbol_add still work with
> your changes?).

They really are different object files though. I've for instance tested 
prelinking the main binary after stripping out the debug info (this is 
something we need to support). This works fine, but means that the exec 
lib will be relocated to another address, and the debug lib won't. So 
things like section_offsets in the two objfiles will differ.
 
> Does the reread_symbols() function still work?  To see this you need
> to run the inferior, then w/o quitting from gdb, modify the
> executable/recompile and then issue a 'run' command.  That would cause
> the excutable and symbols to be reread.  It walks the list of existing
> objfiles and updates those that need it, but it doesn't call
> symbol_file_add, so the crc check would not be done and the 2 files
> could get out of sync.

It looks like it would "work", but yeah, it can get out of sync. I'll look 
into this.
 
> What does the global variable symfile_objfile point to now?
> I wonder if lookups of this type would still work:
>   sym = lookup_minimal_symbol ("_Prelude", NULL, symfile_objfile);
> 
> If I understand correctly, we would have the minimal symbols hanging
> off the exec objfile, but the symbol tables would hang off the symbol
> objfile? (I haven't looked at this closely, maybe this is not the
> case). No, wait, I think we would have 2 copies of the minimal symbols?

I think there are two copies of the minimal symbols, yes.

>  > diff --exclude '*~' --exclude '*.orig' --exclude '*.rej' --exclude '.#*' --exclude '*.o' --exclude CVS --exclude SCCS --exclude RCS --exclude '.*.flags' --exclude .cvsignore --exclude .depend -ur gdb-5.2.1/gdb/objfiles.h gdb-5.2.1.separate_debug_symbols/gdb/objfiles.h
>  > --- gdb-5.2.1/gdb/objfiles.h	Sat Jun 29 00:05:47 2002
>  > +++ gdb-5.2.1.separate_debug_symbols/gdb/objfiles.h	Mon Sep 16 13:15:15 2002
>  > @@ -409,6 +409,9 @@
>  >      ExportEntry *export_list;
>  >      int export_list_size;
>  >  
>  > +    struct objfile *separate_debug_objfile;
>  > +    struct objfile *separate_debug_objfile_backlink;
>  > +    
> 
> Could you add some comments, here and elsewhere?
 
Sure.
 
>  > +  debugfile = find_separate_debug_file (objfile, name);
>  > +  if (debugfile)
>  > +    {
>  > +      printf_filtered ("loading separate debug info from '%s'\n", debugfile);
>  > +
>  > +      objfile->separate_debug_objfile = symbol_file_add (debugfile, from_tty, (addrs != NULL) ? &orig_addrs : NULL, 0, flags);
> 
> 
> Suggest using a simple conditional instead of a conditional expression.

Ok.
 
>  > +  unsigned char *end;
>  > +
>  > +  crc = ~crc & 0xffffffff;
>  > +  for (end = buf + len; buf < end; ++buf)
>  > +    crc = crc32_table[(crc ^ *buf) & 0xff] ^ (crc >> 8);
>  > +  return ~crc & 0xffffffff;;
>  > +}
>  > +
> 
> I don't think this function belongs in this file. It is general enough
> that it could be pushed into utils.c. Maybe even into some directory
> outside of gdb? It could be useful to other tools as well.

ok. 
 
> For the below, in theory is Ok, but I would suggest looking at the
> function openp() (in source.c) and how it is used in the solib_open()
> function in solib.c.  The tricky part is going to be assuring that the
> filename is portable, i.e. working on windows/dos systems where the
> directory separator is '\'. Look at IS_DIR_SEPARATOR, IS_ABSOLUTE_PATH
> in the libiberty directory.

I'll look into it.
 
> Once this is done, this change would need to be documented in the gdb
> manual, and a testcase added to the testsuite.

ok.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl@redhat.com    alla@lysator.liu.se 
He's a lounge-singing sweet-toothed gentleman spy on the edge. She's a foxy 
nymphomaniac bodyguard from a family of eight older brothers. They fight 
crime!