This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Make sym_read routines handle separate debug files
On Dec 4, 2009, at 7:01 PM, Tom Tromey wrote:
>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
>
> Tristan> Each object file reader is now responsible to read separate
> Tristan> debug files as they are object file dependant. Of course, in
> Tristan> the case of gnu .debuglink a common subprogram is used.
>
> You updated coffread and elfread, but the existing code in gdb does not
> depend on the format. So it seems to me that any other code defining
> sym_fns will regress. That is, dbxread, mipsread, somread, and
> xcoffread (I omitted machoread since I presume your later patch fixes
> that up).
dbxread is for a.out, which doesn't support .gnu_debuglink because it only has 2 sections (.text and .data).
somread is for PA-HP/UX which doesn't support .gnu_debuglink because it doesn't fit in space/subspace
scheme.
xcoffread is for AIX and xcoff section names are limited to 8 bytes. So .gnu_debuglink will never exist
on this platform.
mipsread is for ecoff used by mips and alpha. Their coff sections name are limited to 8 bytes too.
machoread bfd backend doesn't know about .gnu_debuglink so this feature doesn't work with Mach-O. This is
useless because Mach-O has dsymfiles which are roughly equivalent to .gnu_debuglink.
> Could we not just keep the existing logic as a format-independent
> fallback?
I was hesitant to do that given that only ELF and some COFF support .gnu_debuglink.
> Or am I mistaken about something? Maybe it is impossible for those
> formats to have separate debug files? I know nothing of the details,
> I'm afraid.
Sorry, that's my fault. I should have explained why I changed only ELF and COFF.
(I also forgot to say that there were no regression on x86 GNU/Linux).
> Tristan> + debugfile = find_separate_debug_file_by_buildid (objfile);
>
> If this is only meaningful for ELF, as it seems to be, then it seems we
> might as well put it in elfread.c.
To be honest, I reserved this move for a following patch. Do you prefer I resubmit this patch with this
change ?
(I'd prefer to make a separate patch to keep this one smaller, even if it already contains some
re-indentation).
> Tristan> +/* Add BFD as a separate debug file for OBJFILE. */
> Tristan> +
> Tristan> +void
> Tristan> +symbol_file_add_separate (bfd *bfd, int symfile_flags, struct objfile *objfile)
> Tristan> +{
> Tristan> + objfile->separate_debug_objfile =
>
> I'm wondering if this should have a sanity check and call internal_error
> if the separate debug objfile is already set.
Fine. Will do this change.
> Tristan> @@ -2320,204 +2314,217 @@ reread_symbols (void)
> [...]
> Tristan> + /* The "mainline" parameter is a hideous hack; I think leaving it
> Tristan> + zero is OK since dbxread.c also does what it needs to do if
> Tristan> + objfile->global_psymbols.size is 0. */
>
> Normally I wouldn't ask for changes in a simple reindentation (and there
> are other stale comments here that I didn't single out), but this
> particular comment seems relevant to the overall change. I suggest just
> removing it, but rewording would be ok too.
Thank for this catch. I think we could remove it.
Again thank you for the whole review,
Tristan.