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

[Frederic RISS <frederic dot riss at st dot com>]

Him Jim, 

Thanks for your comments.

On Thu, 2006-04-13 at 10:49 -0700, Jim Blandy wrote:
> Hi, Frederic.  Thanks for the patch!  I think you're right about how
> GDB should interpret DW_AT_comp_dir and directories in the line number
> information.  Some comments:

Great.

> You should generally include a ChangeLog entry along with the patch;
> you'll have to write it anyway, and it helps reviewers get oriented
> for what they're going to see in the patch itself.

Will do for the next round.

> Ideally, the use of SLASH_STRING would be a separate patch, but that's
> not a big deal.

Not a big deal for me to do a separate patch.

> When you add a new argument to dwarf2_start_subfile, you need to
> adjust the comments above the file that describe what the arguments
> mean.

Yep

> The patch changes the logic of dwarf2_start_subfile in a circuitous
> way.  If dirname and comp_dir are absolute and different, and filename
> is relative, then you end up passing start_subfile two absolute paths.
>  I'd rather see the code changed like this:

I agree that the way it was done made dwarf2_start_subfile quite strange
to read.

> - Since dwarf2_start_subfile now knows about comp_dir, there's no need
> to use comp_dir as a default in dwarf_decode_lines.  Just pass NULL
> for dirname when the dir_index is zero.

That's a good idea, it makes things simpler. I've a cleaned up patch for
that, but I'm not sure sure about your next proposal:

> - Then, at the top of dwarf2_start_subfile, check if dirname is
> relative, and if comp_dir is available, prepend comp_dir to it.  At
> this point, we know dirname is as absolute as it can be.  Then proceed
> as in the original unpatched code.  (Watch out for allocation issues.)

I don't think we should concat comp_dir and dirname. In my mind,
comp_dir makes only sense as an 'independant' information. Or the other
way around: dirname gives you information about your source tree
structure, and you lose it by prepending comp_dir to it. Have you an
objection to storing "dirname/filename" as filename and comp_dir as
directory? Maybe this makes only sense when dirname is also relative
like in the snippet that Jason posted. Once we agree on this part, I'll
post an updated patch.

> Also, in the comments, "loose" is the opposite of "tight"; you mean "lose".

Doh... back to school :-)

Regards,
Fred