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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

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