From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14148 invoked by alias); 7 Dec 2009 10:50:18 -0000 Received: (qmail 14133 invoked by uid 22791); 7 Dec 2009 10:50:16 -0000 X-SWARE-Spam-Status: No, hits=0.4 required=5.0 tests=AWL,BAYES_00,KAM_STOCKTIP X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (212.99.106.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Dec 2009 10:50:12 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id F41FE290014; Mon, 7 Dec 2009 11:50:09 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LsFrbppN1lbO; Mon, 7 Dec 2009 11:50:09 +0100 (CET) Received: from ulanbator.act-europe.fr (ulanbator.act-europe.fr [10.10.1.67]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id 5DAB5290029; Mon, 7 Dec 2009 11:50:06 +0100 (CET) Subject: Re: [RFA] Make sym_read routines handle separate debug files Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: text/plain; charset=us-ascii From: Tristan Gingold In-Reply-To: Date: Mon, 07 Dec 2009 10:50:00 -0000 Cc: gdb-patches@sourceware.org Content-Transfer-Encoding: quoted-printable Message-Id: <2CC5EFB8-A1BC-4025-9136-E8FA344B2C04@adacore.com> References: <20091204124838.GA5764@ulanbator.act-europe.fr> To: tromey@redhat.com X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-12/txt/msg00073.txt.bz2 On Dec 4, 2009, at 7:01 PM, Tom Tromey wrote: >>>>>> "Tristan" =3D=3D Tristan Gingold writes: >=20 > 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. >=20 > 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 doe= sn't fit in space/subspace scheme. xcoffread is for AIX and xcoff section names are limited to 8 bytes. So .g= nu_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 doe= sn't work with Mach-O. This is useless because Mach-O has dsymfiles which are roughly equivalent to .gnu_d= ebuglink. > 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_de= buglink. > 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 =3D find_separate_debug_file_by_buildid (objfil= e); >=20 > 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 a= lready 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 o= bjfile *objfile) > Tristan> +{ > Tristan> + objfile->separate_debug_objfile =3D >=20 > 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 leavi= ng it > Tristan> + zero is OK since dbxread.c also does what it needs to do = if > Tristan> + objfile->global_psymbols.size is 0. */ >=20 > 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.