gdb_realpath: dealing with ./ and ../

[Aleksandar Ristovski <ARistovski at qnx dot com>]

Hello,

First a question, to give an idea what I am talking about and then detailed
explanation.

Question: Should gdb_realpath deal with './' and '../' path elements and
compact them along with 'canonicalization' it already does?

Details:
Our binary was created from one compilation unit:
C:/foo/bar/main.cc
Our compilation directory is:
C:/foo/bar/Debug

When our cross-compiler generates binary, it stores relative path in
.debug_line section (relative to compilation dir), i.e. '..'.

readelf -wl output gives this:
...

  Opcode 6 has 0 args
  Opcode 7 has 0 args
  Opcode 8 has 0 args
  Opcode 9 has 1 args

 The Directory Table:
  ..

 The File Name Table:
  Entry	Dir	Time	Size	Name
  1	1	0	0	main.cc

...

GDB internally gets confused by this: it first creates subfile using
filename:  'C:/foo/bar/main.cc'
But then, when breakpoint is set:
(gdb) b main.cc:11
It loads line table, finds '..', constructs absolute path using compilation
directory and creates "C:/foo/bar/Debug/../main.cc" and then compares this
(using FILENAME_CMP macro) to existing subfile-s and fails to find it (file
buildsym.c, function start_subfile).

It looks like storing relative path in .debug_line is correct (is it?) but
gdb_realpath fails to compact paths containing '..' path elements. 


Question: Should gdb_realpath deal with './' and '../' path elements and
compact them along with 'canonicalization' it already does? Alternatively,
should FILENAME_CMP do more to be smarter about comparing two paths?

Thank you,

Aleksandar Ristovski
QNX Software Systems


Patch that illustrates a solution, providing gdb_realpath indeed needs to
deal with relative path elements.

Index: gdb/utils.c
===================================================================
--- gdb/utils.c	(revision 69)
+++ gdb/utils.c	(working copy)
@@ -2867,6 +2867,109 @@
   return addr;
 }
 
+
+/* Normalize_path does lightweight path clean-up. It removes './' 
+ elements and resolves '../' elements by removing previous entry if any.
+ If FILENAME starts with '../', then '../' does not get removed.  
+
+ Returned value should be freed with xfree.
+
+ Examples:
+ ../main.c   -->    ../main.c
+ ./main.c    -->    main.c
+ /main.c     -->    /main.c
+ /foo/./bar/././main.c  -->   /foo/bar/main.c
+ C:/Temp/Debug/../main.c  -->  C:/Temp/main.c
+  */
+
+char *
+normalize_path (const char *filename)
+{
+  char *p;
+  char *pi;
+  int len;
+# if defined (PATH_MAX)
+  char buf[PATH_MAX];
+#  define USE_REALPATH
+# elif defined (MAXPATHLEN)
+  char buf[MAXPATHLEN];
+# endif
+
+ gdb_assert (filename != NULL);
+
+  strncpy (buf, filename, sizeof (buf));
+  buf[sizeof (buf)] = '\0';
+
+  p = buf;
+
+  while ((pi = strstr (p, "./")))
+    {
+      if (pi == p) /* FILENAME starts with './'. Remove it.  */
+	  p += 2;
+      else
+	break;
+    }
+
+  if (p != buf)
+      strncpy (buf, filename + (p - buf), sizeof (buf));
+ 
+  len = strlen (buf);
+
+  /* Remove all double '//' except the leading occurence.  */
+  p = buf + 1;
+  while (p < buf + len)
+    {
+      if (p[0] == '/' && p[1] == '/')
+	{
+	  memmove (p, p+1, buf + len - p);
+	  len--;
+	}
+      p++;
+    }
+
+  /* Replace all other occurences of '/./' with '/'.  */
+  p = buf;
+  while ((pi = strstr (p, "/./")))
+    {
+      p = pi + 3;
+      memmove (pi, p, buf + len  - p);
+      len -= 3;
+      memset (buf + len, 0, 3);
+    }
+
+  /* Remove trailing '/.'.  */
+  while (buf[len-2] == '/' && buf[len-1] == '.')
+    {
+      len -= 2;
+      buf[len] = '\0';
+      buf[len+1] = '\0';
+    }
+
+  /* Deal with '../' sequences.  */
+  p = buf + 1; /* In an odd case that path begins with '/../' we don't want
+		to know.  */
+  while ((pi = strstr (p, "/..")))
+    {
+      p = pi;
+      /* Reverse find '/'.  */
+      pi = p - 1;
+      while (pi > buf && *pi != '/')
+	pi--;
+
+      if (pi != p)
+	{
+	  p += 3;
+	  memmove (pi, p, buf + len - p);
+	  len -= (p - pi);
+	  memset (buf + len, 0, p - pi);
+	}
+      else
+	p++;
+    }
+
+  return xstrdup (buf);
+}
+
 char *
 gdb_realpath (const char *filename)
 {
@@ -2886,7 +2989,9 @@
 # if defined (USE_REALPATH)
     const char *rp = realpath (filename, buf);
     if (rp == NULL)
-      rp = filename;
+      {
+	return normalize_path (filename);
+      }
     return xstrdup (rp);
 # endif
   }
Index: gdb/buildsym.c
===================================================================
--- gdb/buildsym.c	(revision 69)
+++ gdb/buildsym.c	(working copy)
@@ -548,26 +548,46 @@
   for (subfile = subfiles; subfile; subfile = subfile->next)
     {
       char *subfile_name;
+      char *full_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,
+	{
+	  char *path = concat (subfile->dirname, SLASH_STRING,
 			       subfile->name, NULL);
+	  subfile_name = gdb_realpath (path);
+	  xfree (path);
+	}
       else
 	subfile_name = subfile->name;
 
-      if (FILENAME_CMP (subfile_name, name) == 0)
+      /* If NAME is not an absolute path, try to make it an 
+	 absolute path so we compare apples with apples.  */
+      if (!IS_ABSOLUTE_PATH (name) && dirname != NULL)
+	{
+	  char *path = concat (dirname, SLASH_STRING, name, NULL);
+	  full_name = gdb_realpath (path);
+	  xfree (path);
+	}
+      else
+	full_name = name;
+
+      if (FILENAME_CMP (subfile_name, full_name) == 0)
 	{
 	  current_subfile = subfile;
 	  if (subfile_name != subfile->name)
 	    xfree (subfile_name);
+	  if (full_name != name)
+	    xfree (full_name);
 	  return;
 	}
       if (subfile_name != subfile->name)
 	xfree (subfile_name);
+      if (full_name != name)
+	xfree (full_name);
     }
 
   /* This subfile is not known.  Add an entry for it. Make an entry