[RFC] Record objfile->original_name as an absolute path

[RFC] Record objfile->original_name as an absolute path

[Doug Evans]

Hi.

The comment for objfile->original_name says the path is stored as an
absolute path, but that's not the case, and that got me thinking:
What if the user cd's before the debug info is accessed?

This patch is only RFC, and not RFA, because I'm not sure how to do
something better in the test.  I can certainly clean up what's there,
if people think it's not *too* cheezy.

[Later, if we ever choose to display objfile->original_name in
some context, and want to clean up the name a bit (/tmp/./foo -> /tmp/foo),
I can imagine making a function out of the canonicalizer in
cli-cmds.c:cd_command and using that.
[We wouldn't want to remove "../" for this case though unless we go to
the extra trouble of not expanding symlinks.]]

2013-09-25  Doug Evans  <dje@google.com>

	* objfiles.c (allocate_objfile): Save original_name as an absolute
	path.
	* objfiles.c (struct objfile): Expand comment on original_name.
	* utils.c (gdb_abspath): New function.
	* utils.h (gdb_abspath): Declare.

	testsuite/
	* gdb.dwarf/dwp-symlink.c: Fake out gdb to not load debug info
	at start.
	* gdb.dwarf/dwp-symlink.exp: Test trying to load dwp when the binary
	has been specified with a relative path and we have chdir'd before
	accessing the debug info.

Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.168
diff -u -p -r1.168 objfiles.c
--- objfiles.c	26 Sep 2013 01:08:35 -0000	1.168
+++ objfiles.c	26 Sep 2013 01:11:55 -0000
@@ -270,6 +270,7 @@ struct objfile *
 allocate_objfile (bfd *abfd, const char *name, int flags)
 {
   struct objfile *objfile;
+  char *expanded_name;
 
   objfile = (struct objfile *) xzalloc (sizeof (struct objfile));
   objfile->psymbol_cache = psymbol_bcache_init ();
@@ -283,10 +284,18 @@ allocate_objfile (bfd *abfd, const char 
   if (name == NULL)
     {
       gdb_assert (abfd == NULL);
-      name = "<<anonymous objfile>>";
+      expanded_name = xstrdup ("<<anonymous objfile>>");
     }
-  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack, name,
-					  strlen (name));
+  else
+    expanded_name = gdb_abspath (name);
+  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack,
+					  expanded_name,
+					  strlen (expanded_name));
+  xfree (expanded_name);
+
+  /* Update the per-objfile information that comes from the bfd, ensuring
+     that any data that is reference is saved in the per-objfile data
+     region.  */
 
   /* Update the per-objfile information that comes from the bfd, ensuring
      that any data that is reference is saved in the per-objfile data
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.112
diff -u -p -r1.112 objfiles.h
--- objfiles.h	24 Sep 2013 14:00:06 -0000	1.112
+++ objfiles.h	26 Sep 2013 01:11:55 -0000
@@ -205,8 +205,10 @@ struct objfile
 
     struct objfile *next;
 
-    /* The object file's name, tilde-expanded and absolute.  This
-       pointer is never NULL.  This does not have to be freed; it is
+    /* The object file's original name as specified by the user,
+       made absolute, and tilde-expanded.  However, it is not canonicalized
+       (i.e., it has not been passed through gdb_realpath).
+       This pointer is never NULL.  This does not have to be freed; it is
        guaranteed to have a lifetime at least as long as the objfile.  */
 
     char *original_name;
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.306
diff -u -p -r1.306 utils.c
--- utils.c	13 Aug 2013 08:31:20 -0000	1.306
+++ utils.c	26 Sep 2013 01:11:55 -0000
@@ -3204,6 +3204,38 @@ gdb_realpath (const char *filename)
   return xstrdup (filename);
 }
 
