[PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

As reported in PR 27157, if some environment variables read at startup
by GDB are defined but empty, we hit the assert in gdb_abspath:

    $ XDG_CACHE_HOME= ./gdb -nx --data-directory=data-directory -q
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==2007040==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000001b0 (pc 0x5639d4aa4127 bp 0x7ffdac232c00 sp 0x7ffdac232bf0 T0)
    ==2007040==The signal is caused by a READ memory access.
    ==2007040==Hint: address points to the zero page.
        #0 0x5639d4aa4126 in target_stack::top() const /home/smarchi/src/binutils-gdb/gdb/target.h:1334
        #1 0x5639d4aa41f1 in inferior::top_target() /home/smarchi/src/binutils-gdb/gdb/inferior.h:369
        #2 0x5639d4a70b1f in current_top_target() /home/smarchi/src/binutils-gdb/gdb/target.c:120
        #3 0x5639d4b00591 in gdb_readline_wrapper_cleanup::gdb_readline_wrapper_cleanup() /home/smarchi/src/binutils-gdb/gdb/top.c:1046
        #4 0x5639d4afab31 in gdb_readline_wrapper(char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1104
        #5 0x5639d4ccce2c in defaulted_query /home/smarchi/src/binutils-gdb/gdb/utils.c:893
        #6 0x5639d4ccd6af in query(char const*, ...) /home/smarchi/src/binutils-gdb/gdb/utils.c:985
        #7 0x5639d4ccaec1 in internal_vproblem /home/smarchi/src/binutils-gdb/gdb/utils.c:373
        #8 0x5639d4ccb3d1 in internal_verror(char const*, int, char const*, __va_list_tag*) /home/smarchi/src/binutils-gdb/gdb/utils.c:439
        #9 0x5639d5151a92 in internal_error(char const*, int, char const*, ...) /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
        #10 0x5639d5162ab4 in gdb_abspath(char const*) /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:132
        #11 0x5639d5162fac in get_standard_cache_dir[abi:cxx11]() /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:228
        #12 0x5639d3e76a81 in _initialize_index_cache() /home/smarchi/src/binutils-gdb/gdb/dwarf2/index-cache.c:325
        #13 0x5639d4dbbe92 in initialize_all_files() /home/smarchi/build/binutils-gdb/gdb/init.c:321
        #14 0x5639d4b00259 in gdb_init(char*) /home/smarchi/src/binutils-gdb/gdb/top.c:2344
        #15 0x5639d4440715 in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:950
        #16 0x5639d444252e in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1229
        #17 0x5639d44425cf in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1254
        #18 0x5639d3923371 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
        #19 0x7fa002d3f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
        #20 0x5639d392314d in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x4d414d)

gdb_abspath doesn't handle empty strings, so handle this case in the
callers.  If a variable is defined but empty, I think it's reasonable in
this case to just ignore it, as if it was not defined.

Note that this sometimes also lead to a segfault, because the failed
assertion happens very early during startup, before things are fully
initialized.

gdbsupport/ChangeLog:

	PR gdb/27157
	* pathstuff.cc (get_standard_cache_dir, get_standard_config_dir,
	find_gdb_home_config_file): Add empty string check.

gdb/testsuite/ChangeLog:

	PR gdb/27157
	* gdb.base/empty-host-env-vars.exp: New test.

Change-Id: I8654d8e97e74e1dff6d308c111ae4b1bbf07bef9
---
 .../gdb.base/empty-host-env-vars.exp          | 28 +++++++++++++++++++
 gdbsupport/pathstuff.cc                       | 14 +++++-----
 2 files changed, 35 insertions(+), 7 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp

diff --git a/gdb/testsuite/gdb.base/empty-host-env-vars.exp b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
new file mode 100644
index 00000000000..54cd4b92ccf
--- /dev/null
+++ b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
@@ -0,0 +1,28 @@
+# Copyright 2021 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/>.
+
+# GDB reads some environment variables on startup, make sure it behaves
+# correctly if these variables are defined but empty.
+
+foreach_with_prefix env_var_name {HOME XDG_CACHE_HOME LOCALAPPDATA XDG_CONFIG_HOME } {
+    # Restore the original state of the environment variable.
+    save_vars env($env_var_name) {
+	set env($env_var_name) {}
+	clean_restart
+
+	# Check that GDB is really started and working.
+	gdb_test "print 1" " = 1"
+    }
+}
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
index 51ab8c651b2..ad13900819e 100644
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -222,7 +222,7 @@ get_standard_cache_dir ()
 
 #ifndef __APPLE__
   const char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
-  if (xdg_cache_home != NULL)
+  if (xdg_cache_home != NULL && xdg_cache_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
@@ -231,7 +231,7 @@ get_standard_cache_dir ()
 #endif
 
   const char *home = getenv ("HOME");
-  if (home != NULL)
+  if (home != NULL && home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
@@ -240,14 +240,14 @@ get_standard_cache_dir ()
 
 #ifdef WIN32
   const char *win_home = getenv ("LOCALAPPDATA");
-  if (win_home != NULL)
+  if (win_home != NULL && win_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (win_home));
       return string_printf ("%s/gdb", abs.get ());
     }
 #endif
-    
+
   return {};
 }
 
@@ -289,7 +289,7 @@ get_standard_config_dir ()
 
 #ifndef __APPLE__
   const char *xdg_config_home = getenv ("XDG_CONFIG_HOME");
-  if (xdg_config_home != NULL)
+  if (xdg_config_home != NULL && xdg_config_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_config_home));
@@ -298,7 +298,7 @@ get_standard_config_dir ()
 #endif
 
   const char *home = getenv ("HOME");
