[PATCH] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[PATCH] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Sergio Durigan Junior]

Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1728147
Ref.: https://sourceware.org/bugzilla/show_bug.cgi?id=23613

Hi,

This bug has been reported a few days ago against Fedora GDB.  The
problem reported is that GDB segfaults when the working directory is
deleted.  It's pretty use to reproduce it:

  mkdir bla
  cd bla
  rmdir ../bla
  gdb echo

Debugging the problem is a bit tricky, because, since the current
directory doesn't exist anymore, a corefile cannot be saved there.
After a few attempts, I came up with the following:

  gdb -ex 'shell mkdir bla' -ex 'cd bla' -ex 'shell rmdir ../bla' -ex 'r echo' ./gdb/gdb

This assumes that you're inside a build directory which contains
./gdb/gdb, of course.

After investigating it, I found that the problem happens at
gdb_abspath, where we're dereferencing 'current_directory' without
checking if it's NULL:

    ...
    (concat (current_directory,
	     IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
	     ? "" : SLASH_STRING,
    ...

So I fixed the problem with the patch below.  The idea is that, if
'current_directory' is NULL, then the final string returned should be
just the "path".

After fixing the bug, I found a similar one reported against our
bugzilla: PR gdb/23613.  The problem is the same, but the reproducer
is a bit different.

I really tried writing a testcase for this, but unfortunately it's
apparently not possible to start GDB inside a non-existent directory
with DejaGNU.

I regression tested this patch on the BuildBot, and no regressions
were found.

gdb/ChangeLog:
2019-07-10  Sergio Durigan Junior  <sergiodj@redhat.com>

	https://bugzilla.redhat.com/show_bug.cgi?id=1728147
	PR gdb/23613
	* gdbsupport/pathstuff.c (gdb_abspath): Guard against
	'current_directory == NULL' case.
---
 gdb/gdbsupport/pathstuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/gdbsupport/pathstuff.c b/gdb/gdbsupport/pathstuff.c
index fafecd543d..aa51e8f36e 100644
--- a/gdb/gdbsupport/pathstuff.c
+++ b/gdb/gdbsupport/pathstuff.c
@@ -134,7 +134,7 @@ gdb_abspath (const char *path)
   if (path[0] == '~')
     return gdb_tilde_expand_up (path);
 
-  if (IS_ABSOLUTE_PATH (path))
+  if (IS_ABSOLUTE_PATH (path) || current_directory == NULL)
     return make_unique_xstrdup (path);
 
   /* Beware the // my son, the Emacs barfs, the botch that catch...  */
-- 
2.21.0

Re: [PATCH] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> So I fixed the problem with the patch below.  The idea is that, if
Sergio> 'current_directory' is NULL, then the final string returned should be
Sergio> just the "path".

I found other spots relying on current_directory in this way, so I
wonder if this is the best approach.

For example, corelow.c:

  gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
  if (!IS_ABSOLUTE_PATH (filename.get ()))
    filename.reset (concat (current_directory, "/",
			    filename.get (), (char *) NULL));

I guess this should just be converted to gdb_abspath?

... but also code in source.c and some other spots.


If this approach is the right one -- and it seems like it could be --
then the function documentation should at least be updated to reflect
this error case.

Tom

Re: [PATCH] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Sergio Durigan Junior]

Thanks for the review, Tom.

On Thursday, July 11 2019, Tom Tromey wrote:

>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
> Sergio> So I fixed the problem with the patch below.  The idea is that, if
> Sergio> 'current_directory' is NULL, then the final string returned should be
> Sergio> just the "path".
>
> I found other spots relying on current_directory in this way, so I
> wonder if this is the best approach.
>
> For example, corelow.c:
>
>   gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
>   if (!IS_ABSOLUTE_PATH (filename.get ()))
>     filename.reset (concat (current_directory, "/",
> 			    filename.get (), (char *) NULL));
>
> I guess this should just be converted to gdb_abspath?

Yeah, agreed, this is doing the same thing that gdb_abspath does, but
without the proper checks.

> ... but also code in source.c and some other spots.

I will make sure to grep for "current_directory" and adjust the code
where it's necessary.

> If this approach is the right one -- and it seems like it could be --
> then the function documentation should at least be updated to reflect
> this error case.

I'll do that as well, good idea.

As I mentioned on IRC, I think this is the best solution we have for
now.  It's really not possible to do much without a current directory
here.

I'll send v2 soon.  Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH v2] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Sergio Durigan Junior]

Changes from v1:

- Adjust all spots where 'current_directory' was being used with
  'concat' to determine the path of a file.

- Expand 'gdb_abspath' comment and mention what happens when
  'current_directory' is NULL.


Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1728147
Ref.: https://sourceware.org/bugzilla/show_bug.cgi?id=23613

Hi,

This bug has been reported against Fedora GDB, but there's also an
upstream bug.  The problem reported is that GDB segfaults when the
working directory is deleted.  It's pretty use to reproduce it:

  mkdir bla
  cd bla
  rmdir ../bla
  gdb echo

Debugging the problem is a bit tricky, because, since the current
directory doesn't exist anymore, a corefile cannot be saved there.
After a few attempts, I came up with the following:

  gdb -ex 'shell mkdir bla' -ex 'cd bla' -ex 'shell rmdir ../bla' -ex 'r echo' ./gdb/gdb

This assumes that you're inside a build directory which contains
./gdb/gdb, of course.

After investigating it, I found that the problem happens at
gdb_abspath, where we're dereferencing 'current_directory' without
checking if it's NULL:

    ...
    (concat (current_directory,
	     IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
	     ? "" : SLASH_STRING,
    ...

So I fixed the problem with the patch below.  The idea is that, if
'current_directory' is NULL, then the final string returned should be
just the "path".

After fixing the bug, I found a similar one reported against our
bugzilla: PR gdb/23613.  The problem is the same, but the reproducer
is a bit different.

I really tried writing a testcase for this, but unfortunately it's
apparently not possible to start GDB inside a non-existent directory
with DejaGNU.

I regression tested this patch on the BuildBot, and no regressions
were found.

gdb/ChangeLog:
2019-11-21  Sergio Durigan Junior  <sergiodj@redhat.com>

	https://bugzilla.redhat.com/show_bug.cgi?id=1728147
	PR gdb/23613
	* bsd-kvm.c (bsd_kvm_target_open): Use 'gdb_abspath'.
	* corelow.c: Include 'gdbsupport/pathstuff.h'.
	(core_target_open): Use 'gdb_abspath'.
	* gdbsupport/pathstuff.c (gdb_abspath): Guard against
	'current_directory == NULL' case.
	* gdbsupport/pathstuff.h (gdb_abspath): Expand comment and
	explain what happens when 'current_directory' is NULL.
	* go32-nat.c (go32_nat_target::wait): Check if
	'current_directory' is NULL before call to 'chdir'.
	* source.c (add_path): Use 'gdb_abspath'.
	* top.c: Include 'gdbsupport/pathstuff.h'.
	(init_history): Use 'gdb_abspath'.
	(set_history_filename): Likewise.
	* tracefile-tfile.c: Include 'gdbsupport/pathstuff.h'.
	(tfile_target_open): Use 'gdb_abspath'.

Change-Id: Ibb0932fa25bc5c2d3ae4a7f64bd7f32885ca403b
---
 gdb/bsd-kvm.c              |  7 +++----
 gdb/corelow.c              |  4 ++--
 gdb/gdbsupport/pathstuff.c |  2 +-
 gdb/gdbsupport/pathstuff.h |  5 ++++-
 gdb/go32-nat.c             |  3 ++-
 gdb/source.c               |  3 +--
 gdb/top.c                  | 18 ++++++++++++------
 gdb/tracefile-tfile.c      |  4 ++--
 8 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index 21f978728d..895686ef62 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -114,14 +114,13 @@ bsd_kvm_target_open (const char *arg, int from_tty)
 
   if (arg)
     {
-      char *temp;
-
       filename = tilde_expand (arg);
       if (filename[0] != '/')
 	{
-	  temp = concat (current_directory, "/", filename, (char *)NULL);
+	  gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (filename));
+
 	  xfree (filename);
-	  filename = temp;
+	  filename = temp.release ();
 	}
     }
 
diff --git a/gdb/corelow.c b/gdb/corelow.c
index fa1661ee61..c656306fe5 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -43,6 +43,7 @@
 #include "gdb_bfd.h"
 #include "completer.h"
 #include "gdbsupport/filestuff.h"
+#include "gdbsupport/pathstuff.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -373,8 +374,7 @@ core_target_open (const char *arg, int from_tty)
 
   gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
   if (!IS_ABSOLUTE_PATH (filename.get ()))
-    filename.reset (concat (current_directory, "/",
-			    filename.get (), (char *) NULL));
+    filename = gdb_abspath (filename.get ());
 
   flags = O_BINARY | O_LARGEFILE;
   if (write_files)
diff --git a/gdb/gdbsupport/pathstuff.c b/gdb/gdbsupport/pathstuff.c
index fafecd543d..aa51e8f36e 100644
--- a/gdb/gdbsupport/pathstuff.c
+++ b/gdb/gdbsupport/pathstuff.c
@@ -134,7 +134,7 @@ gdb_abspath (const char *path)
   if (path[0] == '~')
     return gdb_tilde_expand_up (path);
 
-  if (IS_ABSOLUTE_PATH (path))
+  if (IS_ABSOLUTE_PATH (path) || current_directory == NULL)
     return make_unique_xstrdup (path);
 
   /* Beware the // my son, the Emacs barfs, the botch that catch...  */
diff --git a/gdb/gdbsupport/pathstuff.h b/gdb/gdbsupport/pathstuff.h
index 7d7c8cd028..e84685b654 100644
--- a/gdb/gdbsupport/pathstuff.h
+++ b/gdb/gdbsupport/pathstuff.h
@@ -44,7 +44,10 @@ extern gdb::unique_xmalloc_ptr<char>
 
    Contrary to "gdb_realpath", this function uses CURRENT_DIRECTORY
    for the path expansion.  This may lead to scenarios the current
-   working directory (CWD) is different than CURRENT_DIRECTORY.  */
+   working directory (CWD) is different than CURRENT_DIRECTORY.
+
+   If CURRENT_DIRECTORY is NULL, this function returns a copy of
+   PATH.  */
 
 extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
 
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 8c71bd7650..94972a2f5c 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -507,7 +507,8 @@ go32_nat_target::wait (ptid_t ptid, struct target_waitstatus *status,
     }
 
   getcwd (child_cwd, sizeof (child_cwd)); /* in case it has changed */
-  chdir (current_directory);
+  if (current_directory != NULL)
+    chdir (current_directory);
 
   if (a_tss.tss_irqn == 0x21)
     {
diff --git a/gdb/source.c b/gdb/source.c
index d9cd5f32ca..b456a517e3 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -540,8 +540,7 @@ add_path (const char *dirname, char **which_path, int parse_separators)
 	new_name_holder.reset (concat (name, ".", (char *) NULL));
 #endif
       else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$')
-	new_name_holder.reset (concat (current_directory, SLASH_STRING, name,
-				       (char *) NULL));
+	new_name_holder = gdb_abspath (name);
       else
 	new_name_holder.reset (savestring (name, p - name));
       name = new_name_holder.get ();
diff --git a/gdb/top.c b/gdb/top.c
index 2953eac819..0893fe057f 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -54,6 +54,7 @@
 #include "gdb_select.h"
 #include "gdbsupport/scope-exit.h"
 #include "gdbarch.h"
+#include "gdbsupport/pathstuff.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -1982,12 +1983,13 @@ init_history (void)
          that was read.  */
 #ifdef __MSDOS__
       /* No leading dots in file names are allowed on MSDOS.  */
-      history_filename = concat (current_directory, "/_gdb_history",
-				 (char *)NULL);
+      const char *fname = "_gdb_history";
 #else
-      history_filename = concat (current_directory, "/.gdb_history",
-				 (char *)NULL);
+      const char *fname = ".gdb_history";
 #endif
+
+      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
+      history_filename = temp.release ();
     }
   read_history (history_filename);
 }
@@ -2065,8 +2067,12 @@ set_history_filename (const char *args,
      directories the file written will be the same as the one
      that was read.  */
   if (!IS_ABSOLUTE_PATH (history_filename))
-    history_filename = reconcat (history_filename, current_directory, "/", 
-				 history_filename, (char *) NULL);
+    {
+      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
+
+      xfree (history_filename);
+      history_filename = temp.release ();
+    }
 }
 
 static void
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 5c3837ec5b..0953b03781 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -32,6 +32,7 @@
 #include "xml-tdesc.h"
 #include "target-descriptions.h"
 #include "gdbsupport/buffer.h"
+#include "gdbsupport/pathstuff.h"
 #include <algorithm>
 
 #ifndef O_LARGEFILE
@@ -470,8 +471,7 @@ tfile_target_open (const char *arg, int from_tty)
 
   gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
   if (!IS_ABSOLUTE_PATH (filename.get ()))
-    filename.reset (concat (current_directory, "/", filename.get (),
-			    (char *) NULL));
+    filename = gdb_abspath (filename.get ());
 
   flags = O_BINARY | O_LARGEFILE;
   flags |= O_RDONLY;
-- 
2.21.0

Re: [PATCH v2] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Sergio Durigan Junior]

