From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28642 invoked by alias); 21 Apr 2006 01:10:01 -0000 Received: (qmail 28620 invoked by uid 22791); 21 Apr 2006 01:10:00 -0000 X-Spam-Check-By: sourceware.org Received: from xproxy.gmail.com (HELO xproxy.gmail.com) (66.249.82.205) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 21 Apr 2006 01:09:58 +0000 Received: by xproxy.gmail.com with SMTP id s16so227577wxc for ; Thu, 20 Apr 2006 18:09:56 -0700 (PDT) Received: by 10.70.75.9 with SMTP id x9mr1567099wxa; Thu, 20 Apr 2006 18:09:54 -0700 (PDT) Received: by 10.70.125.5 with HTTP; Thu, 20 Apr 2006 18:09:54 -0700 (PDT) Message-ID: <8f2776cb0604201809l68067aey47fa372a2468c8ea@mail.gmail.com> Date: Fri, 21 Apr 2006 01:10:00 -0000 From: "Jim Blandy" To: "Frederic RISS" Subject: Re: [RFC] Don't lose compilation directory in Dwarf2 line-tables Cc: "Daniel Jacobowitz" , "GDB Patches" In-Reply-To: <1145368299.14807.889.camel@crx549.cro.st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline References: <1144927446.14807.561.camel@crx549.cro.st.com> <8f2776cb0604131049i69e9b20fv78e60c023f245e56@mail.gmail.com> <1144999942.14807.721.camel@crx549.cro.st.com> <8f2776cb0604140041o63c56c2xa9113d3c4ee259d@mail.gmail.com> <1145002976.14807.744.camel@crx549.cro.st.com> <20060414140449.GA14270@nevyn.them.org> <1145026289.14807.816.camel@crx549.cro.st.com> <1145363529.14807.848.camel@crx549.cro.st.com> <20060418130432.GC10130@nevyn.them.org> <1145368299.14807.889.camel@crx549.cro.st.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-04/txt/msg00305.txt.bz2 On 4/18/06, Frederic RISS wrote: > On Tue, 2006-04-18 at 09:04 -0400, Daniel Jacobowitz wrote: > > On Tue, Apr 18, 2006 at 02:32:09PM +0200, Frederic RISS wrote: > > > All this file matching seems quite fragile, and the current approach > > > will get it wrong sometimes. Shouldn't the loop in dwarf2_start_subfi= le > > > be killed in favor of a search using both xfullpath(dirname'/'filenam= e) > > > at the start of buildsym.c::start_subfile? > > > > The problem with using xfullpath is that it requires the file to exist > > I realized that after having sent the patch. > > > and already be found; as long as we are consistent, it's not too > > fragile, because we're checking against our own output. > > I just made that comment because of the testsuite breakage that ensued > from modifying dirname before the loop... which seemed to imply that the > mechanism was fragile. Let me think aloud a bit. There are two distinct ways we call start_subfile, which expects a directory name and a filename, and stores them as two distinct parts.=20 First, when processing dies from .debug_info, we call start_symtab, passing the CU's DW_AT_name as name and DW_AT_comp_dir as the directory; start_symtab passes these directly through to start_subfile. Second, when reading line number info, we call start_subfile and pass it the filename and the dirname directly from the line number info. These two are inconsistent: in the first case, the subfile's dirname is the value of DW_AT_comp_dir; in the second case, the comp_dir is never recorded at all (in the current code). In each case, the filename is whatever makes up the difference. However, we get away with it because generally the line info dirname + filename matches the CU's DW_AT_name, so concatenating them in dwarf2_start_subfile yields something that matches the 'name' of some subfile. We process the dies first, then the line number info, so all the files the dies mention will already be in the table, so whenever there's something for the line number info to match, we actually hit it in the loop. So we only add subfiles from dwarf2_start_subfile when the loop doesn't find an existing match (good), but the arguments we pass are inconsistent with those we would have passed (probably bad). So, Frederic, your last patch doesn't introduce any new problems that I can see, and it does solve the problem you set out to solve originally (losing comp_dir), but if you're willing go around one more time, we can try to add the info consistently: - Leave the comparison loop alone, as in your last patch. - If dwarf2_start_subfile does have to start a subfile itself, always pass comp_dir as start_subfile's second argument, whether it's NULL or not (because this is what we do when calling start_symtab), and concatenate dirname, if it's non-null, with filename to get start_subfile's first argument. I think this means that 'fullname' always gets used, so you can hoist that computation and its xfree out of the 'if'. Either way, this definitely needs a comment. If you'd like to write up one yourself, great; if not, that's fine; I'll put one in after your patch goes in.