-  if (home != NULL)
+  if (home != NULL && home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
@@ -340,7 +340,7 @@ find_gdb_home_config_file (const char *name, struct stat *buf)
     }
 
   const char *homedir = getenv ("HOME");
-  if (homedir != nullptr)
+  if (homedir != nullptr && homedir[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (homedir));
-- 
2.29.2

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> gdb_abspath doesn't handle empty strings, so handle this case in the
Simon> callers.  If a variable is defined but empty, I think it's reasonable in
Simon> this case to just ignore it, as if it was not defined.

Makes sense to me.

For XDG, I read here:

    https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

this:

    All paths set in these environment variables must be absolute. If an
    implementation encounters a relative path in any of these variables it
    should consider the path invalid and ignore it.

So I suppose we could go a bit further here.

Tom

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

On 2021-01-07 4:00 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> gdb_abspath doesn't handle empty strings, so handle this case in the
> Simon> callers.  If a variable is defined but empty, I think it's reasonable in
> Simon> this case to just ignore it, as if it was not defined.
> 
> Makes sense to me.
> 
> For XDG, I read here:
> 
>     https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> this:
> 
>     All paths set in these environment variables must be absolute. If an
>     implementation encounters a relative path in any of these variables it
>     should consider the path invalid and ignore it.
> 
> So I suppose we could go a bit further here.
> 
> Tom
> 

Ok, I can try this as a follow-up patch, check that the path
is absolute using the IS_ABSOLUTE_PATH macro.  Is a tilde path
considered absolute? Like

  ~/my-cache
  ~smarchi/my-cache

I suppose that it is, since it's just a shortcut for
/home/smarchi.

Simon

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Lancelot SIX]

Hi
> Ok, I can try this as a follow-up patch, check that the path
> is absolute using the IS_ABSOLUTE_PATH macro.  Is a tilde path
> considered absolute? Like
>
>    ~/my-cache
>    ~smarchi/my-cache
I would be tempted to say that it is not (according to 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html 
it seems it is not).
> I suppose that it is, since it's just a shortcut for
> /home/smarchi.

This actually is the most usual pattern, but the home directory of 
smarchi could be different. The actual answer is in /etc/passwd or 
accessed via getent:

$ getent passwd smarchi | awk -F: '{ print $6 }'

>
> Simon
Lancelot.

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

On 2021-01-07 4:43 p.m., Lancelot SIX wrote:
> Hi
>> Ok, I can try this as a follow-up patch, check that the path
>> is absolute using the IS_ABSOLUTE_PATH macro.  Is a tilde path
>> considered absolute? Like
>>
>>    ~/my-cache
>>    ~smarchi/my-cache
> I would be tempted to say that it is not (according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html it seems it is not).

I wouldn't mind not supporting tilde paths, but since it worked up to now,
it could be seen as a regression by some.

>> I suppose that it is, since it's just a shortcut for
>> /home/smarchi.
> 
> This actually is the most usual pattern, but the home directory of smarchi could be different. The actual answer is in /etc/passwd or accessed via getent:
> 
> $ getent passwd smarchi | awk -F: '{ print $6 }'

Right.  If we want to support it, we would presumably use gdb_tilde_expand,
which uses glob(3), which gets the real user home.

What's annoying though is that glob won't work with non-existent directories.
So if I use

  /home/smarchi/my/non/existent/directory

it is accepted, and these directories will be created.  But I set

  ~/my/non/existent/directory

gdb_tilde_expand will fail.  Perhaps it's just a limitation of gdb_tilde_expand,
and it should be changed to use something else than glob(3) in order to support
non-existent paths.

Simon

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Lancelot SIX]

