Re: [RFC] Don't lose compilation directory in Dwarf2 line-tables

["Jim Blandy" <jimb at red-bean dot 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.