[patch] [7.6.1] Fix argv[0] symlink regression (PR 15415)

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

Hi,

gdb resolves symbolic links when passing argv[0]
http://sourceware.org/bugzilla/show_bug.cgi?id=15415

7.6 started to pass gdb_realpath of argv[0] to spawned inferiors.
7.5 was passing xfullpath, therefore preserving the basename.

7.5
+PASS: gdb.base/argv0-symlink.exp: kept file symbolic link name
+FAIL: gdb.base/argv0-symlink.exp: kept directory symbolic link name

7.6
+FAIL: gdb.base/argv0-symlink.exp: kept file symbolic link name
+FAIL: gdb.base/argv0-symlink.exp: kept directory symbolic link name

proposed for 7.6.1:
+PASS: gdb.base/argv0-symlink.exp: kept file symbolic link name
+PASS: gdb.base/argv0-symlink.exp: kept directory symbolic link name

IMO even 7.5 was wrong, it did not keep symbol links in the path before
basename.  Yet another issue is that for example '$ gdb -ex r --args true'
will execute ./true if it exists while '$ true' does not - but fixing this
part is definitely out of the scope of 7.6.1.

There remains an issue considered as a regression by Doug
	http://sourceware.org/bugzilla/show_bug.cgi?id=15415#c5
that gdb 7.6 now also switched xfullpath to gdb_realpath for separate debug
info files (like .dwp).  Going to check what to do in a different patch.

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


Thanks,
Jan


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

	PR gdb/15415
	* corefile.c (get_exec_file): Use exec_filename.
	* defs.h (OPF_DISABLE_REALPATH): New definition.  Add new comment.
	* exec.c (exec_close): Free EXEC_FILENAME.
	(exec_file_attach): New variable canonical_pathname.  Use
	OPF_DISABLE_REALPATH.  Call gdb_realpath explicitly.  Set
	EXEC_FILENAME.
	* exec.h (exec_filename): New.
	* inferior.c (print_inferior, inferior_command): Use PSPACE_FILENAME.
	* mi/mi-main.c (print_one_inferior): Likewise.
	* progspace.c (clone_program_space, print_program_space): Likewise.
	* progspace.h (struct program_space): New field pspace_filename.
	* source.c (openp): Describe OPF_DISABLE_REALPATH.  New variable
	realpath_fptr, initialize it from OPF_DISABLE_REALPATH, use it.

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

	PR gdb/15415
	* gdb.base/argv0-symlink.c: New file.
	* gdb.base/argv0-symlink.exp: New file.

diff --git a/gdb/corefile.c b/gdb/corefile.c
index cb7f14e..1b733e2 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -182,8 +182,8 @@ validate_files (void)
 char *
 get_exec_file (int err)
 {
-  if (exec_bfd)
-    return bfd_get_filename (exec_bfd);
+  if (exec_filename)
+    return exec_filename;
   if (!err)
     return NULL;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index 014d7d4..74b607d 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -346,8 +346,10 @@ extern const char *pc_prefix (CORE_ADDR);
 
 /* From source.c */
 
+/* See openp function definition for their description.  */
 #define OPF_TRY_CWD_FIRST     0x01
 #define OPF_SEARCH_IN_PATH    0x02
+#define OPF_DISABLE_REALPATH  0x04
 
 extern int openp (const char *, int, const char *, int, char **);
 
diff --git a/gdb/exec.c b/gdb/exec.c
index 14ff6d7..4ef75e3 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -102,6 +102,9 @@ exec_close (void)
       exec_bfd_mtime = 0;
 
       remove_target_sections (&exec_bfd);
+
+      xfree (exec_filename);
+      exec_filename = NULL;
     }
 }
 
@@ -179,12 +182,13 @@ exec_file_attach (char *filename, int from_tty)
   else
     {
       struct cleanup *cleanups;
-      char *scratch_pathname;
+      char *scratch_pathname, *canonical_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
       char **matching;
 
-      scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
+      scratch_chan = openp (getenv ("PATH"),
+			    OPF_TRY_CWD_FIRST | OPF_DISABLE_REALPATH, filename,
 		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
 			    &scratch_pathname);
 #if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
@@ -193,7 +197,9 @@ exec_file_attach (char *filename, int from_tty)
 	  char *exename = alloca (strlen (filename) + 5);
 
 	  strcat (strcpy (exename, filename), ".exe");
-	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, exename,
+	  scratch_chan = openp (getenv ("PATH"),
+				OPF_TRY_CWD_FIRST | OPF_DISABLE_REALPATH,
+				exename,
 	     write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
 	     &scratch_pathname);
 	}