> What's annoying though is that glob won't work with non-existent directories.
> So if I use
>
>    /home/smarchi/my/non/existent/directory
>
> it is accepted, and these directories will be created.  But I set
>
>    ~/my/non/existent/directory
>
> gdb_tilde_expand will fail.  Perhaps it's just a limitation of gdb_tilde_expand,
> and it should be changed to use something else than glob(3) in order to support
> non-existent paths.
>
> Simon

Using glob just to expand the first path component should do the trick 
quite easily.

I’ll have a look into it to update gdb_tilde_expand (unless someone does 
it before me).

Lancelot

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

On 2021-01-07 5:04 p.m., Lancelot SIX via Gdb-patches wrote:
> 
> Using glob just to expand the first path component should do the trick quite easily.
> 
> I’ll have a look into it to update gdb_tilde_expand (unless someone does it before me).

Feel free to give it a try!

Simon

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Sergio Durigan Junior]

On Thursday, January 07 2021, Simon Marchi via Gdb-patches wrote:

> Perhaps it's just a limitation of gdb_tilde_expand, and it should be
> changed to use something else than glob(3) in order to support
> non-existent paths.

Just to keep things in perspective and try to shed some light into why
things are the way they are:

Back when gdb_tilde_expand was implemented, I tried to find an
alternative to using glob.  Until that point, GDB was using readline's
tilde_expand, which does the job.  Unfortunately, due to the reason
that we needed to use gdb_tilde_expand on gdbserver as well (which
doesn't link against readline), and that gnulib doesn't support
wordexp(3), glob was the only viable alternative I found.

Cheers,

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

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Andrew Burgess]

* Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> [2021-01-07 21:43:37 +0000]:

> Hi
> > Ok, I can try this as a follow-up patch, check that the path
> > is absolute using the IS_ABSOLUTE_PATH macro.  Is a tilde path
> > considered absolute? Like
> > 
> >    ~/my-cache
> >    ~smarchi/my-cache
> I would be tempted to say that it is not (according to
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html it
> seems it is not).

I guess you're suggesting it's not because it doesn't start with a
slash?

However, isn't ~ expansion something a tool does, not a file system
thing?

So '~/my-cache' as a path is really asking to open ${CWD}/~/my-cache,
it's just that many tools actually choose to modify any incoming path
starting with ~.

So, is '~/my-cache' absolute?  No, as an incoming request from the
user, I agree with you it is not. But, GDB attaches special meaning
to paths starting with ~, and converts them into absolute paths...

I think we already agreed not to change this behaviour anyway, so I
probably shouldn't even be writing this....

Thanks,
Andrew

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Andrew Burgess]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-01-07 15:57:25 -0500]:

