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

[Joel Brobecker <brobecker@adacore.com>]

[-- 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