On Thursday, November 21 2019, I wrote:

> Changes from v1:
>
> - Adjust all spots where 'current_directory' was being used with
>   'concat' to determine the path of a file.
>
> - Expand 'gdb_abspath' comment and mention what happens when
>   'current_directory' is NULL.
>

Ping.

> Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1728147
> Ref.: https://sourceware.org/bugzilla/show_bug.cgi?id=23613
>
> Hi,
>
> This bug has been reported against Fedora GDB, but there's also an
> upstream bug.  The problem reported is that GDB segfaults when the
> working directory is deleted.  It's pretty use to reproduce it:
>
>   mkdir bla
>   cd bla
>   rmdir ../bla
>   gdb echo
>
> Debugging the problem is a bit tricky, because, since the current
> directory doesn't exist anymore, a corefile cannot be saved there.
> After a few attempts, I came up with the following:
>
>   gdb -ex 'shell mkdir bla' -ex 'cd bla' -ex 'shell rmdir ../bla' -ex 'r echo' ./gdb/gdb
>
> This assumes that you're inside a build directory which contains
> ./gdb/gdb, of course.
>
> After investigating it, I found that the problem happens at
> gdb_abspath, where we're dereferencing 'current_directory' without
> checking if it's NULL:
>
>     ...
>     (concat (current_directory,
> 	     IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
> 	     ? "" : SLASH_STRING,
>     ...
>
> So I fixed the problem with the patch below.  The idea is that, if
> 'current_directory' is NULL, then the final string returned should be
> just the "path".
>
> After fixing the bug, I found a similar one reported against our
> bugzilla: PR gdb/23613.  The problem is the same, but the reproducer
> is a bit different.
>
> I really tried writing a testcase for this, but unfortunately it's
> apparently not possible to start GDB inside a non-existent directory
> with DejaGNU.
>
> I regression tested this patch on the BuildBot, and no regressions
> were found.
>
> gdb/ChangeLog:
> 2019-11-21  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	https://bugzilla.redhat.com/show_bug.cgi?id=1728147
> 	PR gdb/23613
> 	* bsd-kvm.c (bsd_kvm_target_open): Use 'gdb_abspath'.
> 	* corelow.c: Include 'gdbsupport/pathstuff.h'.
> 	(core_target_open): Use 'gdb_abspath'.
> 	* gdbsupport/pathstuff.c (gdb_abspath): Guard against
> 	'current_directory == NULL' case.
> 	* gdbsupport/pathstuff.h (gdb_abspath): Expand comment and
> 	explain what happens when 'current_directory' is NULL.
> 	* go32-nat.c (go32_nat_target::wait): Check if
> 	'current_directory' is NULL before call to 'chdir'.
> 	* source.c (add_path): Use 'gdb_abspath'.
> 	* top.c: Include 'gdbsupport/pathstuff.h'.
> 	(init_history): Use 'gdb_abspath'.
> 	(set_history_filename): Likewise.
> 	* tracefile-tfile.c: Include 'gdbsupport/pathstuff.h'.
> 	(tfile_target_open): Use 'gdb_abspath'.
>
> Change-Id: Ibb0932fa25bc5c2d3ae4a7f64bd7f32885ca403b
> ---
>  gdb/bsd-kvm.c              |  7 +++----
>  gdb/corelow.c              |  4 ++--
>  gdb/gdbsupport/pathstuff.c |  2 +-
>  gdb/gdbsupport/pathstuff.h |  5 ++++-
>  gdb/go32-nat.c             |  3 ++-
>  gdb/source.c               |  3 +--
>  gdb/top.c                  | 18 ++++++++++++------
>  gdb/tracefile-tfile.c      |  4 ++--
>  8 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
> index 21f978728d..895686ef62 100644
> --- a/gdb/bsd-kvm.c
> +++ b/gdb/bsd-kvm.c
> @@ -114,14 +114,13 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>  
>    if (arg)
>      {
> -      char *temp;
> -
>        filename = tilde_expand (arg);
>        if (filename[0] != '/')
>  	{
> -	  temp = concat (current_directory, "/", filename, (char *)NULL);
> +	  gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (filename));
> +
>  	  xfree (filename);
> -	  filename = temp;
> +	  filename = temp.release ();
>  	}
>      }
>  
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index fa1661ee61..c656306fe5 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -43,6 +43,7 @@
>  #include "gdb_bfd.h"
>  #include "completer.h"
>  #include "gdbsupport/filestuff.h"
> +#include "gdbsupport/pathstuff.h"
>  
>  #ifndef O_LARGEFILE
>  #define O_LARGEFILE 0
> @@ -373,8 +374,7 @@ core_target_open (const char *arg, int from_tty)
>  
>    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
>    if (!IS_ABSOLUTE_PATH (filename.get ()))
> -    filename.reset (concat (current_directory, "/",
> -			    filename.get (), (char *) NULL));
> +    filename = gdb_abspath (filename.get ());
>  
>    flags = O_BINARY | O_LARGEFILE;
>    if (write_files)
> diff --git a/gdb/gdbsupport/pathstuff.c b/gdb/gdbsupport/pathstuff.c
> index fafecd543d..aa51e8f36e 100644
> --- a/gdb/gdbsupport/pathstuff.c
> +++ b/gdb/gdbsupport/pathstuff.c
> @@ -134,7 +134,7 @@ gdb_abspath (const char *path)
>    if (path[0] == '~')
>      return gdb_tilde_expand_up (path);
>  
> -  if (IS_ABSOLUTE_PATH (path))
> +  if (IS_ABSOLUTE_PATH (path) || current_directory == NULL)
>      return make_unique_xstrdup (path);
>  
>    /* Beware the // my son, the Emacs barfs, the botch that catch...  */
> diff --git a/gdb/gdbsupport/pathstuff.h b/gdb/gdbsupport/pathstuff.h
> index 7d7c8cd028..e84685b654 100644
> --- a/gdb/gdbsupport/pathstuff.h
> +++ b/gdb/gdbsupport/pathstuff.h
> @@ -44,7 +44,10 @@ extern gdb::unique_xmalloc_ptr<char>
>  
>     Contrary to "gdb_realpath", this function uses CURRENT_DIRECTORY
>     for the path expansion.  This may lead to scenarios the current
> -   working directory (CWD) is different than CURRENT_DIRECTORY.  */
> +   working directory (CWD) is different than CURRENT_DIRECTORY.
> +
> +   If CURRENT_DIRECTORY is NULL, this function returns a copy of
> +   PATH.  */
>  
>  extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
>  
> diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
> index 8c71bd7650..94972a2f5c 100644
> --- a/gdb/go32-nat.c
> +++ b/gdb/go32-nat.c
> @@ -507,7 +507,8 @@ go32_nat_target::wait (ptid_t ptid, struct target_waitstatus *status,
>      }
>  
>    getcwd (child_cwd, sizeof (child_cwd)); /* in case it has changed */
> -  chdir (current_directory);
> +  if (current_directory != NULL)
> +    chdir (current_directory);
>  
>    if (a_tss.tss_irqn == 0x21)
>      {
> diff --git a/gdb/source.c b/gdb/source.c
> index d9cd5f32ca..b456a517e3 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -540,8 +540,7 @@ add_path (const char *dirname, char **which_path, int parse_separators)
>  	new_name_holder.reset (concat (name, ".", (char *) NULL));
>  #endif
>        else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$')
> -	new_name_holder.reset (concat (current_directory, SLASH_STRING, name,
> -				       (char *) NULL));
> +	new_name_holder = gdb_abspath (name);
>        else
>  	new_name_holder.reset (savestring (name, p - name));
>        name = new_name_holder.get ();
> diff --git a/gdb/top.c b/gdb/top.c
> index 2953eac819..0893fe057f 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -54,6 +54,7 @@
>  #include "gdb_select.h"
>  #include "gdbsupport/scope-exit.h"
>  #include "gdbarch.h"
> +#include "gdbsupport/pathstuff.h"
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -1982,12 +1983,13 @@ init_history (void)
>           that was read.  */
>  #ifdef __MSDOS__
>        /* No leading dots in file names are allowed on MSDOS.  */
> -      history_filename = concat (current_directory, "/_gdb_history",
> -				 (char *)NULL);
> +      const char *fname = "_gdb_history";
>  #else
> -      history_filename = concat (current_directory, "/.gdb_history",
> -				 (char *)NULL);
> +      const char *fname = ".gdb_history";
>  #endif
> +
> +      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
> +      history_filename = temp.release ();
>      }
>    read_history (history_filename);
>  }
> @@ -2065,8 +2067,12 @@ set_history_filename (const char *args,
>       directories the file written will be the same as the one
>       that was read.  */
>    if (!IS_ABSOLUTE_PATH (history_filename))
> -    history_filename = reconcat (history_filename, current_directory, "/", 
> -				 history_filename, (char *) NULL);
> +    {
> +      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
> +
> +      xfree (history_filename);
> +      history_filename = temp.release ();
> +    }
>  }
>  
>  static void
> diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
> index 5c3837ec5b..0953b03781 100644
> --- a/gdb/tracefile-tfile.c
> +++ b/gdb/tracefile-tfile.c
> @@ -32,6 +32,7 @@
>  #include "xml-tdesc.h"
>  #include "target-descriptions.h"
>  #include "gdbsupport/buffer.h"
> +#include "gdbsupport/pathstuff.h"
>  #include <algorithm>
>  
>  #ifndef O_LARGEFILE
> @@ -470,8 +471,7 @@ tfile_target_open (const char *arg, int from_tty)
>  
>    gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
>    if (!IS_ABSOLUTE_PATH (filename.get ()))
> -    filename.reset (concat (current_directory, "/", filename.get (),
> -			    (char *) NULL));
> +    filename = gdb_abspath (filename.get ());
>  
>    flags = O_BINARY | O_LARGEFILE;
>    flags |= O_RDONLY;
> -- 
> 2.21.0

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH v2] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> gdb/ChangeLog:
Sergio> 2019-11-21  Sergio Durigan Junior  <sergiodj@redhat.com>