> As reported in PR 27157, if some environment variables read at startup
> by GDB are defined but empty, we hit the assert in gdb_abspath:
> 
>     $ XDG_CACHE_HOME= ./gdb -nx --data-directory=data-directory -q
>     AddressSanitizer:DEADLYSIGNAL
>     =================================================================
>     ==2007040==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000001b0 (pc 0x5639d4aa4127 bp 0x7ffdac232c00 sp 0x7ffdac232bf0 T0)
>     ==2007040==The signal is caused by a READ memory access.
>     ==2007040==Hint: address points to the zero page.
>         #0 0x5639d4aa4126 in target_stack::top() const /home/smarchi/src/binutils-gdb/gdb/target.h:1334
>         #1 0x5639d4aa41f1 in inferior::top_target() /home/smarchi/src/binutils-gdb/gdb/inferior.h:369
>         #2 0x5639d4a70b1f in current_top_target() /home/smarchi/src/binutils-gdb/gdb/target.c:120
>         #3 0x5639d4b00591 in gdb_readline_wrapper_cleanup::gdb_readline_wrapper_cleanup() /home/smarchi/src/binutils-gdb/gdb/top.c:1046
>         #4 0x5639d4afab31 in gdb_readline_wrapper(char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1104
>         #5 0x5639d4ccce2c in defaulted_query /home/smarchi/src/binutils-gdb/gdb/utils.c:893
>         #6 0x5639d4ccd6af in query(char const*, ...) /home/smarchi/src/binutils-gdb/gdb/utils.c:985
>         #7 0x5639d4ccaec1 in internal_vproblem /home/smarchi/src/binutils-gdb/gdb/utils.c:373
>         #8 0x5639d4ccb3d1 in internal_verror(char const*, int, char const*, __va_list_tag*) /home/smarchi/src/binutils-gdb/gdb/utils.c:439
>         #9 0x5639d5151a92 in internal_error(char const*, int, char const*, ...) /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
>         #10 0x5639d5162ab4 in gdb_abspath(char const*) /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:132
>         #11 0x5639d5162fac in get_standard_cache_dir[abi:cxx11]() /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:228
>         #12 0x5639d3e76a81 in _initialize_index_cache() /home/smarchi/src/binutils-gdb/gdb/dwarf2/index-cache.c:325
>         #13 0x5639d4dbbe92 in initialize_all_files() /home/smarchi/build/binutils-gdb/gdb/init.c:321
>         #14 0x5639d4b00259 in gdb_init(char*) /home/smarchi/src/binutils-gdb/gdb/top.c:2344
>         #15 0x5639d4440715 in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:950
>         #16 0x5639d444252e in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1229
>         #17 0x5639d44425cf in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1254
>         #18 0x5639d3923371 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
>         #19 0x7fa002d3f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
>         #20 0x5639d392314d in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x4d414d)
> 
> gdb_abspath doesn't handle empty strings, so handle this case in the
> callers.  If a variable is defined but empty, I think it's reasonable in
> this case to just ignore it, as if it was not defined.
> 
> Note that this sometimes also lead to a segfault, because the failed
> assertion happens very early during startup, before things are fully
> initialized.
> 
> gdbsupport/ChangeLog:
> 
> 	PR gdb/27157
> 	* pathstuff.cc (get_standard_cache_dir, get_standard_config_dir,
> 	find_gdb_home_config_file): Add empty string check.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR gdb/27157
> 	* gdb.base/empty-host-env-vars.exp: New test.
> 
> Change-Id: I8654d8e97e74e1dff6d308c111ae4b1bbf07bef9
> ---
>  .../gdb.base/empty-host-env-vars.exp          | 28 +++++++++++++++++++
>  gdbsupport/pathstuff.cc                       | 14 +++++-----
>  2 files changed, 35 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp
> 
> diff --git a/gdb/testsuite/gdb.base/empty-host-env-vars.exp b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
> new file mode 100644
> index 00000000000..54cd4b92ccf
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
> @@ -0,0 +1,28 @@
> +# Copyright 2021 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/>.
> +
> +# GDB reads some environment variables on startup, make sure it behaves
> +# correctly if these variables are defined but empty.
> +
> +foreach_with_prefix env_var_name {HOME XDG_CACHE_HOME LOCALAPPDATA XDG_CONFIG_HOME } {
> +    # Restore the original state of the environment variable.
> +    save_vars env($env_var_name) {
> +	set env($env_var_name) {}
> +	clean_restart
> +
> +	# Check that GDB is really started and working.
> +	gdb_test "print 1" " = 1"
> +    }
> +}

I wonder if it's worth including the case where ALL of the environment
variables are empty as well, just to check that there's no issues with
GDB falling through all the checks?

Otherwise, LGTM.

Thanks,
Andrew


> diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
> index 51ab8c651b2..ad13900819e 100644
> --- a/gdbsupport/pathstuff.cc
> +++ b/gdbsupport/pathstuff.cc
> @@ -222,7 +222,7 @@ get_standard_cache_dir ()
>  
>  #ifndef __APPLE__
>    const char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
> -  if (xdg_cache_home != NULL)
> +  if (xdg_cache_home != NULL && xdg_cache_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
> @@ -231,7 +231,7 @@ get_standard_cache_dir ()
>  #endif
>  
>    const char *home = getenv ("HOME");
> -  if (home != NULL)
> +  if (home != NULL && home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
> @@ -240,14 +240,14 @@ get_standard_cache_dir ()
>  
>  #ifdef WIN32
>    const char *win_home = getenv ("LOCALAPPDATA");
> -  if (win_home != NULL)
> +  if (win_home != NULL && win_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (win_home));
>        return string_printf ("%s/gdb", abs.get ());
>      }
>  #endif
> -    
> +
>    return {};
>  }
>  
> @@ -289,7 +289,7 @@ get_standard_config_dir ()
>  
>  #ifndef __APPLE__
>    const char *xdg_config_home = getenv ("XDG_CONFIG_HOME");
> -  if (xdg_config_home != NULL)
> +  if (xdg_config_home != NULL && xdg_config_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_config_home));
> @@ -298,7 +298,7 @@ get_standard_config_dir ()
>  #endif
>  
>    const char *home = getenv ("HOME");
> -  if (home != NULL)
> +  if (home != NULL && home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
> @@ -340,7 +340,7 @@ find_gdb_home_config_file (const char *name, struct stat *buf)
>      }
>  
>    const char *homedir = getenv ("HOME");
> -  if (homedir != nullptr)
> +  if (homedir != nullptr && homedir[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (homedir));
> -- 
> 2.29.2
> 

Re: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir

[Christian Biesinger]

On Thu, Jan 7, 2021 at 10:15 PM Simon Marchi via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> On 2021-01-07 4:00 p.m., Tom Tromey wrote:
> >>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> >
> > Simon> gdb_abspath doesn't handle empty strings, so handle this case in the
> > Simon> callers.  If a variable is defined but empty, I think it's reasonable in
> > Simon> this case to just ignore it, as if it was not defined.
> >
> > Makes sense to me.
> >
> > For XDG, I read here:
> >
> >     https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> >
> > this:
> >
> >     All paths set in these environment variables must be absolute. If an
> >     implementation encounters a relative path in any of these variables it
> >     should consider the path invalid and ignore it.
> >
> > So I suppose we could go a bit further here.
> >
> > Tom
> >
>
> Ok, I can try this as a follow-up patch, check that the path
> is absolute using the IS_ABSOLUTE_PATH macro.  Is a tilde path
> considered absolute? Like
>
>   ~/my-cache
>   ~smarchi/my-cache

IMO tildes in environment variables are pretty uncommon and often
unsupported by tools; usually the shell expands them before setting
it.

So I don't know if it's worth writing code to support that.

Christian

[PATCH v2] gdb: check for empty strings in, get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

On 2021-01-08 4:48 a.m., Andrew Burgess wrote:
> I wonder if it's worth including the case where ALL of the environment
> variables are empty as well, just to check that there's no issues with
> GDB falling through all the checks?

I don't know, but it wasn't too difficult so I did it.

> 
> Otherwise, LGTM.

I also thought it would be a good idea to check that the index-cache
directory value doesn't get affected by these empty values, that we
properly skip over them.  See updated patch below.


From c010770be40c91b97be5e97ce830abea4efd8f86 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 8 Jan 2021 10:47:53 -0500
Subject: [PATCH v2] gdb: check for empty strings in
 get_standard_cache_dir/get_standard_config_dir

New in v2:

 - Check the value of the index-cache directory before and after.
 - Check with all the env vars defined to empty.

As reported in PR 27157, if some environment variables read at startup
by GDB are defined but empty, we hit the assert in gdb_abspath:

    $ XDG_CACHE_HOME= ./gdb -nx --data-directory=data-directory -q
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==2007040==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000001b0 (pc 0x5639d4aa4127 bp 0x7ffdac232c00 sp 0x7ffdac232bf0 T0)
    ==2007040==The signal is caused by a READ memory access.
    ==2007040==Hint: address points to the zero page.
        #0 0x5639d4aa4126 in target_stack::top() const /home/smarchi/src/binutils-gdb/gdb/target.h:1334
        #1 0x5639d4aa41f1 in inferior::top_target() /home/smarchi/src/binutils-gdb/gdb/inferior.h:369
        #2 0x5639d4a70b1f in current_top_target() /home/smarchi/src/binutils-gdb/gdb/target.c:120
        #3 0x5639d4b00591 in gdb_readline_wrapper_cleanup::gdb_readline_wrapper_cleanup() /home/smarchi/src/binutils-gdb/gdb/top.c:1046
        #4 0x5639d4afab31 in gdb_readline_wrapper(char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1104
        #5 0x5639d4ccce2c in defaulted_query /home/smarchi/src/binutils-gdb/gdb/utils.c:893
        #6 0x5639d4ccd6af in query(char const*, ...) /home/smarchi/src/binutils-gdb/gdb/utils.c:985
        #7 0x5639d4ccaec1 in internal_vproblem /home/smarchi/src/binutils-gdb/gdb/utils.c:373
        #8 0x5639d4ccb3d1 in internal_verror(char const*, int, char const*, __va_list_tag*) /home/smarchi/src/binutils-gdb/gdb/utils.c:439
        #9 0x5639d5151a92 in internal_error(char const*, int, char const*, ...) /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
        #10 0x5639d5162ab4 in gdb_abspath(char const*) /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:132
        #11 0x5639d5162fac in get_standard_cache_dir[abi:cxx11]() /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:228
        #12 0x5639d3e76a81 in _initialize_index_cache() /home/smarchi/src/binutils-gdb/gdb/dwarf2/index-cache.c:325
        #13 0x5639d4dbbe92 in initialize_all_files() /home/smarchi/build/binutils-gdb/gdb/init.c:321
        #14 0x5639d4b00259 in gdb_init(char*) /home/smarchi/src/binutils-gdb/gdb/top.c:2344
        #15 0x5639d4440715 in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:950
        #16 0x5639d444252e in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1229
        #17 0x5639d44425cf in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1254
        #18 0x5639d3923371 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
        #19 0x7fa002d3f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
        #20 0x5639d392314d in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x4d414d)

gdb_abspath doesn't handle empty strings, so handle this case in the
callers.  If a variable is defined but empty, I think it's reasonable in
this case to just ignore it, as if it was not defined.

Note that this sometimes also lead to a segfault, because the failed
assertion happens very early during startup, before things are fully
initialized.

gdbsupport/ChangeLog:

	PR gdb/27157
	* pathstuff.cc (get_standard_cache_dir, get_standard_config_dir,
	find_gdb_home_config_file): Add empty string check.

gdb/testsuite/ChangeLog:

	PR gdb/27157
	* gdb.base/empty-host-env-vars.exp: New test.

Change-Id: I8654d8e97e74e1dff6d308c111ae4b1bbf07bef9
---
 .../gdb.base/empty-host-env-vars.exp          | 72 +++++++++++++++++++
 gdbsupport/pathstuff.cc                       | 14 ++--
 2 files changed, 79 insertions(+), 7 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp

diff --git a/gdb/testsuite/gdb.base/empty-host-env-vars.exp b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
new file mode 100644
index 000000000000..bd7212e7b655
--- /dev/null
+++ b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
@@ -0,0 +1,72 @@
+# Copyright 2021 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/>.
+
+# GDB reads some environment variables on startup, make sure it behaves
+# correctly if these variables are defined but empty.
+
+set all_env_vars { HOME XDG_CACHE_HOME LOCALAPPDATA XDG_CONFIG_HOME }
+
+# Record the initial value of the index-cache directory.
+clean_restart
+set index_cache_directory ""
+gdb_test_multiple "show index-cache directory" "" {
+    -re -wrap "The directory of the index cache is \"(.*)\"\\." {
+	set index_cache_directory $expect_out(1,string)
+	set index_cache_directory [string_to_regexp $index_cache_directory]
+	pass $gdb_test_name
+    }
+}
+
+foreach_with_prefix env_var_name $all_env_vars {
+    # Restore the original state of the environment variable.
+    save_vars env($env_var_name) {
+	set env($env_var_name) {}
+	clean_restart
+
+	# Verify that the empty environment variable didn't affect the
+	# index-cache directory setting, that we still see the initial value.
+	# "HOME" is different, because if that  one is unset, GDB isn't even
+	# able to compute the default location.  In that case, we expect it to
+	# be empty.
+	if { $env_var_name == "HOME" } {
+	    gdb_test "show index-cache directory" \
+		"The directory of the index cache is \"\"\\."
+	} else {
+	    gdb_test "show index-cache directory" \
+		"The directory of the index cache is \"$index_cache_directory\"\\."
+	}
+    }
+}
+
+# Try the same, but with all the env vars set to an empty value at the same
+# time.
+with_test_prefix "all env vars" {
+    set save_vars_arg {}
+    foreach env_var_name $all_env_vars {
+	lappend save_vars_arg env($env_var_name)
+    }
+
+    # Restore the original state of all the environment variables.
+    save_vars $save_vars_arg {
+	foreach env_var_name $all_env_vars {
+	    set env($env_var_name) {}
+	}
+
+	clean_restart
+
+	gdb_test "show index-cache directory" \
+	    "The directory of the index cache is \"\"\\."
+    }
+}
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
index 51ab8c651b20..ad13900819ef 100644
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -222,7 +222,7 @@ get_standard_cache_dir ()
 
 #ifndef __APPLE__
   const char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
-  if (xdg_cache_home != NULL)
+  if (xdg_cache_home != NULL && xdg_cache_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
@@ -231,7 +231,7 @@ get_standard_cache_dir ()
 #endif
 
   const char *home = getenv ("HOME");
-  if (home != NULL)
+  if (home != NULL && home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
@@ -240,14 +240,14 @@ get_standard_cache_dir ()
 
 #ifdef WIN32
   const char *win_home = getenv ("LOCALAPPDATA");
-  if (win_home != NULL)
+  if (win_home != NULL && win_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (win_home));
       return string_printf ("%s/gdb", abs.get ());
     }
 #endif
-    
+
   return {};
 }
 
@@ -289,7 +289,7 @@ get_standard_config_dir ()
 
 #ifndef __APPLE__
   const char *xdg_config_home = getenv ("XDG_CONFIG_HOME");
-  if (xdg_config_home != NULL)
+  if (xdg_config_home != NULL && xdg_config_home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_config_home));
@@ -298,7 +298,7 @@ get_standard_config_dir ()
 #endif
 
   const char *home = getenv ("HOME");
