From: "Jim Blandy" <jimb@red-bean.com>
To: "Frederic RISS" <frederic.riss@st.com>
Cc: "Daniel Jacobowitz" <drow@false.org>,
"GDB Patches" <gdb-patches@sourceware.org>
Subject: Re: [RFC] Don't lose compilation directory in Dwarf2 line-tables
Date: Fri, 21 Apr 2006 01:10:00 -0000 [thread overview]
Message-ID: <8f2776cb0604201809l68067aey47fa372a2468c8ea@mail.gmail.com> (raw)
In-Reply-To: <1145368299.14807.889.camel@crx549.cro.st.com>
On 4/18/06, Frederic RISS <frederic.riss@st.com> 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_subfile
> > > be killed in favor of a search using both xfullpath(dirname'/'filename)
> > > 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.
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.
next prev parent reply other threads:[~2006-04-21 1:10 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-04-13 11:25 [RFC] Don't loose " Frederic RISS
2006-04-13 17:49 ` Jim Blandy
2006-04-14 7:32 ` [RFC] Don't lose " Frederic RISS
2006-04-14 7:41 ` Jim Blandy
2006-04-14 8:23 ` Frederic RISS
2006-04-14 14:04 ` Daniel Jacobowitz
2006-04-14 14:51 ` Frederic RISS
2006-04-18 12:32 ` Frederic RISS
2006-04-18 13:04 ` Daniel Jacobowitz
2006-04-18 14:00 ` Frederic RISS
2006-04-20 16:27 ` Daniel Jacobowitz
2006-04-21 7:14 ` Frederic RISS
2006-04-21 1:10 ` Jim Blandy [this message]
2006-04-21 8:35 ` Frederic RISS
2006-04-21 16:46 ` Jim Blandy
2006-04-21 17:00 ` Frederic RISS
2006-04-24 12:49 ` Andrew STUBBS
2006-04-14 8:44 ` Eli Zaretskii
2006-04-14 11:44 ` Frederic RISS
2006-04-14 13:08 ` Eli Zaretskii
2006-04-14 13:42 ` Frederic RISS
2006-04-14 14:21 ` Eli Zaretskii
2006-04-14 14:58 ` Daniel Jacobowitz
2006-04-14 15:03 ` Eli Zaretskii
2006-04-14 18:39 ` Frédéric Riss
2006-04-13 19:58 ` [RFC] Don't loose " Jason Molenda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8f2776cb0604201809l68067aey47fa372a2468c8ea@mail.gmail.com \
--to=jimb@red-bean.com \
--cc=drow@false.org \
--cc=frederic.riss@st.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox