Re: gdb_realpath: dealing with ./ and ../

Joel Brobecker
2008-01-07T14:33:32Z

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

> An alternative would be to do some canonicalization - not
> gdb_realpath, which accesses the filesystem, but just string
> manipulation - on the subfile names iff nothing matches the main
> file. You could remove the ".." there.

Aleksandar liked this option best, but I think I liked your first
suggestion more:

> > Would this not introduce a bit of guessing? Consider a case where there are
> > several files with the same basename but different paths (possibly specified
> > using relative path elements, i.e. different pathnames like in my case). In
> > this case none of the files with the same base name would perfectly fit and
> > picking the first one will likely not give the correct answer.
>
> It's a guess, but a good one. 99.9% of the time, a C file does not
> include another C file with the same basename. If the compiler is
> going to generate debug info which refers to the same file by two
> different names, I don't see what else we can do.

I looked at the whole discussion and at the patches, and they seem
pretty simple in the idea, but pretty complex in the implementation.

If I undertand the problem correctly, the problem is matching our
main.cc entry in the linetable back to the main unit subfile where
the name&comp_unit don't exactly match character for character. Right?

Going further with the idea that 99.9% of the time, one file does not
depend on another file with the same name, then how about doing the
matching using basenames? As a starting point, I propose the following
idea: Modify start_subfile so that:

1. file name is reduced to a basename only
If there is any path information inside name (such as ".." or
"/foo/bar/"), it is appended to the dir name. As a special case,
if name was an absolute path, then we ignore the dirname and
use the dirname from the filename only.

2. subfile matching is done using the basename only.

I am attaching a patch that illustrates roughly what I have in mind
(not compiled, not tested). The great advantage (if it works :),
is that no rewriting is necessary.

The reason why I agree that the changes should be done inside
start-subfiles is that we don't want to have to handle this sort of
problem with every debug info reader. This was, the handling is
centralized. It also looks a lot simpler than the patches I have seen
that touch the DWARF reader.

The one thing that this might break, however, is when the user
provides a relative path in the break command:

(gdb) break bar/main.c

As a matter of fact, this is already broken if DW_AT_name is main.c
and not bar/main.c. So I'm pretty sure that it'll break it more.
But the good news is that it would be easy to fix it: Modify the
matching to concat the dirname and basename and do the match with
that.

Does this sound like it would work?

--
Joel

[-- Attachment #2: buildsym.c.diff --]
[-- Type: text/plain, Size: 3078 bytes --]

Index: buildsym.c
===================================================================
--- buildsym.c (revision 44)
+++ buildsym.c (working copy)
@@ -580,7 +580,7 @@ make_blockvector (struct objfile *objfil
the directory in which the file was compiled (or NULL if not known). */

void
-start_subfile (char *name, char *dirname)
+start_subfile_1 (char *name, char *dirname)
{
struct subfile *subfile;

@@ -589,27 +589,11 @@ start_subfile (char *name, char *dirname

for (subfile = subfiles; subfile; subfile = subfile->next)
{
- char *subfile_name;
-
- /* If NAME is an absolute path, and this subfile is not, then
- attempt to create an absolute path to compare. */
- if (IS_ABSOLUTE_PATH (name)
- && !IS_ABSOLUTE_PATH (subfile->name)
- && subfile->dirname != NULL)
- subfile_name = concat (subfile->dirname, SLASH_STRING,
- subfile->name, NULL);
- else
- subfile_name = subfile->name;
-
- if (FILENAME_CMP (subfile_name, name) == 0)
+ if (FILENAME_CMP (subfile->name, name) == 0)
{
current_subfile = subfile;
- if (subfile_name != subfile->name)
- xfree (subfile_name);
return;
}
- if (subfile_name != subfile->name)
- xfree (subfile_name);
}

/* This subfile is not known. Add an entry for it. Make an entry
@@ -681,6 +665,53 @@ start_subfile (char *name, char *dirname
}
}

+/* This function is a wrapper around start_subfile_1 which rewrites
+ a bit NAME and DIRNAME to make sure that we are always consistent
+ in how we create subfiles: Turn NAME into a basename, and adjust
+ DIRNAME accordingly. */
+
+void
+start_subfile (char *name, char *dirname)
+{
+ char *new_name, new_dir_name;
+
+ new_name = lbasename (name);
+
+ /* The simple usual case: NAME is the name of a file without any path
+ (in other words, it's already a basename). In that case, we use
+ DIR_NAME as is. */
+ if (FILENAME_CMP (new_name, name) == 0)
+ new_dir_name = dir_name;
+
+ /* Another simple case: NAME is an absolute PATH. In that case,
+ we can ignore the compilation dir, and use the path inside NAME
+ as the dir name. */
+ else if (IS_ABSOLUTE_PATH (name))
+ new_dir_name = ldirname (name);
+
+ /* Otherwise, NAME is a relative PATH, so we extract the relative
+ path from name, and append it at the end of DIRNAME. IF DIRNAME
+ is NULL, then we use that relative path as the DIRNAME. */
+ else
+ {
+ char *name_relative_path = ldirname (name);
+
+ if (dirname != NULL)
+ {
+ new_dir_name = concat (dirname, SLASH_STRING, name_relative_path,
+ NULL);
+ xfree (name_relative_path);
+ }
+ else
+ new_dir_name = name_relative_path;
+ }
+
+ start_subfile_1 (new_name, new_dir_name);
+
+ xfree (new_name);
+ xfree (new_dir_name);
+}
+
/* For stabs readers, the first N_SO symbol is assumed to be the
source file name, and the subfile struct is initialized using that
assumption. If another N_SO symbol is later seen, immediately