RE: gdb_realpath: dealing with ./ and ../

[Aleksandar Ristovski <ARistovski@qnx.com>]

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

> On Thu, Jan 03, 2008 at 01:25:55PM -0500, Aleksandar Ristovski wrote:
> > 1. Problem one - relative NAME and absolute subfile->name case not
> covered:
> 
> I think you're right, but I can't tell for sure.

I am pretty sure this is real problem as this is what I am seeing with our
case (the instrumentation I posted proves it). This is easy and I think
straight forward to fix. (please see the attachement buildsym.c.patch1).

BUT...
This does not solve all my problems. In fact, when debugging on linux a
binary cross-compiled on windows, it won't work:

IS_ABSOLUTE_PATH macro will differ depending on the configured gdb host. The
problem with this is when one is debugging binary cross-compiled on windows,
on a unix host, IS_ABSOLUTE_PATH will return 0 on "C:.." style paths which
is not correct. Looking at the code, in many places we concatenate directory
and file name if IS_ABSOLUTE_PATH returns zero so in case of cross-compiled
binary I suspect there will be many issues.

> 
> > 2. Problem two - one physical file is specified with two pathnames.
> 
> The specific case that's important here is associating the main file
> from a dwarf2 CU DIE with the right lines from .debug_line.  When we
> go to create that subfile we haven't created the other subfiles yet.
> So what we need, I think, is to handle this in dwarf2read.c by walking
> through the filename / directory table.

dwarf2read.c calls start_subfile or start_symtab which in turn calls
start_subfile. I think we should let our readers deal with "compiled" paths
(i.e. as found in the binary), without conversion. In start_subfile,
however, we can do something with paths. Currently, we do not try to
substitute-path on paths coming from the binary, even though they are
clearly our source paths and should be dealt with the same as with any other
source path. 

My attempt would be patch2 attached (start_subfile.patch). Please take a
look. This should solve both problems.

Note: IS_ABSOLUTE_PATH issue will still exist since we deal with compiled
paths throughout gdb code. Wherever IS_ABSOLUTE_PATH is used on a windows
style path (and gdb was configured for *ix) it will return 0 leading to
problems.

Note2: for some reason, my post to gdb-patches didn't go through even after
two attempts. There I submitted only source.c change dealing with rewriting
source path in case when NAME is an absolute path and at the same time
DIRNAME is not NULL (which can happen).
 

Aleksandar Ristovski
QNX Software Systems