@@ -203,11 +209,14 @@ exec_file_attach (char *filename, int from_tty)
 
       cleanups = make_cleanup (xfree, scratch_pathname);
 
+      canonical_pathname = gdb_realpath (scratch_pathname);
+      make_cleanup (xfree, canonical_pathname);
+
       if (write_files)
-	exec_bfd = gdb_bfd_fopen (scratch_pathname, gnutarget,
+	exec_bfd = gdb_bfd_fopen (canonical_pathname, gnutarget,
 				  FOPEN_RUB, scratch_chan);
       else
-	exec_bfd = gdb_bfd_open (scratch_pathname, gnutarget, scratch_chan);
+	exec_bfd = gdb_bfd_open (canonical_pathname, gnutarget, scratch_chan);
 
       if (!exec_bfd)
 	{
@@ -215,6 +224,9 @@ exec_file_attach (char *filename, int from_tty)
 		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
 
+      gdb_assert (exec_filename == NULL);
+      exec_filename = xstrdup (scratch_pathname);
+
       if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
diff --git a/gdb/exec.h b/gdb/exec.h
index 21ccba8..87aa2b4 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -32,6 +32,7 @@ extern struct target_ops exec_ops;
 
 #define exec_bfd current_program_space->ebfd
 #define exec_bfd_mtime current_program_space->ebfd_mtime
+#define exec_filename current_program_space->pspace_filename
 
 /* Builds a section table, given args BFD, SECTABLE_PTR, SECEND_PTR.
    Returns 0 if OK, 1 on error.  */
diff --git a/gdb/inferior.c b/gdb/inferior.c
index b9e9a8d..d346973 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -588,9 +588,8 @@ print_inferior (struct ui_out *uiout, char *requested_inferiors)
       ui_out_field_string (uiout, "target-id",
 			   inferior_pid_to_str (inf->pid));
 
-      if (inf->pspace->ebfd)
-	ui_out_field_string (uiout, "exec",
-			     bfd_get_filename (inf->pspace->ebfd));
+      if (inf->pspace->pspace_filename != NULL)
+	ui_out_field_string (uiout, "exec", inf->pspace->pspace_filename);
       else
 	ui_out_field_skip (uiout, "exec");
 
@@ -704,8 +703,8 @@ inferior_command (char *args, int from_tty)
   printf_filtered (_("[Switching to inferior %d [%s] (%s)]\n"),
 		   inf->num,
 		   inferior_pid_to_str (inf->pid),
-		   (inf->pspace->ebfd
-		    ? bfd_get_filename (inf->pspace->ebfd)
+		   (inf->pspace->pspace_filename != NULL
+		    ? inf->pspace->pspace_filename
 		    : _("<noexec>")));
 
   if (inf->pid != 0)
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index c2d8501..33a3082 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -573,10 +573,10 @@ print_one_inferior (struct inferior *inferior, void *xdata)
       if (inferior->pid != 0)
 	ui_out_field_int (uiout, "pid", inferior->pid);
 
-      if (inferior->pspace->ebfd)
+      if (inferior->pspace->pspace_filename != NULL)
 	{
 	  ui_out_field_string (uiout, "executable",
-			       bfd_get_filename (inferior->pspace->ebfd));
+			       inferior->pspace->pspace_filename);
 	}
 
       data.cores = 0;
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 590ea9b..b345568 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -196,8 +196,8 @@ clone_program_space (struct program_space *dest, struct program_space *src)
 
   set_current_program_space (dest);
 
-  if (src->ebfd != NULL)
-    exec_file_attach (bfd_get_filename (src->ebfd), 0);
+  if (src->pspace_filename != NULL)
+    exec_file_attach (src->pspace_filename, 0);
 
   if (src->symfile_object_file != NULL)
     symbol_file_add_main (src->symfile_object_file->name, 0);
@@ -336,9 +336,8 @@ print_program_space (struct ui_out *uiout, int requested)
 
       ui_out_field_int (uiout, "id", pspace->num);
 
-      if (pspace->ebfd)
-	ui_out_field_string (uiout, "exec",
-			     bfd_get_filename (pspace->ebfd));
+      if (pspace->pspace_filename)
+	ui_out_field_string (uiout, "exec", pspace->pspace_filename);
       else
 	ui_out_field_skip (uiout, "exec");
 
diff --git a/gdb/progspace.h b/gdb/progspace.h
index 9d98baf..df71b7a 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -148,6 +148,10 @@ struct program_space
     bfd *ebfd;
     /* The last-modified time, from when the exec was brought in.  */
     long ebfd_mtime;
+    /* Similar to bfd_get_filename (exec_bfd) but in original form given
+       by user, without symbolic links and pathname resolved.
+       It needs to be freed by xfree.  It is not NULL iff EBFD is not NULL.  */
+    char *pspace_filename;
 
     /* The address space attached to this program space.  More than one
        program space may be bound to the same address space.  In the
diff --git a/gdb/source.c b/gdb/source.c
index e1c498b..de3fb7c 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -689,6 +689,11 @@ is_regular_file (const char *name)
    and the file, sigh!  Emacs gets confuzzed by this when we print the
    source file name!!! 
 
+   If OPTS does not have OPF_DISABLE_REALPATH set return FILENAME_OPENED
+   resolved by gdb_realpath.  Even with OPF_DISABLE_REALPATH this function
+   still returns filename starting with "/".  If FILENAME_OPENED is NULL
+   this option has no effect.
+
    If a file is found, return the descriptor.
    Otherwise, return -1, with errno set for the last name we tried to open.  */
 
@@ -848,19 +853,27 @@ done:
       /* If a file was opened, canonicalize its filename.  */
       if (fd < 0)
 	*filename_opened = NULL;
-      else if (IS_ABSOLUTE_PATH (filename))
-	*filename_opened = gdb_realpath (filename);
       else
 	{
-	  /* Beware the // my son, the Emacs barfs, the botch that catch...  */
+	  char *(*realpath_fptr) (const char *);
+
+	  realpath_fptr = ((opts & OPF_DISABLE_REALPATH) != 0
+			   ? xstrdup : gdb_realpath);
+
+	  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);
+	      char *f = concat (current_directory,
+				IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
+				? "" : SLASH_STRING,
+				filename, (char *)NULL);
 
-	  *filename_opened = gdb_realpath (f);
-	  xfree (f);
+	      *filename_opened = realpath_fptr (f);
+	      xfree (f);
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.base/argv0-symlink.c b/gdb/testsuite/gdb.base/argv0-symlink.c
new file mode 100644
index 0000000..5be12fb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/argv0-symlink.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (int argc, char **argv)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/argv0-symlink.exp b/gdb/testsuite/gdb.base/argv0-symlink.exp
new file mode 100644
index 0000000..fc61efb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/argv0-symlink.exp
@@ -0,0 +1,63 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [build_executable ${testfile}.exp ${testfile} ${srcfile}] == -1 } {
+    return -1
+}
+
+set test "kept file symbolic link name"
+set filelink "${testfile}-filelink"
+
+# 'file link' is tcl 8.4+ only.
+remote_exec host "ln -sf ${testfile} [standard_output_file $filelink]"
+
+# Remote host filesystem is not properly tested here.
+if { [file type [standard_output_file $filelink]] != "link" } {
+    unsupported "$test (host does not support symbolic links)"
+    return 0
+}
+
+clean_restart "$filelink"
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_test {print argv[0]} "/$filelink\"" $test
+
+
+set test "kept directory symbolic link name"
+set dirlink "${testfile}-dirlink"
+
+remote_exec host "rm -f [standard_output_file $dirlink]"
+remote_exec host "ln -sf . [standard_output_file $dirlink]"
+
+# Remote host filesystem is not properly tested here.
+if { [file type [standard_output_file $dirlink]] != "link" } {
+    unsupported "$test (host does not support symbolic links)"
+    return 0
+}
+
+clean_restart "$dirlink/$filelink"
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_test {print argv[0]} "/$dirlink/$filelink\"" $test