-  if (home != NULL)
+  if (home != NULL && home[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
@@ -340,7 +340,7 @@ find_gdb_home_config_file (const char *name, struct stat *buf)
     }
 
   const char *homedir = getenv ("HOME");
-  if (homedir != nullptr)
+  if (homedir != nullptr && homedir[0] != '\0')
     {
       /* Make sure the path is absolute and tilde-expanded.  */
       gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (homedir));
-- 
2.29.2

Re: [PATCH v2] gdb: check for empty strings in, get_standard_cache_dir/get_standard_config_dir

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-01-08 10:50:58 -0500]:

> On 2021-01-08 4:48 a.m., Andrew Burgess wrote:
> > I wonder if it's worth including the case where ALL of the environment
> > variables are empty as well, just to check that there's no issues with
> > GDB falling through all the checks?
> 
> I don't know, but it wasn't too difficult so I did it.
> 
> > 
> > Otherwise, LGTM.
> 
> I also thought it would be a good idea to check that the index-cache
> directory value doesn't get affected by these empty values, that we
> properly skip over them.  See updated patch below.
> 
> 
> From c010770be40c91b97be5e97ce830abea4efd8f86 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Fri, 8 Jan 2021 10:47:53 -0500
> Subject: [PATCH v2] gdb: check for empty strings in
>  get_standard_cache_dir/get_standard_config_dir
> 
> New in v2:
> 
>  - Check the value of the index-cache directory before and after.
>  - Check with all the env vars defined to empty.
> 
> As reported in PR 27157, if some environment variables read at startup
> by GDB are defined but empty, we hit the assert in gdb_abspath:
> 
>     $ XDG_CACHE_HOME= ./gdb -nx --data-directory=data-directory -q
>     AddressSanitizer:DEADLYSIGNAL
>     =================================================================
>     ==2007040==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000001b0 (pc 0x5639d4aa4127 bp 0x7ffdac232c00 sp 0x7ffdac232bf0 T0)
>     ==2007040==The signal is caused by a READ memory access.
>     ==2007040==Hint: address points to the zero page.
>         #0 0x5639d4aa4126 in target_stack::top() const /home/smarchi/src/binutils-gdb/gdb/target.h:1334
>         #1 0x5639d4aa41f1 in inferior::top_target() /home/smarchi/src/binutils-gdb/gdb/inferior.h:369
>         #2 0x5639d4a70b1f in current_top_target() /home/smarchi/src/binutils-gdb/gdb/target.c:120
>         #3 0x5639d4b00591 in gdb_readline_wrapper_cleanup::gdb_readline_wrapper_cleanup() /home/smarchi/src/binutils-gdb/gdb/top.c:1046
>         #4 0x5639d4afab31 in gdb_readline_wrapper(char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1104
>         #5 0x5639d4ccce2c in defaulted_query /home/smarchi/src/binutils-gdb/gdb/utils.c:893
>         #6 0x5639d4ccd6af in query(char const*, ...) /home/smarchi/src/binutils-gdb/gdb/utils.c:985
>         #7 0x5639d4ccaec1 in internal_vproblem /home/smarchi/src/binutils-gdb/gdb/utils.c:373
>         #8 0x5639d4ccb3d1 in internal_verror(char const*, int, char const*, __va_list_tag*) /home/smarchi/src/binutils-gdb/gdb/utils.c:439
>         #9 0x5639d5151a92 in internal_error(char const*, int, char const*, ...) /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
>         #10 0x5639d5162ab4 in gdb_abspath(char const*) /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:132
>         #11 0x5639d5162fac in get_standard_cache_dir[abi:cxx11]() /home/smarchi/src/binutils-gdb/gdbsupport/pathstuff.cc:228
>         #12 0x5639d3e76a81 in _initialize_index_cache() /home/smarchi/src/binutils-gdb/gdb/dwarf2/index-cache.c:325
>         #13 0x5639d4dbbe92 in initialize_all_files() /home/smarchi/build/binutils-gdb/gdb/init.c:321
>         #14 0x5639d4b00259 in gdb_init(char*) /home/smarchi/src/binutils-gdb/gdb/top.c:2344
>         #15 0x5639d4440715 in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:950
>         #16 0x5639d444252e in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1229
>         #17 0x5639d44425cf in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1254
>         #18 0x5639d3923371 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
>         #19 0x7fa002d3f0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
>         #20 0x5639d392314d in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x4d414d)
> 
> gdb_abspath doesn't handle empty strings, so handle this case in the
> callers.  If a variable is defined but empty, I think it's reasonable in
> this case to just ignore it, as if it was not defined.
> 
> Note that this sometimes also lead to a segfault, because the failed
> assertion happens very early during startup, before things are fully
> initialized.
> 
> gdbsupport/ChangeLog:
> 
> 	PR gdb/27157
> 	* pathstuff.cc (get_standard_cache_dir, get_standard_config_dir,
> 	find_gdb_home_config_file): Add empty string check.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR gdb/27157
> 	* gdb.base/empty-host-env-vars.exp: New test.