[-- Attachment #2: buildsym.c.patch1 --]
[-- Type: application/octet-stream, Size: 1745 bytes --]

Index: gdb/buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.59
diff -u -3 -p -r1.59 buildsym.c
--- gdb/buildsym.c	1 Jan 2008 22:53:09 -0000	1.59
+++ gdb/buildsym.c	4 Jan 2008 14:41:44 -0000
@@ -583,6 +583,13 @@ void
 start_subfile (char *name, char *dirname)
 {
   struct subfile *subfile;
+  char *absname; /* Absolute NAME.  */
+
+  if (!IS_ABSOLUTE_PATH (name) 
+      && dirname != NULL)
+    absname = concat (dirname, SLASH_STRING, name, NULL);
+  else
+    absname = name;
 
   /* See if this subfile is already known as a subfile of the current
      main source file.  */
@@ -590,6 +597,7 @@ start_subfile (char *name, char *dirname
   for (subfile = subfiles; subfile; subfile = subfile->next)
     {
       char *subfile_name;
+      char *aname;
 
       /* If NAME is an absolute path, and this subfile is not, then
 	 attempt to create an absolute path to compare.  */
@@ -601,7 +609,13 @@ start_subfile (char *name, char *dirname
       else
 	subfile_name = subfile->name;
 
-      if (FILENAME_CMP (subfile_name, name) == 0)
+      if (!IS_ABSOLUTE_PATH (name)
+	  && IS_ABSOLUTE_PATH (subfile->name))
+	aname = absname;
+      else
+	aname = name;
+
+      if (FILENAME_CMP (subfile_name, aname) == 0)
 	{
 	  current_subfile = subfile;
 	  if (subfile_name != subfile->name)
@@ -612,6 +626,9 @@ start_subfile (char *name, char *dirname
 	xfree (subfile_name);
     }
 
+  if (absname != name)
+    xfree (absname);
+
   /* This subfile is not known.  Add an entry for it. Make an entry
      for this subfile in the list of all subfiles of the current main
      source file.  */

[-- Attachment #3: start_subfile.patch --]
[-- Type: application/octet-stream, Size: 5970 bytes --]

Index: gdb/buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.59
diff -u -3 -p -r1.59 buildsym.c
--- gdb/buildsym.c	1 Jan 2008 22:53:09 -0000	1.59
+++ gdb/buildsym.c	4 Jan 2008 16:28:40 -0000
@@ -44,6 +44,7 @@
 #include "cp-support.h"
 #include "dictionary.h"
 #include "addrmap.h"
+#include "source.h"
 
 /* Ask buildsym.h to define the vars it normally declares `extern'.  */
 #define	EXTERN
@@ -574,15 +575,57 @@ make_blockvector (struct objfile *objfil
   return (blockvector);
 }
 \f
+
+/* Workaround for IS_ABSOLUTE_PATH issue. When gdb is configured for
+   *ix, and we are debugging binary cross-compiled on windows, the original
+   IS_ABSOLUTE_PATH will always return 0. This is not good.  */
+#undef IS_ABSOLUTE_PATH
+#undef IS_DIR_SEPARATOR
+
+#define IS_DIR_SEPARATOR(c)	((c) == '/' || (c) == '\\')
+/* Note that IS_ABSOLUTE_PATH accepts d:foo as well, although it is
+   only semi-absolute.  This is because the users of IS_ABSOLUTE_PATH
+   want to know whether to prepend the current working directory to
+   a file name, which should not be done with a name like d:foo.  */
+#define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]) || (((f)[0]) && ((f)[1] == ':')))
+
 /* Start recording information about source code that came from an
    included (or otherwise merged-in) source file with a different
    name.  NAME is the name of the file (cannot be NULL), DIRNAME is
    the directory in which the file was compiled (or NULL if not known).  */
 
 void
-start_subfile (char *name, char *dirname)
+start_subfile (char *name, char *dirname) 
 {
   struct subfile *subfile;
+  char *absname; /* Rewritten absolute NAME.  */
+
+  if (IS_ABSOLUTE_PATH (name))
+    {
+      char *rwname;
+      rwname = rewrite_source_path (name);
+      if (rwname != NULL)
+	{
+	  make_cleanup (xfree, rwname);
+	  name = rwname;
+	}
+      absname = name;
+    }
+  else
+    {
+      if (dirname != NULL)
+	{
+	  char *rwdirname = rewrite_source_path (dirname);
+	  if (rwdirname == NULL)
+	    rwdirname = dirname;
+	  absname = concat (rwdirname, SLASH_STRING, name, NULL);
+	  if (rwdirname != dirname)
+	    xfree (rwdirname);
+	  make_cleanup (xfree, absname);
+	}
+      else
+	absname = name;
+    }
 
   /* See if this subfile is already known as a subfile of the current
      main source file.  */
@@ -590,18 +633,36 @@ start_subfile (char *name, char *dirname
   for (subfile = subfiles; subfile; subfile = subfile->next)
     {
       char *subfile_name;
+      char *aname;
 
       /* 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 *rwsubfile_dirname = rewrite_source_path (subfile->dirname);
+	  if (rwsubfile_dirname == NULL)
+	    rwsubfile_dirname = subfile->dirname;
+	  subfile_name = concat (rwsubfile_dirname, SLASH_STRING,
 			       subfile->name, NULL);
+	  if (rwsubfile_dirname != subfile->dirname)
+	    xfree (rwsubfile_dirname);
+	}
+      else
+	{
+	  subfile_name = rewrite_source_path (subfile->name);
+	  if (subfile_name == NULL)
+	    subfile_name = subfile->name;
+	}
+
+      if (!IS_ABSOLUTE_PATH (name)
+	  && IS_ABSOLUTE_PATH (subfile->name))
+	aname = absname;
       else
-	subfile_name = subfile->name;
+	aname = name;
 
-      if (FILENAME_CMP (subfile_name, name) == 0)
+      if (FILENAME_CMP (subfile_name, aname) == 0)
 	{
 	  current_subfile = subfile;
 	  if (subfile_name != subfile->name)
Index: gdb/source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.83
diff -u -3 -p -r1.83 source.c
--- gdb/source.c	1 Jan 2008 22:53:13 -0000	1.83
+++ gdb/source.c	4 Jan 2008 16:28:40 -0000
@@ -895,10 +895,12 @@ get_substitute_path_rule (const char *pa
    Return NULL if no substitution rule was specified by the user,
    or if no rule applied to the given PATH.  */
    
-static char *
+char *
 rewrite_source_path (const char *path)
 {
-  const struct substitute_path_rule *rule = get_substitute_path_rule (path);
+  const struct substitute_path_rule *rule = (path != NULL) ? 
+					get_substitute_path_rule (path):
+					NULL;
   char *new_path;
   int from_len;
   
@@ -999,10 +1001,11 @@ find_and_open_source (struct objfile *ob
 	  strcat (path + len, source_path + len + cdir_len);	/* After $cdir */
 	}
     }
-  else
+  
+  if (IS_ABSOLUTE_PATH (filename))
     {
-      /* If dirname is NULL, chances are the path is embedded in
-         the filename.  Try the source path substitution on it.  */
+      /* If filename is absolute path, try the source path 
+	 substitution on it.  */
       char *rewritten_filename = rewrite_source_path (filename);
 
       if (rewritten_filename != NULL)
Index: gdb/source.h
===================================================================
RCS file: /cvs/src/src/gdb/source.h,v
retrieving revision 1.9
diff -u -3 -p -r1.9 source.h
--- gdb/source.h	1 Jan 2008 22:53:13 -0000	1.9
+++ gdb/source.h	4 Jan 2008 16:28:40 -0000
@@ -66,4 +66,14 @@ extern struct symtab_and_line set_curren
 
 /* Reset any information stored about a default file and line to print. */
 extern void clear_current_source_symtab_and_line (void);
+
+/* If the user specified a source path substitution rule that applies
+   to PATH, then apply it and return the new path.  This new path must
+   be deallocated afterwards.  
+   
+   Return NULL if no substitution rule was specified by the user,
+   or if no rule applied to the given PATH.  */
+   
+extern char *rewrite_source_path (const char *path);
+
 #endif