Sergio> 	https://bugzilla.redhat.com/show_bug.cgi?id=1728147
Sergio> 	PR gdb/23613
Sergio> 	* bsd-kvm.c (bsd_kvm_target_open): Use 'gdb_abspath'.
Sergio> 	* corelow.c: Include 'gdbsupport/pathstuff.h'.
Sergio> 	(core_target_open): Use 'gdb_abspath'.
Sergio> 	* gdbsupport/pathstuff.c (gdb_abspath): Guard against
Sergio> 	'current_directory == NULL' case.
Sergio> 	* gdbsupport/pathstuff.h (gdb_abspath): Expand comment and
Sergio> 	explain what happens when 'current_directory' is NULL.
Sergio> 	* go32-nat.c (go32_nat_target::wait): Check if
Sergio> 	'current_directory' is NULL before call to 'chdir'.
Sergio> 	* source.c (add_path): Use 'gdb_abspath'.
Sergio> 	* top.c: Include 'gdbsupport/pathstuff.h'.
Sergio> 	(init_history): Use 'gdb_abspath'.
Sergio> 	(set_history_filename): Likewise.
Sergio> 	* tracefile-tfile.c: Include 'gdbsupport/pathstuff.h'.
Sergio> 	(tfile_target_open): Use 'gdb_abspath'.