LGTM.

Thanks,
Andrew


> 
> Change-Id: I8654d8e97e74e1dff6d308c111ae4b1bbf07bef9
> ---
>  .../gdb.base/empty-host-env-vars.exp          | 72 +++++++++++++++++++
>  gdbsupport/pathstuff.cc                       | 14 ++--
>  2 files changed, 79 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp
> 
> diff --git a/gdb/testsuite/gdb.base/empty-host-env-vars.exp b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
> new file mode 100644
> index 000000000000..bd7212e7b655
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/empty-host-env-vars.exp
> @@ -0,0 +1,72 @@
> +# Copyright 2021 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/>.
> +
> +# GDB reads some environment variables on startup, make sure it behaves
> +# correctly if these variables are defined but empty.
> +
> +set all_env_vars { HOME XDG_CACHE_HOME LOCALAPPDATA XDG_CONFIG_HOME }
> +
> +# Record the initial value of the index-cache directory.
> +clean_restart
> +set index_cache_directory ""
> +gdb_test_multiple "show index-cache directory" "" {
> +    -re -wrap "The directory of the index cache is \"(.*)\"\\." {
> +	set index_cache_directory $expect_out(1,string)
> +	set index_cache_directory [string_to_regexp $index_cache_directory]
> +	pass $gdb_test_name
> +    }
> +}
> +
> +foreach_with_prefix env_var_name $all_env_vars {
> +    # Restore the original state of the environment variable.
> +    save_vars env($env_var_name) {
> +	set env($env_var_name) {}
> +	clean_restart
> +
> +	# Verify that the empty environment variable didn't affect the
> +	# index-cache directory setting, that we still see the initial value.
> +	# "HOME" is different, because if that  one is unset, GDB isn't even
> +	# able to compute the default location.  In that case, we expect it to
> +	# be empty.
> +	if { $env_var_name == "HOME" } {
> +	    gdb_test "show index-cache directory" \
> +		"The directory of the index cache is \"\"\\."
> +	} else {
> +	    gdb_test "show index-cache directory" \
> +		"The directory of the index cache is \"$index_cache_directory\"\\."
> +	}
> +    }
> +}
> +
> +# Try the same, but with all the env vars set to an empty value at the same
> +# time.
> +with_test_prefix "all env vars" {
> +    set save_vars_arg {}
> +    foreach env_var_name $all_env_vars {
> +	lappend save_vars_arg env($env_var_name)
> +    }
> +
> +    # Restore the original state of all the environment variables.
> +    save_vars $save_vars_arg {
> +	foreach env_var_name $all_env_vars {
> +	    set env($env_var_name) {}
> +	}
> +
> +	clean_restart
> +
> +	gdb_test "show index-cache directory" \
> +	    "The directory of the index cache is \"\"\\."
> +    }
> +}
> diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
> index 51ab8c651b20..ad13900819ef 100644
> --- a/gdbsupport/pathstuff.cc
> +++ b/gdbsupport/pathstuff.cc
> @@ -222,7 +222,7 @@ get_standard_cache_dir ()
>  
>  #ifndef __APPLE__
>    const char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
> -  if (xdg_cache_home != NULL)
> +  if (xdg_cache_home != NULL && xdg_cache_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
> @@ -231,7 +231,7 @@ get_standard_cache_dir ()
>  #endif
>  
>    const char *home = getenv ("HOME");
> -  if (home != NULL)
> +  if (home != NULL && home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
> @@ -240,14 +240,14 @@ get_standard_cache_dir ()
>  
>  #ifdef WIN32
>    const char *win_home = getenv ("LOCALAPPDATA");
> -  if (win_home != NULL)
> +  if (win_home != NULL && win_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (win_home));
>        return string_printf ("%s/gdb", abs.get ());
>      }
>  #endif
> -    
> +
>    return {};
>  }
>  
> @@ -289,7 +289,7 @@ get_standard_config_dir ()
>  
>  #ifndef __APPLE__
>    const char *xdg_config_home = getenv ("XDG_CONFIG_HOME");
> -  if (xdg_config_home != NULL)
> +  if (xdg_config_home != NULL && xdg_config_home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_config_home));
> @@ -298,7 +298,7 @@ get_standard_config_dir ()
>  #endif
>  
>    const char *home = getenv ("HOME");
> -  if (home != NULL)
> +  if (home != NULL && home[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
> @@ -340,7 +340,7 @@ find_gdb_home_config_file (const char *name, struct stat *buf)
>      }
>  
>    const char *homedir = getenv ("HOME");
> -  if (homedir != nullptr)
> +  if (homedir != nullptr && homedir[0] != '\0')
>      {
>        /* Make sure the path is absolute and tilde-expanded.  */
>        gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (homedir));
> -- 
> 2.29.2
> 

Re: [PATCH v2] gdb: check for empty strings in, get_standard_cache_dir/get_standard_config_dir

[Simon Marchi]

> LGTM.
> 
> Thanks,
> Andrew

Thanks, pushed!

Simon