+/* Return PATH in absolute form, performing tilde-expansion if necessary.
+   PATH cannot be NULL or the empty string.
+   This does not resolve symlinks however, use gdb_realpath for that.
+   Space for the result is allocated with malloc.
+   If the path is already absolute, it is strdup'd.
+   If there is a problem computing the absolute path, the path is returned
+   unchanged (still strdup'd).  */
+
+char *
+gdb_abspath (const char *path)
+{
+#if defined (PATH_MAX)
+  char cwd_buf[PATH_MAX];
+#elif defined (_WIN32)
+  char cwd_buf[MAX_PATH];
+#else
+  char cwd_buf[1024];
+#endif
+
+  gdb_assert (path != NULL && path[0] != '\0');
+
+  if (path[0] == '~')
+    return tilde_expand (path);
+
+  if (IS_ABSOLUTE_PATH (path))
+    return xstrdup (path);
+
+  if (getcwd (cwd_buf, sizeof (cwd_buf)) == NULL)
+    return xstrdup (path);
+  return concat (cwd_buf, SLASH_STRING, path, NULL);
+}
+
 ULONGEST
 align_up (ULONGEST v, int n)
 {
Index: utils.h
===================================================================
RCS file: /cvs/src/src/gdb/utils.h,v
retrieving revision 1.11
diff -u -p -r1.11 utils.h
--- utils.h	1 Aug 2013 09:09:58 -0000	1.11
+++ utils.h	26 Sep 2013 01:11:55 -0000
@@ -128,6 +128,8 @@ extern struct cleanup *make_bpstat_clear
 
 extern char *gdb_realpath (const char *);
 
+extern char *gdb_abspath (const char *);
+
 extern int gdb_filename_fnmatch (const char *pattern, const char *string,
 				 int flags);
 
Index: testsuite/gdb.dwarf2/dwp-symlink.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.dwarf2/dwp-symlink.c,v
retrieving revision 1.1
diff -u -p -r1.1 dwp-symlink.c
--- testsuite/gdb.dwarf2/dwp-symlink.c	24 Sep 2013 14:03:43 -0000	1.1
+++ testsuite/gdb.dwarf2/dwp-symlink.c	26 Sep 2013 01:11:55 -0000
@@ -15,6 +15,13 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+/* Cheezy hack to prevent set_initial_language from trying to look up main.
+   We do this so that gdb won't try to open the dwp file when the file is
+   first selected.  This gives us a chance to do a chdir before attempting
+   to access the debug info.  */
+asm (".globl main.main");
+asm ("main.main: .byte 0");
+
 int
 main (int argc, char **argv)
 {
Index: testsuite/gdb.dwarf2/dwp-symlink.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.dwarf2/dwp-symlink.exp,v
retrieving revision 1.1
diff -u -p -r1.1 dwp-symlink.exp
--- testsuite/gdb.dwarf2/dwp-symlink.exp	24 Sep 2013 14:03:43 -0000	1.1
+++ testsuite/gdb.dwarf2/dwp-symlink.exp	26 Sep 2013 01:11:55 -0000
@@ -75,3 +75,23 @@ gdb_test "ptype main" {type = int \(\)} 
 clean_restart "$thelink"
 
 gdb_test "ptype main" {type = int \(int, char \*\*\)} "binary symlink, dwp at symlink"
+
+# Verify we can still find the dwp if we change directories and we specified
+# a relative path for the program.
+
+set saved_pwd [pwd]
+
+# This is clean_restart, but specifying a relative path to the binary.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_test "cd [file dirname [standard_output_file ${thelink}]]" \
+    "Working directory .*"
+gdb_load "./${thelink}"
+
+gdb_test "cd .." "Working directory .*"
+
+gdb_test "ptype main" {type = int \(int, char \*\*\)} \
+    "relative path, binary symlink, dwp at symlink"
+
+cd $saved_pwd

Re: [RFC] Record objfile->original_name as an absolute path

[Doug Evans]

On Wed, Sep 25, 2013 at 6:29 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> The comment for objfile->original_name says the path is stored as an
> absolute path, but that's not the case, and that got me thinking:
> What if the user cd's before the debug info is accessed?
>
> This patch is only RFC, and not RFA, because I'm not sure how to do
> something better in the test.  I can certainly clean up what's there,
> if people think it's not *too* cheezy.
>
> [...]
>
> 2013-09-25  Doug Evans  <dje@google.com>
>
>         * objfiles.c (allocate_objfile): Save original_name as an absolute
>         path.
>         * objfiles.c (struct objfile): Expand comment on original_name.
>         * utils.c (gdb_abspath): New function.
>         * utils.h (gdb_abspath): Declare.
>
>         testsuite/
>         * gdb.dwarf/dwp-symlink.c: Fake out gdb to not load debug info
>         at start.
>         * gdb.dwarf/dwp-symlink.exp: Test trying to load dwp when the binary
>         has been specified with a relative path and we have chdir'd before
>         accessing the debug info.

One thought for doing the test differently is to have an option to the
"file" command that is the opposite of -readnow, that tells gdb to not
read any debug info (even to set the initial language).

Re: [RFC] Record objfile->original_name as an absolute path

[Jan Kratochvil]

On Thu, 26 Sep 2013 03:53:58 +0200, Doug Evans wrote:
> One thought for doing the test differently is to have an option to the
> "file" command that is the opposite of -readnow, that tells gdb to not
> read any debug info (even to set the initial language).

Fedora still carries
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-6.3-readnever-20050907.patch

although considering to just drop it, IIUC it was used in the past because
loading symbols was unusably slow in some cases but it is no longer needed
thanks to gdbindex.

But occasionally it still may be handy for example for benchmarking.

Maybe "maintenance set load-symbols off" would cover all the needs?
It should never be needed by regular users.


Jan

Re: [RFC] Record objfile->original_name as an absolute path

[Jan Kratochvil]

Hi Doug,

On Thu, 26 Sep 2013 03:29:16 +0200, Doug Evans wrote:
> The comment for objfile->original_name says the path is stored as an
> absolute path, but that's not the case,

oops, sorry.


> [Later, if we ever choose to display objfile->original_name in
> some context, and want to clean up the name a bit (/tmp/./foo -> /tmp/foo),

There is pending
	[patchv2] Improve Executable displayed path (PR 15415 regression kind #2)
	https://sourceware.org/ml/gdb-patches/2013-09/msg00865.html

to fix another filename regression of gdb-7.5 -> gdb-7.6 by me.

Then GDB will display really /tmp/foo for the executable name although that is
unrelated to objfile->original_name.  Therefore the "./" resolving does not
seem to be (currently) an issue I think.


> I can imagine making a function out of the canonicalizer in
> cli-cmds.c:cd_command and using that.
> [We wouldn't want to remove "../" for this case though unless we go to
> the extra trouble of not expanding symlinks.]]
> 
> 2013-09-25  Doug Evans  <dje@google.com>
> 
> 	* objfiles.c (allocate_objfile): Save original_name as an absolute
> 	path.
> 	* objfiles.c (struct objfile): Expand comment on original_name.
> 	* utils.c (gdb_abspath): New function.
> 	* utils.h (gdb_abspath): Declare.
> 
> 	testsuite/
> 	* gdb.dwarf/dwp-symlink.c: Fake out gdb to not load debug info
> 	at start.
> 	* gdb.dwarf/dwp-symlink.exp: Test trying to load dwp when the binary
> 	has been specified with a relative path and we have chdir'd before
> 	accessing the debug info.
> 
> Index: objfiles.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/objfiles.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 objfiles.c
> --- objfiles.c	26 Sep 2013 01:08:35 -0000	1.168
> +++ objfiles.c	26 Sep 2013 01:11:55 -0000
> @@ -270,6 +270,7 @@ struct objfile *
>  allocate_objfile (bfd *abfd, const char *name, int flags)
>  {
>    struct objfile *objfile;
> +  char *expanded_name;
>  
>    objfile = (struct objfile *) xzalloc (sizeof (struct objfile));
>    objfile->psymbol_cache = psymbol_bcache_init ();
> @@ -283,10 +284,18 @@ allocate_objfile (bfd *abfd, const char 
>    if (name == NULL)
>      {
>        gdb_assert (abfd == NULL);
> -      name = "<<anonymous objfile>>";
> +      expanded_name = xstrdup ("<<anonymous objfile>>");
>      }
> -  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack, name,
> -					  strlen (name));
> +  else
> +    expanded_name = gdb_abspath (name);

Maybe that gdb_abspath should be rather done by the caller as there is also
	objfile = allocate_objfile (NULL, "<< JIT compiled code >>", 0);

and calling gdb_abspath ("<< JIT compiled code >>") does not seem great.

OTOH we could call gdb_abspath() only if abfd != NULL but so far nobody
guarantees if abfd == NULL then NAME is not a valid filename.


> +  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack,
> +					  expanded_name,
> +					  strlen (expanded_name));
> +  xfree (expanded_name);
> +
> +  /* Update the per-objfile information that comes from the bfd, ensuring
> +     that any data that is reference is saved in the per-objfile data
> +     region.  */
>  
>    /* Update the per-objfile information that comes from the bfd, ensuring
>       that any data that is reference is saved in the per-objfile data
> Index: objfiles.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/objfiles.h,v
> retrieving revision 1.112
> diff -u -p -r1.112 objfiles.h
> --- objfiles.h	24 Sep 2013 14:00:06 -0000	1.112
> +++ objfiles.h	26 Sep 2013 01:11:55 -0000
> @@ -205,8 +205,10 @@ struct objfile
>  
>      struct objfile *next;
>  
> -    /* The object file's name, tilde-expanded and absolute.  This
> -       pointer is never NULL.  This does not have to be freed; it is
> +    /* The object file's original name as specified by the user,
> +       made absolute, and tilde-expanded.  However, it is not canonicalized
> +       (i.e., it has not been passed through gdb_realpath).
> +       This pointer is never NULL.  This does not have to be freed; it is
>         guaranteed to have a lifetime at least as long as the objfile.  */
>  
>      char *original_name;
> Index: utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/utils.c,v
> retrieving revision 1.306
> diff -u -p -r1.306 utils.c
> --- utils.c	13 Aug 2013 08:31:20 -0000	1.306
> +++ utils.c	26 Sep 2013 01:11:55 -0000
> @@ -3204,6 +3204,38 @@ gdb_realpath (const char *filename)
>    return xstrdup (filename);
>  }
>  
> +/* Return PATH in absolute form, performing tilde-expansion if necessary.
> +   PATH cannot be NULL or the empty string.
> +   This does not resolve symlinks however, use gdb_realpath for that.
> +   Space for the result is allocated with malloc.
> +   If the path is already absolute, it is strdup'd.
> +   If there is a problem computing the absolute path, the path is returned
> +   unchanged (still strdup'd).  */
> +
> +char *
> +gdb_abspath (const char *path)
> +{
> +#if defined (PATH_MAX)
> +  char cwd_buf[PATH_MAX];
> +#elif defined (_WIN32)
> +  char cwd_buf[MAX_PATH];
> +#else
> +  char cwd_buf[1024];
> +#endif
> +
> +  gdb_assert (path != NULL && path[0] != '\0');
> +
> +  if (path[0] == '~')
> +    return tilde_expand (path);
> +
> +  if (IS_ABSOLUTE_PATH (path))
> +    return xstrdup (path);
> +
> +  if (getcwd (cwd_buf, sizeof (cwd_buf)) == NULL)
> +    return xstrdup (path);
> +  return concat (cwd_buf, SLASH_STRING, path, NULL);

It should be integrated with the tail of openp() where is a check for:
	/* Beware the // my son, the Emacs barfs, the botch that catch...  */
It also uses global 'current_directory' variable.


> +}
> +
>  ULONGEST
>  align_up (ULONGEST v, int n)
>  {
[...]


Thanks,
Jan

Re: [RFC] Record objfile->original_name as an absolute path

[Tom Tromey]

Jan> Maybe that gdb_abspath should be rather done by the caller as there is also
Jan> 	objfile = allocate_objfile (NULL, "<< JIT compiled code >>", 0);
Jan> and calling gdb_abspath ("<< JIT compiled code >>") does not seem great.

I've wanted an objfile flag for a while that would indicate whether the
underlying BFD corresponds to a file or some other thing.
Right now I think you can see lookups of bogusly-named files coming from
the Python auto-loader.

Tom

[patch] OBJF_NOT_FILENAME [Re: [RFC] Record objfile->original_name as an absolute path]

[Jan Kratochvil]

On Fri, 27 Sep 2013 21:37:35 +0200, Tom Tromey wrote:
> I've wanted an objfile flag for a while that would indicate whether the
> underlying BFD corresponds to a file or some other thing.
> Right now I think you can see lookups of bogusly-named files coming from
> the Python auto-loader.

That's true, for example for gdb.base/jit.exp:

open("<in-memory>-gdb.gdb", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/usr/lib/debug<in-memory>-gdb.gdb", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/usr/share/gdb/auto-load<in-memory>-gdb.gdb", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
etc.

I will integrate it with add-ons to the Doug's patch.

No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu.


Thanks,
Jan


gdb/
2013-10-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	New flag OBJF_NOT_FILENAME.
	* auto-load.c (auto_load_objfile_script): Check also OBJF_NOT_FILENAME.
	* jit.c (jit_object_close_impl): Use OBJF_NOT_FILENAME for
	allocate_objfile.
	(jit_bfd_try_read_symtab): Use OBJF_NOT_FILENAME for
	symbol_file_add_from_bfd.
	* jv-lang.c (get_dynamics_objfile): Use OBJF_NOT_FILENAME for
	allocate_objfile.
	* objfiles.c (allocate_objfile): Assert OBJF_NOT_FILENAME if NAME is
	NULL.
	* objfiles.h (OBJF_NOT_FILENAME): New.

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 6d0d6d9..4eb7cdd 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -840,7 +840,7 @@ auto_load_objfile_script (struct objfile *objfile,
 void
 load_auto_scripts_for_objfile (struct objfile *objfile)
 {
-  if (!global_auto_load)
+  if (!global_auto_load || (objfile->flags & OBJF_NOT_FILENAME) != 0)
     return;
 
   if (auto_load_gdb_scripts)
diff --git a/gdb/jit.c b/gdb/jit.c
index c7edd4a..ba0be5e 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -785,7 +785,8 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
 
   priv_data = cb->priv_data;
 
-  objfile = allocate_objfile (NULL, "<< JIT compiled code >>", 0);
+  objfile = allocate_objfile (NULL, "<< JIT compiled code >>",
+			      OBJF_NOT_FILENAME);
   objfile->per_bfd->gdbarch = target_gdbarch ();
 
   terminate_minimal_symbol_table (objfile);
@@ -926,7 +927,7 @@ JITed symbol file is not an object file, ignoring it.\n"));
   /* This call does not take ownership of SAI.  */
   make_cleanup_bfd_unref (nbfd);
   objfile = symbol_file_add_from_bfd (nbfd, bfd_get_filename (nbfd), 0, sai,
-				      OBJF_SHARED, NULL);
+				      OBJF_SHARED | OBJF_NOT_FILENAME, NULL);
 
   do_cleanups (old_cleanups);
   add_objfile_entry (objfile, entry_addr);
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index 0d07bbd..63bcc98 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -118,7 +118,8 @@ get_dynamics_objfile (struct gdbarch *gdbarch)
 
       /* Mark it as shared so that it is cleared when the inferior is
 	 re-run.  */
-      dynamics_objfile = allocate_objfile (NULL, NULL, OBJF_SHARED);
+      dynamics_objfile = allocate_objfile (NULL, NULL,
+					   OBJF_SHARED | OBJF_NOT_FILENAME);
       dynamics_objfile->per_bfd->gdbarch = gdbarch;
 
       data = XCNEW (struct jv_per_objfile_data);
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index a10540a..d1f3121 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -289,6 +289,7 @@ allocate_objfile (bfd *abfd, const char *name, int flags)
   if (name == NULL)
     {
       gdb_assert (abfd == NULL);
+      gdb_assert ((flags & OBJF_NOT_FILENAME) != 0);
       name = "<<anonymous objfile>>";
     }
   objfile->original_name = obstack_copy0 (&objfile->objfile_obstack, name,
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 8586e5a..281a698 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -429,6 +429,11 @@ struct objfile
 
 #define OBJF_MAINLINE (1 << 5)
 
+/* ORIGINAL_NAME and OBFD->FILENAME correspong to text description unrelated to
+   filesystem names.  It can be for example "<image in memory>".  */
+
+#define OBJF_NOT_FILENAME (1 << 6)
+
 /* Declarations for functions defined in objfiles.c */
 
 extern struct objfile *allocate_objfile (bfd *, const char *name, int);

Re: [patch] OBJF_NOT_FILENAME [Re: [RFC] Record objfile->original_name as an absolute path]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I will integrate it with add-ons to the Doug's patch.

Thanks, Jan.
It looks good.  Just one nit:

Jan> +/* ORIGINAL_NAME and OBFD->FILENAME correspong to text description unrelated to

Typo, "correspond".

Tom

[commit] [patch] OBJF_NOT_FILENAME [Re: [RFC] Record objfile->original_name as an absolute path]

[Jan Kratochvil]

On Tue, 08 Oct 2013 22:18:00 +0200, Tom Tromey wrote:
> Jan> I will integrate it with add-ons to the Doug's patch.
> 
> Thanks, Jan.
> It looks good.  Just one nit:

Checked in, Doug's patch conflicts on top of it but I guess I will be the one
updating the Doug's patch anyway.


> Jan> +/* ORIGINAL_NAME and OBFD->FILENAME correspong to text description unrelated to
> 
> Typo, "correspond".

Fixed.


Thanks,
Jan


https://sourceware.org/ml/gdb-cvs/2013-10/msg00054.html

--- src/gdb/ChangeLog	2013/10/08 19:56:14	1.16084
+++ src/gdb/ChangeLog	2013/10/09 13:22:35	1.16085
@@ -1,3 +1,17 @@
+2013-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	New flag OBJF_NOT_FILENAME.
+	* auto-load.c (auto_load_objfile_script): Check also OBJF_NOT_FILENAME.
+	* jit.c (jit_object_close_impl): Use OBJF_NOT_FILENAME for
+	allocate_objfile.
+	(jit_bfd_try_read_symtab): Use OBJF_NOT_FILENAME for
+	symbol_file_add_from_bfd.
+	* jv-lang.c (get_dynamics_objfile): Use OBJF_NOT_FILENAME for
+	allocate_objfile.
+	* objfiles.c (allocate_objfile): Assert OBJF_NOT_FILENAME if NAME is
+	NULL.
+	* objfiles.h (OBJF_NOT_FILENAME): New.
+
 2013-10-08  Tom Tromey  <tromey@redhat.com>
 
 	* Makefile.in (SFILES): Add build-id.c.
--- src/gdb/auto-load.c	2013/09/24 13:57:36	1.22
+++ src/gdb/auto-load.c	2013/10/09 13:22:35	1.23
@@ -840,7 +840,7 @@
 void
 load_auto_scripts_for_objfile (struct objfile *objfile)
 {
-  if (!global_auto_load)
+  if (!global_auto_load || (objfile->flags & OBJF_NOT_FILENAME) != 0)
     return;
 
   if (auto_load_gdb_scripts)
--- src/gdb/jit.c	2013/09/24 14:00:06	1.56
+++ src/gdb/jit.c	2013/10/09 13:22:36	1.57
@@ -785,7 +785,8 @@
 
   priv_data = cb->priv_data;
 
-  objfile = allocate_objfile (NULL, "<< JIT compiled code >>", 0);
+  objfile = allocate_objfile (NULL, "<< JIT compiled code >>",
+			      OBJF_NOT_FILENAME);
   objfile->per_bfd->gdbarch = target_gdbarch ();
 
   terminate_minimal_symbol_table (objfile);
@@ -926,7 +927,7 @@
   /* This call does not take ownership of SAI.  */
   make_cleanup_bfd_unref (nbfd);
   objfile = symbol_file_add_from_bfd (nbfd, bfd_get_filename (nbfd), 0, sai,
-				      OBJF_SHARED, NULL);
+				      OBJF_SHARED | OBJF_NOT_FILENAME, NULL);
 
   do_cleanups (old_cleanups);
   add_objfile_entry (objfile, entry_addr);
--- src/gdb/jv-lang.c	2013/09/24 14:00:06	1.112
+++ src/gdb/jv-lang.c	2013/10/09 13:22:36	1.113
@@ -118,7 +118,8 @@
 
       /* Mark it as shared so that it is cleared when the inferior is
 	 re-run.  */
-      dynamics_objfile = allocate_objfile (NULL, NULL, OBJF_SHARED);
+      dynamics_objfile = allocate_objfile (NULL, NULL,
+					   OBJF_SHARED | OBJF_NOT_FILENAME);
       dynamics_objfile->per_bfd->gdbarch = gdbarch;
 
       data = XCNEW (struct jv_per_objfile_data);
--- src/gdb/objfiles.c	2013/10/07 19:40:38	1.170
+++ src/gdb/objfiles.c	2013/10/09 13:22:36	1.171
@@ -289,6 +289,7 @@
   if (name == NULL)
     {
       gdb_assert (abfd == NULL);
+      gdb_assert ((flags & OBJF_NOT_FILENAME) != 0);
       name = "<<anonymous objfile>>";
     }
   objfile->original_name = obstack_copy0 (&objfile->objfile_obstack, name,
--- src/gdb/objfiles.h	2013/10/07 19:40:38	1.114
+++ src/gdb/objfiles.h	2013/10/09 13:22:36	1.115
@@ -429,6 +429,11 @@
 
 #define OBJF_MAINLINE (1 << 5)
 
+/* ORIGINAL_NAME and OBFD->FILENAME correspond to text description unrelated to
+   filesystem names.  It can be for example "<image in memory>".  */
+
+#define OBJF_NOT_FILENAME (1 << 6)
+
 /* Declarations for functions defined in objfiles.c */
 
 extern struct objfile *allocate_objfile (bfd *, const char *name, int);

[patchv2] Record objfile->original_name as an absolute path

[Jan Kratochvil]

Hi Doug,

here is an updated+rebased patch.  The testcase is kept as is.

No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu.


Thanks,
Jan


gdb/
2013-09-25  Doug Evans  <dje@google.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	* objfiles.c (allocate_objfile): Save original_name as an absolute
	path.
	* objfiles.h (struct objfile): Expand comment on original_name.
	* source.c (openp): Call gdb_abspath.
	* utils.c (gdb_abspath): New function.
	* utils.h (gdb_abspath): Declare.

gdb/testsuite/
2013-09-25  Doug Evans  <dje@google.com>

	* gdb.dwarf/dwp-symlink.c: Fake out gdb to not load debug info
	at start.
	* gdb.dwarf/dwp-symlink.exp: Test trying to load dwp when the binary
	has been specified with a relative path and we have chdir'd before
	accessing the debug info.

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index d1f3121..8cc93c5 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -276,6 +276,7 @@ struct objfile *
 allocate_objfile (bfd *abfd, const char *name, int flags)
 {
   struct objfile *objfile;
+  char *expanded_name;
 
   objfile = (struct objfile *) xzalloc (sizeof (struct objfile));
   objfile->psymbol_cache = psymbol_bcache_init ();
@@ -290,10 +291,20 @@ allocate_objfile (bfd *abfd, const char *name, int flags)
     {
       gdb_assert (abfd == NULL);
       gdb_assert ((flags & OBJF_NOT_FILENAME) != 0);
-      name = "<<anonymous objfile>>";
+      expanded_name = xstrdup ("<<anonymous objfile>>");
     }
-  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack, name,
-					  strlen (name));
+  else if ((flags & OBJF_NOT_FILENAME) != 0)
+    expanded_name = xstrdup (name);
+  else
+    expanded_name = gdb_abspath (name);
+  objfile->original_name = obstack_copy0 (&objfile->objfile_obstack,
+					  expanded_name,
+					  strlen (expanded_name));
+  xfree (expanded_name);
+
+  /* Update the per-objfile information that comes from the bfd, ensuring
+     that any data that is reference is saved in the per-objfile data
+     region.  */
 
   /* Update the per-objfile information that comes from the bfd, ensuring
      that any data that is reference is saved in the per-objfile data
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 08771d0..9f3d35f 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -212,8 +212,10 @@ struct objfile
 
     struct objfile *next;
 
-    /* The object file's name, tilde-expanded and absolute.  This
-       pointer is never NULL.  This does not have to be freed; it is
+    /* The object file's original name as specified by the user,
+       made absolute, and tilde-expanded.  However, it is not canonicalized
+       (i.e., it has not been passed through gdb_realpath).
+       This pointer is never NULL.  This does not have to be freed; it is
        guaranteed to have a lifetime at least as long as the objfile.  */
 
     char *original_name;
diff --git a/gdb/source.c b/gdb/source.c
index 9fa99b4..b115d79 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -853,28 +853,10 @@ done:
       /* If a file was opened, canonicalize its filename.  */
       if (fd < 0)
 	*filename_opened = NULL;
+      else if ((opts & OPF_RETURN_REALPATH) != 0)
+	*filename_opened = gdb_realpath (filename);
       else
-	{
-	  char *(*realpath_fptr) (const char *);
-
-	  realpath_fptr = ((opts & OPF_RETURN_REALPATH) != 0
-			   ? gdb_realpath : xstrdup);
-
-	  if (IS_ABSOLUTE_PATH (filename))
-	    *filename_opened = realpath_fptr (filename);
-	  else
-	    {
-	      /* Beware the // my son, the Emacs barfs, the botch that catch...  */
-
-	      char *f = concat (current_directory,
-				IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
-				? "" : SLASH_STRING,
-				filename, (char *)NULL);
-
-	      *filename_opened = realpath_fptr (f);
-	      xfree (f);
-	    }
-	}
+	*filename_opened = gdb_abspath (filename);
     }
 
   return fd;
diff --git a/gdb/testsuite/gdb.dwarf2/dwp-symlink.c b/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
index 5be12fb..a93d5e7 100644
--- a/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
+++ b/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
@@ -15,6 +15,13 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+/* Cheezy hack to prevent set_initial_language from trying to look up main.
+   We do this so that gdb won't try to open the dwp file when the file is
+   first selected.  This gives us a chance to do a chdir before attempting
+   to access the debug info.  */
+asm (".globl main.main");
+asm ("main.main: .byte 0");
+
 int
 main (int argc, char **argv)
 {
diff --git a/gdb/testsuite/gdb.dwarf2/dwp-symlink.exp b/gdb/testsuite/gdb.dwarf2/dwp-symlink.exp
index ad0522b..a0daae4 100644
--- a/gdb/testsuite/gdb.dwarf2/dwp-symlink.exp
+++ b/gdb/testsuite/gdb.dwarf2/dwp-symlink.exp
@@ -75,3 +75,23 @@ gdb_test "ptype main" {type = int \(\)} "binary default, dwp at symlink"
 clean_restart "$thelink"
 
 gdb_test "ptype main" {type = int \(int, char \*\*\)} "binary symlink, dwp at symlink"
+
+# Verify we can still find the dwp if we change directories and we specified
+# a relative path for the program.
+
+set saved_pwd [pwd]
+
+# This is clean_restart, but specifying a relative path to the binary.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_test "cd [file dirname [standard_output_file ${thelink}]]" \
+    "Working directory .*"
+gdb_load "./${thelink}"
+
+gdb_test "cd .." "Working directory .*"
+
+gdb_test "ptype main" {type = int \(int, char \*\*\)} \
+    "relative path, binary symlink, dwp at symlink"
+
+cd $saved_pwd
diff --git a/gdb/utils.c b/gdb/utils.c
index 26879ec..62d07d6 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3204,6 +3204,32 @@ gdb_realpath (const char *filename)
   return xstrdup (filename);
 }
 
+/* Return PATH in absolute form, performing tilde-expansion if necessary.
+   PATH cannot be NULL or the empty string.
+   This does not resolve symlinks however, use gdb_realpath for that.
+   Space for the result is allocated with malloc.
+   If the path is already absolute, it is strdup'd.
+   If there is a problem computing the absolute path, the path is returned
+   unchanged (still strdup'd).  */
+
+char *
+gdb_abspath (const char *path)
+{
+  gdb_assert (path != NULL && path[0] != '\0');
+
+  if (path[0] == '~')
+    return tilde_expand (path);
+
+  if (IS_ABSOLUTE_PATH (path))
+    return xstrdup (path);
+
+  /* Beware the // my son, the Emacs barfs, the botch that catch...  */
+  return concat (current_directory,
+	    IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
+		 ? "" : SLASH_STRING,
+		 path, (char *) NULL);
+}
+
 ULONGEST
 align_up (ULONGEST v, int n)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 3492f09..29f79d5 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -128,6 +128,8 @@ extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
 
 extern char *gdb_realpath (const char *);
 
+extern char *gdb_abspath (const char *);
+
 extern int gdb_filename_fnmatch (const char *pattern, const char *string,
 				 int flags);
 

Re: [patchv2] Record objfile->original_name as an absolute path

[Doug Evans]

On Wed, Oct 9, 2013 at 7:46 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> diff --git a/gdb/testsuite/gdb.dwarf2/dwp-symlink.c b/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
> index 5be12fb..a93d5e7 100644
> --- a/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
> +++ b/gdb/testsuite/gdb.dwarf2/dwp-symlink.c
> @@ -15,6 +15,13 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
> +/* Cheezy hack to prevent set_initial_language from trying to look up main.
> +   We do this so that gdb won't try to open the dwp file when the file is
> +   first selected.  This gives us a chance to do a chdir before attempting
> +   to access the debug info.  */
> +asm (".globl main.main");
> +asm ("main.main: .byte 0");
> +
>  int
>  main (int argc, char **argv)
>  {

People are actually ok with this?

Re: [patchv2] Record objfile->original_name as an absolute path

[Joel Brobecker]

> > +/* Cheezy hack to prevent set_initial_language from trying to look up main.
> > +   We do this so that gdb won't try to open the dwp file when the file is
> > +   first selected.  This gives us a chance to do a chdir before attempting
> > +   to access the debug info.  */
> > +asm (".globl main.main");
> > +asm ("main.main: .byte 0");
> > +
> >  int
> >  main (int argc, char **argv)
> >  {
> 
> People are actually ok with this?

FWIW, I tend to be a little less demanding in the testsuite.
But the first question is whether there is another alternative.
Is there?

-- 
Joel

Re: [patchv2] Record objfile->original_name as an absolute path

[Doug Evans]

On Wed, Oct 9, 2013 at 9:58 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> > +/* Cheezy hack to prevent set_initial_language from trying to look up main.
>> > +   We do this so that gdb won't try to open the dwp file when the file is
>> > +   first selected.  This gives us a chance to do a chdir before attempting
>> > +   to access the debug info.  */
>> > +asm (".globl main.main");
>> > +asm ("main.main: .byte 0");
>> > +
>> >  int
>> >  main (int argc, char **argv)
>> >  {
>>
>> People are actually ok with this?
>
> FWIW, I tend to be a little less demanding in the testsuite.
> But the first question is whether there is another alternative.
> Is there?

I was thinking,
We have auto-solib-add and sharedlibrary.
We just need something like that for symfile_objfile.
[I realize it's a bit different.  What I mean is we need a way to stop
gdb from auto-loading symbols and then a command to load the symbols
at a later point.  The catch here is that we need gdb to already know
about the file (we need to exercise whatever path gets set in the
objfile) - we just need to defer loading symbols until after we get
gdb to cd to a different directory.]
I'm trying to think of a use-case beyond the testsuite to better
justify having such a feature.

[One could do something to defer setting the initial language, but
that's just one reason why gdb might want to load symbols
immediately.]

Re: [patchv2] Record objfile->original_name as an absolute path

[Jan Kratochvil]

On Thu, 10 Oct 2013 18:31:22 +0200, Doug Evans wrote:
> I'm trying to think of a use-case beyond the testsuite to better
> justify having such a feature.

When you do not use .gdb_index loading the symbols can be very slow.

For example if you just want to gcore a process to a file (/usr/bin/gcore) you
need no symbols for it.


Jan

Re: [patchv2] Record objfile->original_name as an absolute path

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> What I mean is we need a way to stop
Doug> gdb from auto-loading symbols and then a command to load the symbols
Doug> at a later point.

Red Hat has a "-readnever" patch that we've shipped for a long time.
I think what's missing is a way to request that gdb start reading
symbols again.  Anyway we could upstream the patch if it sounds useful.

Doug> [One could do something to defer setting the initial language, but
Doug> that's just one reason why gdb might want to load symbols
Doug> immediately.]

FWIW I've implemented this idea at least once, maybe a couple of times.

Tom

Re: [patchv2] Record objfile->original_name as an absolute path

[Doug Evans]

On Thu, Oct 10, 2013 at 11:07 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> What I mean is we need a way to stop
> Doug> gdb from auto-loading symbols and then a command to load the symbols
> Doug> at a later point.
>
> Red Hat has a "-readnever" patch that we've shipped for a long time.
> I think what's missing is a way to request that gdb start reading
> symbols again.  Anyway we could upstream the patch if it sounds useful.

Yeah.  It's the missing part that's, umm, missing. :-)
With that, -readnever is misnamed, maybe upstream it as "-readlater"? :-)

[commit] [patchv2] Record objfile->original_name as an absolute path

[Jan Kratochvil]

On Wed, 09 Oct 2013 16:46:29 +0200, Jan Kratochvil wrote:
> gdb/
> 2013-09-25  Doug Evans  <dje@google.com>
> 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* objfiles.c (allocate_objfile): Save original_name as an absolute
> 	path.
> 	* objfiles.h (struct objfile): Expand comment on original_name.
> 	* source.c (openp): Call gdb_abspath.
> 	* utils.c (gdb_abspath): New function.
> 	* utils.h (gdb_abspath): Declare.
> 
> gdb/testsuite/
> 2013-09-25  Doug Evans  <dje@google.com>
> 
> 	* gdb.dwarf/dwp-symlink.c: Fake out gdb to not load debug info
> 	at start.
> 	* gdb.dwarf/dwp-symlink.exp: Test trying to load dwp when the binary
> 	has been specified with a relative path and we have chdir'd before
> 	accessing the debug info.

Checked in: 04affae3ef7aa124b6ac0ce4f3a54063b7b4784f


Jan