From 6abd4cf281deda4b1eb2d569a2729a485105e553 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 8 Jan 2021 13:46:56 -0500 Subject: [PATCH] gdb: check for empty strings in get_standard_cache_dir/get_standard_config_dir 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/testsuite/ChangeLog | 5 ++ .../gdb.base/empty-host-env-vars.exp | 72 +++++++++++++++++++ gdbsupport/ChangeLog | 6 ++ gdbsupport/pathstuff.cc | 14 ++-- 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 gdb/testsuite/gdb.base/empty-host-env-vars.exp diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 0c5e3604c8e..ef3e7949164 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2021-01-08 Simon Marchi + + PR gdb/27157 + * gdb.base/empty-host-env-vars.exp: New test. + 2021-01-08 Andrew Burgess * gdb.base/completion.exp: Add a new test. 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..bd7212e7b65 --- /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 . + +# 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/ChangeLog b/gdbsupport/ChangeLog index 4cac86fbe49..d973a6daa4f 100644 --- a/gdbsupport/ChangeLog +++ b/gdbsupport/ChangeLog @@ -1,3 +1,9 @@ +2021-01-08 Simon Marchi + + PR gdb/27157 + * pathstuff.cc (get_standard_cache_dir, get_standard_config_dir, + find_gdb_home_config_file): Add empty string check. + 2021-01-06 Mike Frysinger * common-utils.h (align_up): Fix typo in header comment. 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 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 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 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 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 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 abs (gdb_abspath (homedir)); -- 2.30.2