Re: RFC: support debug info in separate files

[Jim Blandy <jimb at redhat dot com>]

Elena Zannoni <ezannoni@redhat.com> writes:

> Jim Blandy writes:
>  > 
>  > This patch, written by Alexander Larsson, allows GDB to use debug info
>  > that has been extracted into a separate executable file, using the
>  > "strip -f" command as provided in Ulrich Drepper's elfutils.  This
>  > allows a distribution to provide debug info in separate packages from
>  > the executables it describes.
>  > 
> 
> Actually thinking about this, where can one get a copy of elfutils?
> Could the same functionality be added to binutils?

Sure, I don't see why not.

>  > If you've got all the previous patches I've posted (and if I haven't
>  > botched anything), this should cause no test suite regressions, and
>  > the results of running the test suite both with and without separated
>  > debug info are identical.  Starting with current GDB sources, the
>  > other patches you need are:
>  > 
>  > RFA: testsuite: pass optimization flag in the proper way
>  > RFA: testsuite: attach.exp: don't move executables to /tmp
>  > Re: RFC: allow syms_from_objfile to take a section offset table directly
>  > RFC: Allow symbol_file_add to take section_offsets table
>  > 
>  > To run the GDB test suite against executables with separated debug info, 
>  > see: http://sources.redhat.com/ml/gdb/2002-11/msg00202.html
>  > and: http://sources.redhat.com/ml/gdb/2002-11/msg00215.html
>  > 
>  > (Note: there is a bug in the elfutils strip that causes some stripped
>  > executables to crash.  This has been fixed only very recently, so
>  > you'll need a bleeding-edge version of elfutils, or else you'll get
>  > failures in try_catch.exp due to the inferior crashing.)
>  > 
>  > If folks don't have concerns about this, I'll commit it on Fri Dec 6.
>  > 
> 
> Some random comments...
> 
> This works only for Elf. Will this interfere when other file formats
> are processed? (I haven't tried with a, say, coff file, which is
> impossible of course because this is elfutils based).

Actually, the GDB side of things is completely format-agnostic.  It
should work with anything that BFD says has a section named
".gnu_debuglink".  If you can get the file constructed in the first
place, GDB should handle it.  It shouldn't affect GDB's handling of
files that don't have .gnu_debuglink sections at all.

>  > 	* utils.c (calc_crc32): New function.
>  > 	* defs.h (calc_crc32): New declaration.
> 
> Now we have 4 identical crc32's functions in gdb. Any chance to
> delete a few?

The one in utils.c and the one in remote.c are not the same:

(top-gdb) print calc_crc32(0, "howdy", 5)
$2 = 1976775978
(top-gdb) print crc32("howdy", 5, 0)
$3 = 701963740

A CRC isn't a well-defined function like MD5 or SHA; you also need to
know which polynomial the CRC is using, and maybe some other things I
don't really understand.  It looks to me as if crc32 and calc_crc32
are using different polynomials.

The choice of which CRC to use in these cases is a matter of public
interface.  We obviously can't change the one remote.c uses, since it
has complementary functions in stubs all over the place.  Since
.gnu_debuglink is a new feature, its CRC function could be changed
without much trouble, but since we'll run into a variety of CRC's
anyway, it's not worth the trouble.

The best thing is probably to give the functions names that better
reflect their purpose; calling them just "crc" is a bit like naming a
Modula-2 compiler "compile".  gnu_debuglink_crc32 and
gdb_remote_crc32?  (It would be nice if someone who really understands
CRCs could write a comment that characterizes each of them.)

m32r-stub.c's crc32 clearly needs to match the one in remote.c ---
which, in fact, it does.  Moving the GDB remote protocol's CRC
function into a separate file, so it could be shared with the stub, is
a possibility, but it'd need to be compiled separately for the host
and target machines.  Definitely work for a separate patch, in any
case.

xmodem.c has a 16-bit CRC, not a 32-bit CRC.  Again a matter of public
protocol.

> For the debug file name suggest looking at HAVE_DOS_BASED_FILE_SYSTEM
> in libiberty, and its uses in gdb/source.c.
> 
> [...]
> 
>  > +  strcat (debugfile, ".debug/");
> [...]
> 
>  > +  strcat (debugfile, "/");
> 
> [...]
>  > +  strcat (debugfile, "/");
> 
> Should these be DIR_SEPARATOR instead? I guess DJGPP doesn't care
> though.

Yeah.  Eli has said it doesn't matter, but I think using DIR_SEPARATOR
would be better.

> In this message,
> http://sources.redhat.com/ml/gdb/2002-09/msg00312.html I pointed out a
> few things that could be done to improve this patch.  For instance,
> instead of adding a completely new objfile that would be only for the
> debug info, add the debug info to the existing objfiles.  I haven't
> had a chance to see if you changed the patch to do something different
> or not.

Yes, I remember the suggestion; this patch still creates a separate
objfile.  It *seems* to me that reading two file's data into a single
objfile structure would make the patch quite a bit more complex than
it is.  It'd be nice to see the patch, so we could assess how much of
a difference it made.

> It also seemed at that stage that we were gaining an extra
> copy of the minimal symbols, and this can bloat gdb even more. Was
> this changed?

No, it wasn't changed.  There is some duplication going on, but not
quite the way the question suggests.  Here's the whole story:

An unstripped executable has separate symbol tables for the static
linker (.symtab/.strtab) and the dynamic linker
(.dynsym/.dynstr/.hash).  The ordinary `strip' command everyone has
been using for years deletes the static linker symbols and the debug
info, but leaves the dynamic linker symbol table there --- since the
program can't run without it.  The new Elfutils 'strip -f' command
moves the static linker symbols and the debug info into a separate
file, instead of just tossing it.  So 'strip -f' itself never
duplicates anything.  The resulting pair of files contains the same
amount of information the original file did.

The dynamic symtab is usually just a subset of the static symtab, so
there is duplication there.  GDB constructs its minsym table by
reading both the static and dynamic linkers' symbol tables, but if
they're both present, it ignores almost everything from the dynamic
symtab (except for symbols naming PLT entries).  (See elf_symtab_read,
called from elf_symfile_read, for details.)  So the duplication
present in an ordinary executable's static and dynamic linker symbol
tables doesn't bother GDB.

In a separated executable, however, GDB doesn't realize that, although
the stripped file does not have a static linker symbol table, it will
eventually be getting those symbols from the separated debug file.  So
it effectively reads in both the dynamic and static linker symtabs,
where otherwise it would ignore the dynamic symtab.

    $ nm gdb | wc -l
       7970
    $ nm -D gdb | wc -l
       4059
    $

So there's quite a bit of extra data involved.

In context, though, the alternative to using separated debug files is
to have no debug info at all, or to make the user recompile from
source.  So while the dynamic/static duplication should be addressed,
I don't think the patch should be blocked on it.