Thanks.  This is ok.

Tom

Re: [PATCH v2] Guard against 'current_directory == NULL' on gdb_abspath (PR gdb/23613)

[Sergio Durigan Junior]

On Friday, December 13 2019, Tom Tromey wrote:

>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
> Sergio> gdb/ChangeLog:
> Sergio> 2019-11-21  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> Sergio> 	https://bugzilla.redhat.com/show_bug.cgi?id=1728147
> Sergio> 	PR gdb/23613
> Sergio> 	* bsd-kvm.c (bsd_kvm_target_open): Use 'gdb_abspath'.
> Sergio> 	* corelow.c: Include 'gdbsupport/pathstuff.h'.
> Sergio> 	(core_target_open): Use 'gdb_abspath'.
> Sergio> 	* gdbsupport/pathstuff.c (gdb_abspath): Guard against
> Sergio> 	'current_directory == NULL' case.
> Sergio> 	* gdbsupport/pathstuff.h (gdb_abspath): Expand comment and
> Sergio> 	explain what happens when 'current_directory' is NULL.
> Sergio> 	* go32-nat.c (go32_nat_target::wait): Check if
> Sergio> 	'current_directory' is NULL before call to 'chdir'.
> Sergio> 	* source.c (add_path): Use 'gdb_abspath'.
> Sergio> 	* top.c: Include 'gdbsupport/pathstuff.h'.
> Sergio> 	(init_history): Use 'gdb_abspath'.
> Sergio> 	(set_history_filename): Likewise.
> Sergio> 	* tracefile-tfile.c: Include 'gdbsupport/pathstuff.h'.
> Sergio> 	(tfile_target_open): Use 'gdb_abspath'.
>
> Thanks.  This is ok.

Thanks for the review, Tom.

Pushed: ff8577f64987a898e1dc5eb6afb66a404fb7bb16

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/